Message ID | 20190618091743.3078-1-anup.patel@wdc.com |
---|---|
Headers | show |
Series | Update SiFive Unleashed Clock Driver | expand |
> 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)
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