mbox series

[U-Boot,0/4] Update SiFive Unleashed Clock Driver

Message ID 20190618091743.3078-1-anup.patel@wdc.com
Headers show
Series Update SiFive Unleashed Clock Driver | expand

Message

Anup Patel June 18, 2019, 9:18 a.m. UTC
This series update SiFive Unleashed clock driver so that:
1. It is in sync with upstream Linux driver
2. It uses latest DT bindings as-per upstream Linux driver

With this series, we can now use latest DT bindings with U-Boot. I have
tested SiFive Serial driver and Cadence MACB ethernet driver with this
changes and both work fine.

The legacy FSBL will still pass DTB to U-Boot with older DT bindings
which will break the updated SiFive Unleashed clock driver. To tackle
this, we have embedded DTB in OpenSBI FW_PAYLOAD firmware for SiFive
Unleashed so that OpenSBI will override and pass updated DTB to U-Boot.

In fact, the updated DTB passed by OpenSBI can be used by latest Linux
(i.e. V5.2-rc1 or higher) as well.

The OpenSBI changes to embed SiFive Unleashed DTB can be found in
sifive_unleashed_dtb_fix_v1 branch of:
https://github.com/avpatel/opensbi.git

This series can be found in riscv_unleashed_clk_sync_v1 branch of:
https://github.com/avpatel/u-boot.git

Anup Patel (4):
  clk: sifive: Factor-out PLL library as separate module
  clk: sifive: Sync-up WRPLL library with upstream Linux
  clk: sifive: Sync-up DT bindings header with upstream Linux
  clk: sifive: Sync-up main driver with upstream Linux

 drivers/clk/Kconfig                           |   1 +
 drivers/clk/Makefile                          |   1 +
 drivers/clk/analogbits/Kconfig                |   4 +
 drivers/clk/analogbits/Makefile               |   3 +
 .../{sifive => analogbits}/wrpll-cln28hpc.c   | 168 ++++++++----------
 drivers/clk/sifive/Kconfig                    |   3 -
 drivers/clk/sifive/Makefile                   |   2 -
 drivers/clk/sifive/fu540-prci.c               | 123 +++++++------
 include/dt-bindings/clk/sifive-fu540-prci.h   |  29 ---
 include/dt-bindings/clock/sifive-fu540-prci.h |  18 ++
 .../linux/clk}/analogbits-wrpll-cln28hpc.h    |  70 +++-----
 11 files changed, 195 insertions(+), 227 deletions(-)
 create mode 100644 drivers/clk/analogbits/Kconfig
 create mode 100644 drivers/clk/analogbits/Makefile
 rename drivers/clk/{sifive => analogbits}/wrpll-cln28hpc.c (69%)
 delete mode 100644 include/dt-bindings/clk/sifive-fu540-prci.h
 create mode 100644 include/dt-bindings/clock/sifive-fu540-prci.h
 rename {drivers/clk/sifive => include/linux/clk}/analogbits-wrpll-cln28hpc.h (52%)

--
2.17.1

Comments

Troy Benjegerdes June 19, 2019, 3:11 a.m. UTC | #1
> On Jun 18, 2019, at 4:18 AM, Anup Patel <Anup.Patel@wdc.com> wrote:
> 
> This series update SiFive Unleashed clock driver so that:
> 1. It is in sync with upstream Linux driver
> 2. It uses latest DT bindings as-per upstream Linux driver
> 
> With this series, we can now use latest DT bindings with U-Boot. I have
> tested SiFive Serial driver and Cadence MACB ethernet driver with this
> changes and both work fine.
> 
> The legacy FSBL will still pass DTB to U-Boot with older DT bindings
> which will break the updated SiFive Unleashed clock driver. To tackle
> this, we have embedded DTB in OpenSBI FW_PAYLOAD firmware for SiFive
> Unleashed so that OpenSBI will override and pass updated DTB to U-Boot.
> 
> In fact, the updated DTB passed by OpenSBI can be used by latest Linux
> (i.e. V5.2-rc1 or higher) as well.
> 
> The OpenSBI changes to embed SiFive Unleashed DTB can be found in
> sifive_unleashed_dtb_fix_v1 branch of:
> https://github.com/avpatel/opensbi.git
> 
> This series can be found in riscv_unleashed_clk_sync_v1 branch of:
> https://github.com/avpatel/u-boot.git

There are some discrepancies I’m finding in the clocking… If I use the
HiFive_U-Boot as FSBL, it does the following (with some extra debug
code)

U-Boot 2018.09-g8bbdd51-dirty (Jun 18 2019 - 19:46:55 -0700)

DRAM:  2 GiB
MMC:
In:    serial
Out:   serial
Err:   serial
Net:   gem_mdc_clk_div: macb_hz: 500000000
gmac0
Hit any key to stop autoboot:  0

Where as the close-to-u-boot master version does this:

U-Boot 2019.07-rc4-05661-gc998035-dirty (Jun 18 2019 - 19:51:10 -0700)

CPU:   rv64imafdc
Model: sifive,hifive-unleashed-a00
DRAM:  8 GiB
In:    serial@10010000
Out:   serial@10010000
Err:   serial@10010000
Net:   gem_mdc_clk_div: macb_hz: 933333324

For reference, I’ve checked all my changes into the ‘dev/new-dts’ branch
of github.com/tmagik/freedom-u-sdk, and the ‘make test_s’ target will build
and then copy various files to /var/lib/tftpboot to run the test, and assumes
you have a DHCP and TFTP server running locally.

Another issue with the mainline u-boot is it does not have the OTP driver
that the HiFive_U-Boot version does, so when it selects a random mac
address this causes TFTP to fail in one of my test environments.

