| Message ID | 1560745167-9866-1-git-send-email-yash.shah@sifive.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add macb support for SiFive FU540-C000 | expand |
On Mon, Jun 17, 2019 at 9:49 AM Yash Shah <yash.shah@sifive.com> wrote: > > On FU540, the management IP block is tightly coupled with the Cadence > MACB IP block. It manages many of the boundary signals from the MACB IP > This patchset controls the tx_clk input signal to the MACB IP. It > switches between the local TX clock (125MHz) and PHY TX clocks. This > is necessary to toggle between 1Gb and 100/10Mb speeds. > > Future patches may add support for monitoring or controlling other IP > boundary signals. > > This patchset is mostly based on work done by > Wesley Terpstra <wesley@sifive.com> > > This patchset is based on Linux v5.2-rc1 and tested on HiFive Unleashed > board with additional board related patches needed for testing can be > found at dev/yashs/ethernet branch of: Correction in branch name: dev/yashs/ethernet_v2 > https://github.com/yashshah7/riscv-linux.git > > Change History: > V2: > - Change compatible string from "cdns,fu540-macb" to "sifive,fu540-macb" > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb > driver. This is needed because on FU540, the macb driver depends on > SiFive GPIO driver. > - Avoid writing the result of a comparison to a register. > - Fix the issue of probe fail on reloading the module reported by: > Andreas Schwab <schwab@suse.de> > > Yash Shah (2): > macb: bindings doc: add sifive fu540-c000 binding > macb: Add support for SiFive FU540-C000 > > Documentation/devicetree/bindings/net/macb.txt | 3 + > drivers/net/ethernet/cadence/Kconfig | 6 ++ > drivers/net/ethernet/cadence/macb_main.c | 129 +++++++++++++++++++++++++ > 3 files changed, 138 insertions(+) > > -- > 1.9.1 >
On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote: > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb > driver. This is needed because on FU540, the macb driver depends on > SiFive GPIO driver. This of course requires that the GPIO driver is upstreamed first. Andreas.
Hi Yash, On Mon, 17 Jun 2019, Andreas Schwab wrote: > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote: > > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb > > driver. This is needed because on FU540, the macb driver depends on > > SiFive GPIO driver. > > This of course requires that the GPIO driver is upstreamed first. What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO driver isn't present? (After modifying the Kconfig "depends" line appropriately.) Looks to me that it shouldn't have an impact unless the DT string is present, and even then, the impact might simply be that the MACB driver may not work? - Paul
On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: > Looks to me that it shouldn't have an impact unless the DT string is > present, and even then, the impact might simply be that the MACB driver > may not work? If the macb driver doesn't work you have an unusable system, of course. Andreas.
On Mon, 17 Jun 2019, Andreas Schwab wrote: > On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > Looks to me that it shouldn't have an impact unless the DT string is > > present, and even then, the impact might simply be that the MACB driver > > may not work? > > If the macb driver doesn't work you have an unusable system, of course. Why? - Paul
On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: > On Mon, 17 Jun 2019, Andreas Schwab wrote: > >> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: >> >> > Looks to me that it shouldn't have an impact unless the DT string is >> > present, and even then, the impact might simply be that the MACB driver >> > may not work? >> >> If the macb driver doesn't work you have an unusable system, of course. > > Why? Because a system is useless without network. Andreas.
On Mon, Jun 17, 2019 at 3:28 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > Hi Yash, > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > > > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote: > > > > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb > > > driver. This is needed because on FU540, the macb driver depends on > > > SiFive GPIO driver. > > > > This of course requires that the GPIO driver is upstreamed first. > > What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO > driver isn't present? (After modifying the Kconfig "depends" line > appropriately.) > > Looks to me that it shouldn't have an impact unless the DT string is > present, and even then, the impact might simply be that the MACB driver > may not work? Yes, there won't be an impact other than MACB driver not working. In any case, without GPIO driver, PHY won't get reset and the network interface won't come up. > > > - Paul
On Mon, 17 Jun 2019, Yash Shah wrote: > On Mon, Jun 17, 2019 at 3:28 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > > > > > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote: > > > > > > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb > > > > driver. This is needed because on FU540, the macb driver depends on > > > > SiFive GPIO driver. > > > > > > This of course requires that the GPIO driver is upstreamed first. > > > > What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO > > driver isn't present? (After modifying the Kconfig "depends" line > > appropriately.) > > > > Looks to me that it shouldn't have an impact unless the DT string is > > present, and even then, the impact might simply be that the MACB driver > > may not work? > > Yes, there won't be an impact other than MACB driver not working. OK. In that case, there doesn't seem much point to adding the Kconfig option. Could you please post a new version without it? > In any case, without GPIO driver, PHY won't get reset and the network > interface won't come up. Naturally, in the medium term, we want Linux to handle the reset. But if there's no GPIO driver present, and the bootloader handles the PHY reset before the kernel starts, would the network driver work in that case? - Paul
On Mon, Jun 17, 2019 at 3:58 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > On Mon, 17 Jun 2019, Yash Shah wrote: > > > On Mon, Jun 17, 2019 at 3:28 PM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > > > > > > > On Jun 17 2019, Yash Shah <yash.shah@sifive.com> wrote: > > > > > > > > > - Add "MACB_SIFIVE_FU540" in Kconfig to support SiFive FU540 in macb > > > > > driver. This is needed because on FU540, the macb driver depends on > > > > > SiFive GPIO driver. > > > > > > > > This of course requires that the GPIO driver is upstreamed first. > > > > > > What's the impact of enabling CONFIG_MACB_SIFIVE_FU540 when the GPIO > > > driver isn't present? (After modifying the Kconfig "depends" line > > > appropriately.) > > > > > > Looks to me that it shouldn't have an impact unless the DT string is > > > present, and even then, the impact might simply be that the MACB driver > > > may not work? > > > > Yes, there won't be an impact other than MACB driver not working. > > OK. In that case, there doesn't seem much point to adding the Kconfig > option. Could you please post a new version without it? Sure, will do that. > > > In any case, without GPIO driver, PHY won't get reset and the network > > interface won't come up. > > Naturally, in the medium term, we want Linux to handle the reset. But if > there's no GPIO driver present, and the bootloader handles the PHY reset > before the kernel starts, would the network driver work in that case? Yes, if bootloader handles the PHY reset then the network driver will work in that case. I will post a new version without the GPIO driver dependency. > > > - Paul
On Mon, 17 Jun 2019, Andreas Schwab wrote: > On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > > > >> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: > >> > >> > Looks to me that it shouldn't have an impact unless the DT string is > >> > present, and even then, the impact might simply be that the MACB driver > >> > may not work? > >> > >> If the macb driver doesn't work you have an unusable system, of course. > > > > Why? > > Because a system is useless without network. From an upstream Linux point of view, Yash's patches should be an improvement over the current mainline kernel situation, since there's currently no upstream support for the (SiFive-specific) TX clock switch register. With the right DT data, and a bootloader that handles the PHY reset, I think networking should work after his patches are upstream -- although I myself haven't tried this yet. - Paul
> On Jun 17, 2019, at 6:34 AM, Paul Walmsley <paul.walmsley@sifive.com> wrote: > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > >> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: >> >>> On Mon, 17 Jun 2019, Andreas Schwab wrote: >>> >>>> On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: >>>> >>>>> Looks to me that it shouldn't have an impact unless the DT string is >>>>> present, and even then, the impact might simply be that the MACB driver >>>>> may not work? >>>> >>>> If the macb driver doesn't work you have an unusable system, of course. >>> >>> Why? >> >> Because a system is useless without network. > > From an upstream Linux point of view, Yash's patches should be an > improvement over the current mainline kernel situation, since there's > currently no upstream support for the (SiFive-specific) TX clock switch > register. With the right DT data, and a bootloader that handles the PHY > reset, I think networking should work after his patches are upstream -- > although I myself haven't tried this yet. > Have we documented this tx clock switch register in something with a direct URL link (rather than a PDF)? I’d like to update freedom-u-sdk (or yocto) to create bootable images with a working U-boot (upstream or not, I don’t care, as long as it works), and what I have right now is the old legacy HiFive U-boot[1] and a 4.19 kernel with a bunch of extra patches. The legacy M-mode U-boot handles the phy reset already, and I’ve been able to load upstream S-mode uboot as a payload via TFTP, and then load and boot a 4.19 kernel. It would be nice to get this all working with 5.x, however there are still several missing pieces to really have it work well. [1] https://github.com/sifive/HiFive_U-Boot
On Mon, 17 Jun 2019, Troy Benjegerdes wrote: > Have we documented this tx clock switch register in something with a > direct URL link (rather than a PDF)? The SiFive FU540 user manual PDF is the canonical public reference: https://static.dev.sifive.com/FU540-C000-v1.0.pdf This practice aligns with other SoC vendors, who also release PDFs. The relevant Ethernet documentation, including register maps, is in Chapter 19. - Paul
On Mon, 2019-06-17 at 09:14 -0500, Troy Benjegerdes wrote: > > On Jun 17, 2019, at 6:34 AM, Paul Walmsley < > > paul.walmsley@sifive.com> wrote: > > > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > > > > > On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> wrote: > > > > > > > On Mon, 17 Jun 2019, Andreas Schwab wrote: > > > > > > > > > On Jun 17 2019, Paul Walmsley <paul.walmsley@sifive.com> > > > > > wrote: > > > > > > > > > > > Looks to me that it shouldn't have an impact unless the DT > > > > > > string is > > > > > > present, and even then, the impact might simply be that the > > > > > > MACB driver > > > > > > may not work? > > > > > > > > > > If the macb driver doesn't work you have an unusable system, > > > > > of course. > > > > > > > > Why? > > > > > > Because a system is useless without network. > > > > From an upstream Linux point of view, Yash's patches should be an > > improvement over the current mainline kernel situation, since > > there's > > currently no upstream support for the (SiFive-specific) TX clock > > switch > > register. With the right DT data, and a bootloader that handles > > the PHY > > reset, I think networking should work after his patches are > > upstream -- > > although I myself haven't tried this yet. > > > > Have we documented this tx clock switch register in something with a > direct URL link (rather than a PDF)? > > I’d like to update freedom-u-sdk (or yocto) to create bootable images > with a working U-boot (upstream or not, I don’t care, as long as it > works), > and what I have right now is the old legacy HiFive U-boot[1] and a > 4.19 > kernel with a bunch of extra patches. Yocto/OpenEmbedded does this today. TFTP boot works with the 2019.04 U- Boot (+ some patches ontop for SMP support). We use the latest 5.1 stable kernel plus 5 or so patches to boot on the Unleased. Networking, display and audio are all working with the Microsemi expansion board as well. Let me know if there is something else missing and I'll add it in. There are probably documentation fixes that are needed as well. I was thinking of skipping the 5.2 release though as I thought the DT stuff wasn't going to make it [1]. I will probably re-evaluate that decision though when 5.2 comes out as it looks like it's all going to work :) With U-boot 2017.09 and Linux 5.2/5.3 we should finally be upstream only! > > The legacy M-mode U-boot handles the phy reset already, and I’ve been > able to load upstream S-mode uboot as a payload via TFTP, and then > load and boot a 4.19 kernel. > > It would be nice to get this all working with 5.x, however there are > still > several missing pieces to really have it work well. Let me know what is still missing/doesn't work and I can add it. At the moment the only known issue I know of is a missing SD card driver in U- Boot. 1: https://github.com/riscv/meta-riscv/issues/143 Alistair > > > [1] https://github.com/sifive/HiFive_U-Boot > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Mon, 17 Jun 2019, Alistair Francis wrote: > > The legacy M-mode U-boot handles the phy reset already, and I’ve been > > able to load upstream S-mode uboot as a payload via TFTP, and then > > load and boot a 4.19 kernel. > > > > It would be nice to get this all working with 5.x, however there are > > still > > several missing pieces to really have it work well. > > Let me know what is still missing/doesn't work and I can add it. At the > moment the only known issue I know of is a missing SD card driver in U- > Boot. The DT data has changed between the non-upstream data that people developed against previously, vs. the DT data that just went upstream here: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852 and https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205 So Upstream U-Boot is going to need several patches to get things working again. Clock identifiers and Ethernet are two known areas. - Paul
On Tue, Jun 18, 2019 at 8:56 AM Paul Walmsley <paul.walmsley@sifive.com> wrote: > > On Mon, 17 Jun 2019, Alistair Francis wrote: > > > > The legacy M-mode U-boot handles the phy reset already, and I’ve been > > > able to load upstream S-mode uboot as a payload via TFTP, and then > > > load and boot a 4.19 kernel. > > > > > > It would be nice to get this all working with 5.x, however there are > > > still > > > several missing pieces to really have it work well. > > > > Let me know what is still missing/doesn't work and I can add it. At the > > moment the only known issue I know of is a missing SD card driver in U- > > Boot. > > The DT data has changed between the non-upstream data that people > developed against previously, vs. the DT data that just went upstream > here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852 > > and > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205 > > So Upstream U-Boot is going to need several patches to get things working > again. Clock identifiers and Ethernet are two known areas. Done. I just send-out few patches to fix U-Boot SiFive Clock driver. The U-Boot SiFive Clock driver fix series can be found in riscv_unleashed_clk_sync_v1 branch of: https://github.com/avpatel/u-boot.git Users will also require OpenSBI DTB fix which can be found in sifive_unleashed_dtb_fix_v1 branch of: https://github.com/avpatel/opensbi.git With above fixes, we can now use same DTB for both U-Boot and Linux kernel (5.2-rc1). Although, users are free to pass a different DTB to Linux kernel via TFTP. I have tested SiFive serial and Cadance MACB ethernet on both U-Boot and Linux (5.2-rc1) Regards, Anup
> On Jun 18, 2019, at 4:32 AM, Anup Patel <anup@brainfault.org> wrote: > >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852 I added your patches, along with two of mine, and rebased them to the latest U-boot master, and put them on the ‘to-upstream’ branch at https://github.com/sifive/u-boot/tree/to-upstream I am most interested in review of the patch that adds the DTS files from Linux to U-boot, along with a ‘-u-boot.dtsi’ file which includes several extra things, most notably an ethernet entry [1] which does not match the new proposed changes for the MacB driver that Yash is working on. How close are we to consensus on the new “sifive,fu540-macb” device tree entry format? Is this something that is stable enough to start basing some work in M-mode U-boot on yet, or do we expect more changes? [1] https://github.com/sifive/u-boot/commit/35e4168e36139722f30143a0ca0aa8637dd3ee04#diff-27d2d375ddac52f1bca71594075e1be4R93
On Mon, 2019-06-17 at 20:26 -0700, Paul Walmsley wrote: > On Mon, 17 Jun 2019, Alistair Francis wrote: > > > > The legacy M-mode U-boot handles the phy reset already, and I’ve > > > been > > > able to load upstream S-mode uboot as a payload via TFTP, and > > > then > > > load and boot a 4.19 kernel. > > > > > > It would be nice to get this all working with 5.x, however there > > > are > > > still > > > several missing pieces to really have it work well. > > > > Let me know what is still missing/doesn't work and I can add it. At > > the > > moment the only known issue I know of is a missing SD card driver > > in U- > > Boot. > > The DT data has changed between the non-upstream data that people > developed against previously, vs. the DT data that just went > upstream > here: > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852 > > and > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205 > > So Upstream U-Boot is going to need several patches to get things > working > again. Clock identifiers and Ethernet are two known areas. Yep, Anup has sent patches to U-Boot and OpenSBI. Alistair > > > - Paul
On Tue, 2019-06-18 at 15:02 +0530, Anup Patel wrote: > On Tue, Jun 18, 2019 at 8:56 AM Paul Walmsley < > paul.walmsley@sifive.com> wrote: > > On Mon, 17 Jun 2019, Alistair Francis wrote: > > > > > > The legacy M-mode U-boot handles the phy reset already, and > > > > I’ve been > > > > able to load upstream S-mode uboot as a payload via TFTP, and > > > > then > > > > load and boot a 4.19 kernel. > > > > > > > > It would be nice to get this all working with 5.x, however > > > > there are > > > > still > > > > several missing pieces to really have it work well. > > > > > > Let me know what is still missing/doesn't work and I can add it. > > > At the > > > moment the only known issue I know of is a missing SD card driver > > > in U- > > > Boot. > > > > The DT data has changed between the non-upstream data that people > > developed against previously, vs. the DT data that just went > > upstream > > here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=72296bde4f4207566872ee355950a59cbc29f852 > > > > and > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c35f1b87fc595807ff15d2834d241f9771497205 > > > > So Upstream U-Boot is going to need several patches to get things > > working > > again. Clock identifiers and Ethernet are two known areas. > > Done. > > I just send-out few patches to fix U-Boot SiFive Clock driver. > > The U-Boot SiFive Clock driver fix series can be found in > riscv_unleashed_clk_sync_v1 branch of: > https://github.com/avpatel/u-boot.git > > Users will also require OpenSBI DTB fix which can be found > in sifive_unleashed_dtb_fix_v1 branch of: > https://github.com/avpatel/opensbi.git > Additionally, user can also use FW_PAYLOAD_FDT_PATH argument during build to include the dtb built from kernel. example: make -j 32 PLATFORM=sifive/fu540 FW_PAYLOAD_PATH=<u-boot path>/u- boot.bin FW_PAYLOAD_FDT_PATH=<kernel_dtb_path> > With above fixes, we can now use same DTB for both U-Boot > and Linux kernel (5.2-rc1). Although, users are free to pass a > different DTB to Linux kernel via TFTP. > > I have tested SiFive serial and Cadance MACB ethernet on > both U-Boot and Linux (5.2-rc1) > > Regards, > Anup