| Message ID | 20230211031821.976408-1-cristian.ciocaltea@collabora.com |
|---|---|
| Headers | show |
| Series | Enable networking support for StarFive JH7100 SoC | expand |
Hey Cristian! +CC Arnd, Prabhakar On Sat, Feb 11, 2023 at 05:18:09AM +0200, Cristian Ciocaltea wrote: > This patch series adds ethernet support for the StarFive JH7100 SoC and > makes it available for the StarFive VisionFive V1 and BeagleV Starlight > boards, although I could only validate on the former SBC. > > The work is heavily based on the reference implementation [1] and requires > the non-coherent DMA support provided by Emil via the Sifive Composable > Cache controller. > > Also note there is an overlap in "[PATCH 08/12] net: stmmac: Add glue layer > for StarFive JH7100 SoC" with the Yanhong Wang's upstreaming attempt [2]: > "[PATCH v4 5/7] net: stmmac: Add glue layer for StarFive JH7110 SoCs". > > Since I cannot test the JH7110 SoC, I dropped the support for it from Emil's > variant of the stmmac glue layer. Hence, we might need a bit of coordination > in order to get this properly merged. To be honest, that one is the least of your worries sequencing wise. Anything doing non-coherent DMA on RISC-V that doesn't use instructions is dependant on Prabhakar's series: https://lore.kernel.org/linux-riscv/20230106185526.260163-1-prabhakar.mahadev-lad.rj@bp.renesas.com/#t That's kinda stalled out though, waiting on Arnd to make some changes to the cross-arch DMA code: https://lore.kernel.org/linux-riscv/ea4cb121-97e9-4365-861a-b3635fd34721@app.fastmail.com/ I was talking to Emil about the non-coherent support at FOSDEM actually, and I see no real reason not to bring the JH7100 non-coherent support in if we are doing it for other SoCs. So yeah, hopefully we shall get there at some point soonTM... Sorry, Conor. > [1] https://github.com/starfive-tech/linux/commits/visionfive > [2] https://lore.kernel.org/linux-riscv/20230118061701.30047-6-yanhong.wang@starfivetech.com/ > > Cristian Ciocaltea (7): > dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 > SoC > dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property > dt-bindings: net: Add StarFive JH7100 SoC > riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC > riscv: dts: starfive: jh7100: Add ccache DT node > riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes > riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac > > Emil Renner Berthing (5): > soc: sifive: ccache: Add StarFive JH7100 support > soc: sifive: ccache: Add non-coherent DMA handling > riscv: Implement non-coherent DMA support via SiFive cache flushing > dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible > net: stmmac: Add glue layer for StarFive JH7100 SoC > > .../devicetree/bindings/mfd/syscon.yaml | 1 + > .../devicetree/bindings/net/snps,dwmac.yaml | 15 +- > .../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++ > .../bindings/riscv/sifive,ccache0.yaml | 33 +++- > MAINTAINERS | 6 + > arch/riscv/Kconfig | 6 +- > .../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++ > arch/riscv/boot/dts/starfive/jh7100.dtsi | 55 +++++++ > arch/riscv/mm/dma-noncoherent.c | 37 ++++- > drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++ > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++ > drivers/soc/sifive/Kconfig | 1 + > drivers/soc/sifive/sifive_ccache.c | 71 +++++++- > include/soc/sifive/sifive_ccache.h | 21 +++ > 15 files changed, 587 insertions(+), 11 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c > > -- > 2.39.1 >
Hi Conor, On 2/11/23 13:11, Conor Dooley wrote: > Hey Cristian! > > +CC Arnd, Prabhakar > > On Sat, Feb 11, 2023 at 05:18:09AM +0200, Cristian Ciocaltea wrote: >> This patch series adds ethernet support for the StarFive JH7100 SoC and >> makes it available for the StarFive VisionFive V1 and BeagleV Starlight >> boards, although I could only validate on the former SBC. >> >> The work is heavily based on the reference implementation [1] and requires >> the non-coherent DMA support provided by Emil via the Sifive Composable >> Cache controller. >> >> Also note there is an overlap in "[PATCH 08/12] net: stmmac: Add glue layer >> for StarFive JH7100 SoC" with the Yanhong Wang's upstreaming attempt [2]: >> "[PATCH v4 5/7] net: stmmac: Add glue layer for StarFive JH7110 SoCs". >> >> Since I cannot test the JH7110 SoC, I dropped the support for it from Emil's >> variant of the stmmac glue layer. Hence, we might need a bit of coordination >> in order to get this properly merged. > > To be honest, that one is the least of your worries sequencing wise. > Anything doing non-coherent DMA on RISC-V that doesn't use instructions is > dependant on Prabhakar's series: > https://lore.kernel.org/linux-riscv/20230106185526.260163-1-prabhakar.mahadev-lad.rj@bp.renesas.com/#t > That's kinda stalled out though, waiting on Arnd to make some changes to > the cross-arch DMA code: > https://lore.kernel.org/linux-riscv/ea4cb121-97e9-4365-861a-b3635fd34721@app.fastmail.com/ Thank you for pointing this out, I wasn't aware of it! > I was talking to Emil about the non-coherent support at FOSDEM actually, > and I see no real reason not to bring the JH7100 non-coherent support in > if we are doing it for other SoCs. > > So yeah, hopefully we shall get there at some point soonTM... That would be great, I'll try to monitor the progress and re-spin the series as soon as the non-coherent support is figured out. Regards, Cristian > Sorry, > Conor. >> [1] https://github.com/starfive-tech/linux/commits/visionfive >> [2] https://lore.kernel.org/linux-riscv/20230118061701.30047-6-yanhong.wang@starfivetech.com/ >> >> Cristian Ciocaltea (7): >> dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 >> SoC >> dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property >> dt-bindings: net: Add StarFive JH7100 SoC >> riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC >> riscv: dts: starfive: jh7100: Add ccache DT node >> riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes >> riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac >> >> Emil Renner Berthing (5): >> soc: sifive: ccache: Add StarFive JH7100 support >> soc: sifive: ccache: Add non-coherent DMA handling >> riscv: Implement non-coherent DMA support via SiFive cache flushing >> dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible >> net: stmmac: Add glue layer for StarFive JH7100 SoC >> >> .../devicetree/bindings/mfd/syscon.yaml | 1 + >> .../devicetree/bindings/net/snps,dwmac.yaml | 15 +- >> .../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++ >> .../bindings/riscv/sifive,ccache0.yaml | 33 +++- >> MAINTAINERS | 6 + >> arch/riscv/Kconfig | 6 +- >> .../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++ >> arch/riscv/boot/dts/starfive/jh7100.dtsi | 55 +++++++ >> arch/riscv/mm/dma-noncoherent.c | 37 ++++- >> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++ >> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + >> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++ >> drivers/soc/sifive/Kconfig | 1 + >> drivers/soc/sifive/sifive_ccache.c | 71 +++++++- >> include/soc/sifive/sifive_ccache.h | 21 +++ >> 15 files changed, 587 insertions(+), 11 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml >> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c >> >> -- >> 2.39.1 >>
> + > +#define JH7100_SYSMAIN_REGISTER28 0x70 > +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */ > +#define JH7100_SYSMAIN_REGISTER49 0xc8 Seems like the comment should be one line earlier? There is value in basing the names on the datasheet, but you could append something meaningful on the end: #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8 ??? > + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", >xclk_dlychain)) { > + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain); > + if (ret) > + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n"); > + } You should probably document that if starfive,gtxclk-dlychain is not found in the DT blob, the value for the delay chain is undefined. It would actually be better to define it, set it to 0 for example. That way, you know you don't have any dependency on the bootloader for example. Andrew
On 11/02/2023 03:18, Cristian Ciocaltea wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > This variant is used on the StarFive JH7100 SoC. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > arch/riscv/Kconfig | 6 ++++-- > arch/riscv/mm/dma-noncoherent.c | 37 +++++++++++++++++++++++++++++++-- > 2 files changed, 39 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 9c687da7756d..05f6c77faf6f 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -232,12 +232,14 @@ config LOCKDEP_SUPPORT > def_bool y > > config RISCV_DMA_NONCOHERENT > - bool > + bool "Support non-coherent DMA" > + default SOC_STARFIVE > select ARCH_HAS_DMA_PREP_COHERENT > + select ARCH_HAS_DMA_SET_UNCACHED > + select ARCH_HAS_DMA_CLEAR_UNCACHED > select ARCH_HAS_SYNC_DMA_FOR_DEVICE > select ARCH_HAS_SYNC_DMA_FOR_CPU > select ARCH_HAS_SETUP_DMA_OPS > - select DMA_DIRECT_REMAP > > config AS_HAS_INSN > def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) t0$(comma) t0$(comma) zero) > diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c > index d919efab6eba..e07e53aea537 100644 > --- a/arch/riscv/mm/dma-noncoherent.c > +++ b/arch/riscv/mm/dma-noncoherent.c > @@ -9,14 +9,21 @@ > #include <linux/dma-map-ops.h> > #include <linux/mm.h> > #include <asm/cacheflush.h> > +#include <soc/sifive/sifive_ccache.h> > > static bool noncoherent_supported; > > void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, > enum dma_data_direction dir) > { > - void *vaddr = phys_to_virt(paddr); > + void *vaddr; > > + if (sifive_ccache_handle_noncoherent()) { > + sifive_ccache_flush_range(paddr, size); > + return; > + } > + > + vaddr = phys_to_virt(paddr); > switch (dir) { > case DMA_TO_DEVICE: > ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size); > @@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, > void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, > enum dma_data_direction dir) > { > - void *vaddr = phys_to_virt(paddr); > + void *vaddr; > + > + if (sifive_ccache_handle_noncoherent()) { > + sifive_ccache_flush_range(paddr, size); > + return; > + } ok, what happens if we have an system where the ccache and another level of cache also requires maintenance operations? > > + vaddr = phys_to_virt(paddr); > switch (dir) { > case DMA_TO_DEVICE: > break; > @@ -49,10 +62,30 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, > } > } > > +void *arch_dma_set_uncached(void *addr, size_t size) > +{ > + if (sifive_ccache_handle_noncoherent()) > + return sifive_ccache_set_uncached(addr, size); > + > + return addr; > +} > + > +void arch_dma_clear_uncached(void *addr, size_t size) > +{ > + if (sifive_ccache_handle_noncoherent()) > + sifive_ccache_clear_uncached(addr, size); > +} > + > void arch_dma_prep_coherent(struct page *page, size_t size) > { > void *flush_addr = page_address(page); > > + if (sifive_ccache_handle_noncoherent()) { > + memset(flush_addr, 0, size); > + sifive_ccache_flush_range(__pa(flush_addr), size); > + return; > + } > + > ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size); > } >
On 11/02/2023 04:18, Cristian Ciocaltea wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > This adds a glue layer for the Synopsys DesignWare MAC IP core on the > StarFive JH7100 SoC. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > [drop references to JH7110, update JH7100 compatible string] > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > MAINTAINERS | 1 + > drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++ > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + > .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++ > 4 files changed, 169 insertions(+) > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c > > diff --git a/MAINTAINERS b/MAINTAINERS > index d48468b81b94..defedaff6041 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -19820,6 +19820,7 @@ STARFIVE DWMAC GLUE LAYER > M: Emil Renner Berthing <kernel@esmil.dk> > S: Maintained > F: Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml > +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c > > STARFIVE JH7100 CLOCK DRIVERS > M: Emil Renner Berthing <kernel@esmil.dk> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig > index f77511fe4e87..2c81aa594291 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig > @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA > for the stmmac device driver. This driver is used for > arria5 and cyclone5 FPGA SoCs. > > +config DWMAC_STARFIVE > + tristate "StarFive DWMAC support" Bring only one driver. https://lore.kernel.org/all/20230118061701.30047-6-yanhong.wang@starfivetech.com/ Best regards, Krzysztof
On 11/02/2023 04:18, Cristian Ciocaltea wrote: > Provide the sysmain and gmac DT nodes supporting the DWMAC found on the > StarFive JH7100 SoC. > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > arch/riscv/boot/dts/starfive/jh7100.dtsi | 38 ++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi > index 88f91bc5753b..0918af7b6eb0 100644 > --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi > @@ -164,6 +164,44 @@ rstgen: reset-controller@11840000 { > #reset-cells = <1>; > }; > > + sysmain: syscon@11850000 { > + compatible = "starfive,jh7100-sysmain", "syscon"; > + reg = <0x0 0x11850000 0x0 0x10000>; > + }; > + > + gmac: ethernet@10020000 { Aren't the nodes ordered by address? Best regards, Krzysztof
On 2/13/23 10:30, Ben Dooks wrote: > On 11/02/2023 03:18, Cristian Ciocaltea wrote: >> From: Emil Renner Berthing <kernel@esmil.dk> >> >> This variant is used on the StarFive JH7100 SoC. >> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> arch/riscv/Kconfig | 6 ++++-- >> arch/riscv/mm/dma-noncoherent.c | 37 +++++++++++++++++++++++++++++++-- >> 2 files changed, 39 insertions(+), 4 deletions(-) >> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig >> index 9c687da7756d..05f6c77faf6f 100644 >> --- a/arch/riscv/Kconfig >> +++ b/arch/riscv/Kconfig >> @@ -232,12 +232,14 @@ config LOCKDEP_SUPPORT >> def_bool y >> config RISCV_DMA_NONCOHERENT >> - bool >> + bool "Support non-coherent DMA" >> + default SOC_STARFIVE >> select ARCH_HAS_DMA_PREP_COHERENT >> + select ARCH_HAS_DMA_SET_UNCACHED >> + select ARCH_HAS_DMA_CLEAR_UNCACHED >> select ARCH_HAS_SYNC_DMA_FOR_DEVICE >> select ARCH_HAS_SYNC_DMA_FOR_CPU >> select ARCH_HAS_SETUP_DMA_OPS >> - select DMA_DIRECT_REMAP >> config AS_HAS_INSN >> def_bool $(as-instr,.insn r 51$(comma) 0$(comma) 0$(comma) >> t0$(comma) t0$(comma) zero) >> diff --git a/arch/riscv/mm/dma-noncoherent.c >> b/arch/riscv/mm/dma-noncoherent.c >> index d919efab6eba..e07e53aea537 100644 >> --- a/arch/riscv/mm/dma-noncoherent.c >> +++ b/arch/riscv/mm/dma-noncoherent.c >> @@ -9,14 +9,21 @@ >> #include <linux/dma-map-ops.h> >> #include <linux/mm.h> >> #include <asm/cacheflush.h> >> +#include <soc/sifive/sifive_ccache.h> >> static bool noncoherent_supported; >> void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, >> enum dma_data_direction dir) >> { >> - void *vaddr = phys_to_virt(paddr); >> + void *vaddr; >> + if (sifive_ccache_handle_noncoherent()) { >> + sifive_ccache_flush_range(paddr, size); >> + return; >> + } >> + >> + vaddr = phys_to_virt(paddr); >> switch (dir) { >> case DMA_TO_DEVICE: >> ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size); >> @@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr, >> size_t size, >> void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, >> enum dma_data_direction dir) >> { >> - void *vaddr = phys_to_virt(paddr); >> + void *vaddr; >> + >> + if (sifive_ccache_handle_noncoherent()) { >> + sifive_ccache_flush_range(paddr, size); >> + return; >> + } > > ok, what happens if we have an system where the ccache and another level > of cache also requires maintenance operations? According to [1], the handling of non-coherent DMA on RISC-V is currently being worked on, so I will respin the series as soon as the proper support arrives. [1] https://lore.kernel.org/lkml/Y+d36nz0xdfXmDI1@spud/ >> + vaddr = phys_to_virt(paddr); >> switch (dir) { >> case DMA_TO_DEVICE: >> break; >> @@ -49,10 +62,30 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, >> size_t size, >> } >> } >> +void *arch_dma_set_uncached(void *addr, size_t size) >> +{ >> + if (sifive_ccache_handle_noncoherent()) >> + return sifive_ccache_set_uncached(addr, size); >> + >> + return addr; >> +} >> + >> +void arch_dma_clear_uncached(void *addr, size_t size) >> +{ >> + if (sifive_ccache_handle_noncoherent()) >> + sifive_ccache_clear_uncached(addr, size); >> +} >> + >> void arch_dma_prep_coherent(struct page *page, size_t size) >> { >> void *flush_addr = page_address(page); >> + if (sifive_ccache_handle_noncoherent()) { >> + memset(flush_addr, 0, size); >> + sifive_ccache_flush_range(__pa(flush_addr), size); >> + return; >> + } >> + >> ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size); >> } >
On 2/13/23 11:26, Krzysztof Kozlowski wrote: > On 11/02/2023 04:18, Cristian Ciocaltea wrote: >> From: Emil Renner Berthing <kernel@esmil.dk> >> >> This adds a glue layer for the Synopsys DesignWare MAC IP core on the >> StarFive JH7100 SoC. >> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> [drop references to JH7110, update JH7100 compatible string] >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> MAINTAINERS | 1 + >> drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++ >> drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + >> .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++ >> 4 files changed, 169 insertions(+) >> create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index d48468b81b94..defedaff6041 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -19820,6 +19820,7 @@ STARFIVE DWMAC GLUE LAYER >> M: Emil Renner Berthing <kernel@esmil.dk> >> S: Maintained >> F: Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml >> +F: drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c >> >> STARFIVE JH7100 CLOCK DRIVERS >> M: Emil Renner Berthing <kernel@esmil.dk> >> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig >> index f77511fe4e87..2c81aa594291 100644 >> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig >> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig >> @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA >> for the stmmac device driver. This driver is used for >> arria5 and cyclone5 FPGA SoCs. >> >> +config DWMAC_STARFIVE >> + tristate "StarFive DWMAC support" > > Bring only one driver. > > https://lore.kernel.org/all/20230118061701.30047-6-yanhong.wang@starfivetech.com/ Already mentioned in the cover letter that we have this overlap (will be merged into a single driver). Thanks, Cristian > > Best regards, > Krzysztof > >
On 2/13/23 11:26, Krzysztof Kozlowski wrote: > On 11/02/2023 04:18, Cristian Ciocaltea wrote: >> Provide the sysmain and gmac DT nodes supporting the DWMAC found on the >> StarFive JH7100 SoC. >> >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> >> --- >> arch/riscv/boot/dts/starfive/jh7100.dtsi | 38 ++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi >> index 88f91bc5753b..0918af7b6eb0 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7100.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi >> @@ -164,6 +164,44 @@ rstgen: reset-controller@11840000 { >> #reset-cells = <1>; >> }; >> >> + sysmain: syscon@11850000 { >> + compatible = "starfive,jh7100-sysmain", "syscon"; >> + reg = <0x0 0x11850000 0x0 0x10000>; >> + }; >> + >> + gmac: ethernet@10020000 { > > Aren't the nodes ordered by address? Right, I missed the ordering trying to keep the related nodes together. Will fix. Thanks, Cristian > Best regards, > Krzysztof > >
On Tue, Feb 14, 2023 at 08:06:49PM +0200, Cristian Ciocaltea wrote: > On 2/13/23 10:30, Ben Dooks wrote: > > On 11/02/2023 03:18, Cristian Ciocaltea wrote: > > > From: Emil Renner Berthing <kernel@esmil.dk> > > > diff --git a/arch/riscv/mm/dma-noncoherent.c > > > b/arch/riscv/mm/dma-noncoherent.c > > > index d919efab6eba..e07e53aea537 100644 > > > --- a/arch/riscv/mm/dma-noncoherent.c > > > +++ b/arch/riscv/mm/dma-noncoherent.c > > > @@ -9,14 +9,21 @@ > > > #include <linux/dma-map-ops.h> > > > #include <linux/mm.h> > > > #include <asm/cacheflush.h> > > > +#include <soc/sifive/sifive_ccache.h> > > > static bool noncoherent_supported; > > > void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, > > > enum dma_data_direction dir) > > > { > > > - void *vaddr = phys_to_virt(paddr); > > > + void *vaddr; > > > + if (sifive_ccache_handle_noncoherent()) { > > > + sifive_ccache_flush_range(paddr, size); > > > + return; > > > + } > > > + > > > + vaddr = phys_to_virt(paddr); > > > switch (dir) { > > > case DMA_TO_DEVICE: > > > ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size); > > > @@ -35,8 +42,14 @@ void arch_sync_dma_for_device(phys_addr_t paddr, > > > size_t size, > > > void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, > > > enum dma_data_direction dir) > > > { > > > - void *vaddr = phys_to_virt(paddr); > > > + void *vaddr; > > > + > > > + if (sifive_ccache_handle_noncoherent()) { > > > + sifive_ccache_flush_range(paddr, size); > > > + return; > > > + } > > > > ok, what happens if we have an system where the ccache and another level > > of cache also requires maintenance operations? TBH, I'd hope that a system with that complexity is also not trying to manage the cache in this manner! > According to [1], the handling of non-coherent DMA on RISC-V is currently > being worked on, so I will respin the series as soon as the proper support > arrives. But yeah, once that stuff lands we can carry out these operations only for the platforms that need/"need" it. Cheers, Conor.
On 2/11/23 18:11, Andrew Lunn wrote: >> + >> +#define JH7100_SYSMAIN_REGISTER28 0x70 >> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */ >> +#define JH7100_SYSMAIN_REGISTER49 0xc8 > > Seems like the comment should be one line earlier? > > There is value in basing the names on the datasheet, but you could > append something meaningful on the end: > > #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8 > > ??? Unfortunately the JH7100 datasheet I have access to doesn't provide any information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil could provide some details here? >> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", >xclk_dlychain)) { >> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain); >> + if (ret) >> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n"); >> + } > > You should probably document that if starfive,gtxclk-dlychain is not > found in the DT blob, the value for the delay chain is undefined. It > would actually be better to define it, set it to 0 for example. That > way, you know you don't have any dependency on the bootloader for > example. Sure, I will set it to 0. > > Andrew Thanks for reviewing, Cristian
On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea <cristian.ciocaltea@collabora.com> wrote: > > On 2/11/23 18:11, Andrew Lunn wrote: > >> + > >> +#define JH7100_SYSMAIN_REGISTER28 0x70 > >> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */ > >> +#define JH7100_SYSMAIN_REGISTER49 0xc8 > > > > Seems like the comment should be one line earlier? Well yes, the very generic register names are also bad, but this comment refers to the fact that it kind of makes sense that register 28 has the offset 28 * 4 bytes pr. register = 0x70 ..but then register 49 is oddly out of place at offset 0xc8 instead of 49 * 4 bytes pr. register = 0xc4 > > There is value in basing the names on the datasheet, but you could > > append something meaningful on the end: > > > > #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8 > > Unfortunately the JH7100 datasheet I have access to doesn't provide any > information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil > could provide some details here? This is reverse engineered from the auto generated headers in their u-boot: https://github.com/starfive-tech/u-boot/blob/JH7100_VisionFive_devel/arch/riscv/include/asm/arch-jh7100/syscon_sysmain_ctrl_macro.h Christian, I'm happy that you're working on this, but mess like this and waiting for the non-coherent dma to be sorted is why I didn't send it upstream yet. > >> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", >xclk_dlychain)) { > >> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain); > >> + if (ret) > >> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n"); > >> + } > > > > You should probably document that if starfive,gtxclk-dlychain is not > > found in the DT blob, the value for the delay chain is undefined. It > > would actually be better to define it, set it to 0 for example. That > > way, you know you don't have any dependency on the bootloader for > > example. > > Sure, I will set it to 0. > > > > > Andrew > > Thanks for reviewing, > Cristian > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On 2/15/23 13:20, Emil Renner Berthing wrote: > On Wed, 15 Feb 2023 at 01:09, Cristian Ciocaltea > <cristian.ciocaltea@collabora.com> wrote: >> >> On 2/11/23 18:11, Andrew Lunn wrote: >>>> + >>>> +#define JH7100_SYSMAIN_REGISTER28 0x70 >>>> +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */ >>>> +#define JH7100_SYSMAIN_REGISTER49 0xc8 >>> >>> Seems like the comment should be one line earlier? > > Well yes, the very generic register names are also bad, but this > comment refers to the fact that it kind of makes sense that register > 28 has the offset > 28 * 4 bytes pr. register = 0x70 > ..but then register 49 is oddly out of place at offset 0xc8 instead of > 49 * 4 bytes pr. register = 0xc4 > >>> There is value in basing the names on the datasheet, but you could >>> append something meaningful on the end: >>> >>> #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8 >> >> Unfortunately the JH7100 datasheet I have access to doesn't provide any >> information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil >> could provide some details here? > > This is reverse engineered from the auto generated headers in their u-boot: > https://github.com/starfive-tech/u-boot/blob/JH7100_VisionFive_devel/arch/riscv/include/asm/arch-jh7100/syscon_sysmain_ctrl_macro.h > > Christian, I'm happy that you're working on this, but mess like this > and waiting for the non-coherent dma to be sorted is why I didn't send > it upstream yet. Thank you for clarifying this and for all the work you have done so far, Emil! If you don't mind, I would be glad to continue helping with this mainlining effort. >>>> + if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", >xclk_dlychain)) { >>>> + ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain); >>>> + if (ret) >>>> + return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n"); >>>> + } >>> >>> You should probably document that if starfive,gtxclk-dlychain is not >>> found in the DT blob, the value for the delay chain is undefined. It >>> would actually be better to define it, set it to 0 for example. That >>> way, you know you don't have any dependency on the bootloader for >>> example. >> >> Sure, I will set it to 0. >> >>> >>> Andrew >> >> Thanks for reviewing, >> Cristian >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Feb 15, 2023 at 02:08:01AM +0200, Cristian Ciocaltea wrote: > On 2/11/23 18:11, Andrew Lunn wrote: > > > + > > > +#define JH7100_SYSMAIN_REGISTER28 0x70 > > > +/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */ > > > +#define JH7100_SYSMAIN_REGISTER49 0xc8 > > > > Seems like the comment should be one line earlier? > > > > There is value in basing the names on the datasheet, but you could > > append something meaningful on the end: > > > > #define JH7100_SYSMAIN_REGISTER49_DLYCHAIN 0xc8 > > > > ??? > > Unfortunately the JH7100 datasheet I have access to doesn't provide any > information regarding the SYSCTRL-MAINSYS related registers. Maybe Emil > could provide some details here? If you have no reliable source of naming, just make a name up from how the register is used. This is why i suggested adding _DLYCHAIN, because that is what is written to it. You should be able to do the same with register 28. Andrew
Emil, +CC Daire On Sat, Feb 11, 2023 at 05:18:13AM +0200, Cristian Ciocaltea wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > Add functions to flush the caches and handle non-coherent DMA. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > [replace <asm/cacheflush.h> with <linux/cacheflush.h>] > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > --- > +void *sifive_ccache_set_uncached(void *addr, size_t size) > +{ > + phys_addr_t phys_addr = __pa(addr) + uncached_offset; > + void *mem_base; > + > + mem_base = memremap(phys_addr, size, MEMREMAP_WT); > + if (!mem_base) { > + pr_err("%s memremap failed for addr %p\n", __func__, addr); > + return ERR_PTR(-EINVAL); > + } > + > + return mem_base; > +} The rest of this I either get b/c we did it, or will become moot so I amn't worried about it, but can you please explain this, in particular the memremap that you're doing here? Cheers, Conor.
On Thu, 16 Feb 2023 at 19:51, Conor Dooley <conor@kernel.org> wrote: > > Emil, > > +CC Daire > > On Sat, Feb 11, 2023 at 05:18:13AM +0200, Cristian Ciocaltea wrote: > > From: Emil Renner Berthing <kernel@esmil.dk> > > > > Add functions to flush the caches and handle non-coherent DMA. > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > [replace <asm/cacheflush.h> with <linux/cacheflush.h>] > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > > --- > > > +void *sifive_ccache_set_uncached(void *addr, size_t size) > > +{ > > + phys_addr_t phys_addr = __pa(addr) + uncached_offset; > > + void *mem_base; > > + > > + mem_base = memremap(phys_addr, size, MEMREMAP_WT); > > + if (!mem_base) { > > + pr_err("%s memremap failed for addr %p\n", __func__, addr); > > + return ERR_PTR(-EINVAL); > > + } > > + > > + return mem_base; > > +} > > The rest of this I either get b/c we did it, or will become moot so I > amn't worried about it, but can you please explain this, in particular > the memremap that you're doing here? No, I can't really. As we talked about it's also based on a prototype by Atish. I'm sure you know that the general idea is that we want to return a pointer that accesses the same physical memory, but through the uncached alias. I can't tell you exactly why it's done this way though, sorry. /Emil > Cheers, > Conor. > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Sun, Feb 19, 2023 at 10:32:52PM +0100, Emil Renner Berthing wrote: > On Thu, 16 Feb 2023 at 19:51, Conor Dooley <conor@kernel.org> wrote: > > > > Emil, > > > > +CC Daire > > > > On Sat, Feb 11, 2023 at 05:18:13AM +0200, Cristian Ciocaltea wrote: > > > From: Emil Renner Berthing <kernel@esmil.dk> > > > > > > Add functions to flush the caches and handle non-coherent DMA. > > > > > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > > > [replace <asm/cacheflush.h> with <linux/cacheflush.h>] > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > > > --- > > > > > +void *sifive_ccache_set_uncached(void *addr, size_t size) > > > +{ > > > + phys_addr_t phys_addr = __pa(addr) + uncached_offset; > > > + void *mem_base; > > > + > > > + mem_base = memremap(phys_addr, size, MEMREMAP_WT); > > > + if (!mem_base) { > > > + pr_err("%s memremap failed for addr %p\n", __func__, addr); > > > + return ERR_PTR(-EINVAL); > > > + } > > > + > > > + return mem_base; > > > +} > > > > The rest of this I either get b/c we did it, or will become moot so I > > amn't worried about it, but can you please explain this, in particular > > the memremap that you're doing here? > > No, I can't really. As we talked about it's also based on a prototype > by Atish. I'm sure you know that the general idea is that we want to > return a pointer that accesses the same physical memory, but through > the uncached alias. Yah, I follow all the rest of what's going on - it's just this bit of it that I don't. > I can't tell you exactly why it's done this way > though, sorry. I had a bit of a look on lore, but don't really see anything there that contained any discussion of what was going on here. Adding Atish in the off-chance that he remembers!
On Sat, Feb 11, 2023 at 05:18:12AM +0200, Cristian Ciocaltea wrote: > From: Emil Renner Berthing <kernel@esmil.dk> > > This adds support for the StarFive JH7100 SoC which also feature this > SiFive cache controller. > > Unfortunately the interrupt for uncorrected data is broken on the JH7100 > and fires continuously, so add a quirk to not register a handler for it. > > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> > [drop JH7110, rework Kconfig] > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> This driver doesn't really do very much of anything as things stand, so I don't see really see all that much value in picking it up right now, since the non-coherent bits aren't usable yet. > --- > drivers/soc/sifive/Kconfig | 1 + > drivers/soc/sifive/sifive_ccache.c | 11 ++++++++++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig > index e86870be34c9..867cf16273a4 100644 > --- a/drivers/soc/sifive/Kconfig > +++ b/drivers/soc/sifive/Kconfig > @@ -4,6 +4,7 @@ if SOC_SIFIVE || SOC_STARFIVE > > config SIFIVE_CCACHE > bool "Sifive Composable Cache controller" > + default SOC_STARFIVE I don't think this should have a default set w/ the support that this patch brings in. Perhaps later we should be doing defaulting, but not at this point in the series. Other than that, this is fine by me: Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks, Conor.
On 3/7/23 01:32, Conor Dooley wrote: > On Sat, Feb 11, 2023 at 05:18:12AM +0200, Cristian Ciocaltea wrote: >> From: Emil Renner Berthing <kernel@esmil.dk> >> >> This adds support for the StarFive JH7100 SoC which also feature this >> SiFive cache controller. >> >> Unfortunately the interrupt for uncorrected data is broken on the JH7100 >> and fires continuously, so add a quirk to not register a handler for it. >> >> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk> >> [drop JH7110, rework Kconfig] >> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com> > > This driver doesn't really do very much of anything as things stand, so > I don't see really see all that much value in picking it up right now, > since the non-coherent bits aren't usable yet. > >> --- >> drivers/soc/sifive/Kconfig | 1 + >> drivers/soc/sifive/sifive_ccache.c | 11 ++++++++++- >> 2 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/soc/sifive/Kconfig b/drivers/soc/sifive/Kconfig >> index e86870be34c9..867cf16273a4 100644 >> --- a/drivers/soc/sifive/Kconfig >> +++ b/drivers/soc/sifive/Kconfig >> @@ -4,6 +4,7 @@ if SOC_SIFIVE || SOC_STARFIVE >> >> config SIFIVE_CCACHE >> bool "Sifive Composable Cache controller" >> + default SOC_STARFIVE > > I don't think this should have a default set w/ the support that this > patch brings in. Perhaps later we should be doing defaulting, but not at > this point in the series. I will handle this is v2 as soon as the non-coherency stuff is ready. > Other than that, this is fine by me: > Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Thanks for reviewing, Cristian > Thanks, > Conor.
This patch series adds ethernet support for the StarFive JH7100 SoC and makes it available for the StarFive VisionFive V1 and BeagleV Starlight boards, although I could only validate on the former SBC. The work is heavily based on the reference implementation [1] and requires the non-coherent DMA support provided by Emil via the Sifive Composable Cache controller. Also note there is an overlap in "[PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC" with the Yanhong Wang's upstreaming attempt [2]: "[PATCH v4 5/7] net: stmmac: Add glue layer for StarFive JH7110 SoCs". Since I cannot test the JH7110 SoC, I dropped the support for it from Emil's variant of the stmmac glue layer. Hence, we might need a bit of coordination in order to get this properly merged. [1] https://github.com/starfive-tech/linux/commits/visionfive [2] https://lore.kernel.org/linux-riscv/20230118061701.30047-6-yanhong.wang@starfivetech.com/ Cristian Ciocaltea (7): dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property dt-bindings: net: Add StarFive JH7100 SoC riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC riscv: dts: starfive: jh7100: Add ccache DT node riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac Emil Renner Berthing (5): soc: sifive: ccache: Add StarFive JH7100 support soc: sifive: ccache: Add non-coherent DMA handling riscv: Implement non-coherent DMA support via SiFive cache flushing dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible net: stmmac: Add glue layer for StarFive JH7100 SoC .../devicetree/bindings/mfd/syscon.yaml | 1 + .../devicetree/bindings/net/snps,dwmac.yaml | 15 +- .../bindings/net/starfive,jh7100-dwmac.yaml | 106 ++++++++++++ .../bindings/riscv/sifive,ccache0.yaml | 33 +++- MAINTAINERS | 6 + arch/riscv/Kconfig | 6 +- .../boot/dts/starfive/jh7100-common.dtsi | 78 +++++++++ arch/riscv/boot/dts/starfive/jh7100.dtsi | 55 +++++++ arch/riscv/mm/dma-noncoherent.c | 37 ++++- drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++ drivers/net/ethernet/stmicro/stmmac/Makefile | 1 + .../ethernet/stmicro/stmmac/dwmac-starfive.c | 155 ++++++++++++++++++ drivers/soc/sifive/Kconfig | 1 + drivers/soc/sifive/sifive_ccache.c | 71 +++++++- include/soc/sifive/sifive_ccache.h | 21 +++ 15 files changed, 587 insertions(+), 11 deletions(-) create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c