FYI, the patch applied is:

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 5a6cba5..d21026d 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -813,6 +813,8 @@ static u32 macb_mdc_clk_div(int id, struct macb_device *macb)
        unsigned long macb_hz = HIFIVE_ETHERNET_CLK_FREQ;//get_macb_pclk_rate(id);
 #endif
 
+       printf("%s: macb_hz: %d\n", __func__, macb_hz);
+
        if (macb_hz < 20000000)
                config = MACB_BF(CLK, MACB_CLK_DIV8);
        else if (macb_hz < 40000000)
@@ -835,6 +837,7 @@ static u32 gem_mdc_clk_div(int id, struct macb_device *macb)
        unsigned long macb_hz = HIFIVE_ETHERNET_CLK_FREQ;//get_macb_pclk_rate(id);
 #endif
 
+       printf("%s: macb_hz: %d\n", __func__, macb_hz);
        if (macb_hz < 20000000)
                config = GEM_BF(CLK, GEM_CLK_DIV8);
        else if (macb_hz < 40000000)
Anup Patel June 19, 2019, 5:42 a.m. UTC | #2
On Wed, Jun 19, 2019 at 8:41 AM Troy Benjegerdes
<troy.benjegerdes@sifive.com> wrote:
>
>
>
> > On Jun 18, 2019, at 4:18 AM, Anup Patel <Anup.Patel@wdc.com> wrote:
> >
> > This series update SiFive Unleashed clock driver so that:
> > 1. It is in sync with upstream Linux driver
> > 2. It uses latest DT bindings as-per upstream Linux driver
> >
> > With this series, we can now use latest DT bindings with U-Boot. I have
> > tested SiFive Serial driver and Cadence MACB ethernet driver with this
> > changes and both work fine.
> >
> > The legacy FSBL will still pass DTB to U-Boot with older DT bindings
> > which will break the updated SiFive Unleashed clock driver. To tackle
> > this, we have embedded DTB in OpenSBI FW_PAYLOAD firmware for SiFive
> > Unleashed so that OpenSBI will override and pass updated DTB to U-Boot.
> >
> > In fact, the updated DTB passed by OpenSBI can be used by latest Linux
> > (i.e. V5.2-rc1 or higher) as well.
> >
> > The OpenSBI changes to embed SiFive Unleashed DTB can be found in
> > sifive_unleashed_dtb_fix_v1 branch of:
> > https://github.com/avpatel/opensbi.git
> >
> > This series can be found in riscv_unleashed_clk_sync_v1 branch of:
> > https://github.com/avpatel/u-boot.git
>
> There are some discrepancies I’m finding in the clocking… If I use the
> HiFive_U-Boot as FSBL, it does the following (with some extra debug
> code)
>
> U-Boot 2018.09-g8bbdd51-dirty (Jun 18 2019 - 19:46:55 -0700)
>
> DRAM:  2 GiB
> MMC:
> In:    serial
> Out:   serial
> Err:   serial
> Net:   gem_mdc_clk_div: macb_hz: 500000000
> gmac0
> Hit any key to stop autoboot:  0
>
> Where as the close-to-u-boot master version does this:
>
> U-Boot 2019.07-rc4-05661-gc998035-dirty (Jun 18 2019 - 19:51:10 -0700)
>
> CPU:   rv64imafdc
> Model: sifive,hifive-unleashed-a00
> DRAM:  8 GiB
> In:    serial@10010000
> Out:   serial@10010000
> Err:   serial@10010000
> Net:   gem_mdc_clk_div: macb_hz: 933333324
>
> For reference, I’ve checked all my changes into the ‘dev/new-dts’ branch
> of github.com/tmagik/freedom-u-sdk, and the ‘make test_s’ target will build
> and then copy various files to /var/lib/tftpboot to run the test, and assumes
> you have a DHCP and TFTP server running locally.
>
> Another issue with the mainline u-boot is it does not have the OTP driver
> that the HiFive_U-Boot version does, so when it selects a random mac
> address this causes TFTP to fail in one of my test environments.
>
> FYI, the patch applied is:
>
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 5a6cba5..d21026d 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -813,6 +813,8 @@ static u32 macb_mdc_clk_div(int id, struct macb_device *macb)
>         unsigned long macb_hz = HIFIVE_ETHERNET_CLK_FREQ;//get_macb_pclk_rate(id);
>  #endif
>
> +       printf("%s: macb_hz: %d\n", __func__, macb_hz);
> +
>         if (macb_hz < 20000000)
>                 config = MACB_BF(CLK, MACB_CLK_DIV8);
>         else if (macb_hz < 40000000)
> @@ -835,6 +837,7 @@ static u32 gem_mdc_clk_div(int id, struct macb_device *macb)
>         unsigned long macb_hz = HIFIVE_ETHERNET_CLK_FREQ;//get_macb_pclk_rate(id);
>  #endif
>
> +       printf("%s: macb_hz: %d\n", __func__, macb_hz);
>         if (macb_hz < 20000000)
>                 config = GEM_BF(CLK, GEM_CLK_DIV8);
>         else if (macb_hz < 40000000)

We had noticed this difference when we first got upstream U-Boot
MACB driver working on SiFive Unleashed board.

The real issue is the hard-coding in HiFive_U-Boot because even the
Linux MACB driver sees different value. The clock frequency we see
upstream U-Boot matches Linux MACB driver because we are using
similar SiFive clock driver. (@Atish, please correct me if I am wrong
here?)

Eventually, we realized that hard-coding in HiFive_U-Boot for MACB
driver is just a HACK. There were lot of such hacks in HiFive_U-Boot
without any GIT history. We trust the upstream Linux drivers more
hence took those as reference for upstream U-Boot.

Regards,
Anup