[PATCH 00/12] Enable networking support for StarFive JH7100 SoC

[PATCH 00/12] Enable networking support for StarFive JH7100 SoC

[Cristian Ciocaltea]

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

-- 
2.39.1

[PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

[Cristian Ciocaltea]

Document the compatible for the SiFive Composable Cache Controller found
on the StarFive JH7100 SoC.

This also requires extending the 'reg' property to handle distinct
ranges, as specified via 'reg-names'.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../bindings/riscv/sifive,ccache0.yaml        | 28 ++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
index 31d20efaa6d3..2b864b2f12c9 100644
--- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
+++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
@@ -25,6 +25,7 @@ select:
           - sifive,ccache0
           - sifive,fu540-c000-ccache
           - sifive,fu740-c000-ccache
+          - starfive,jh7100-ccache
 
   required:
     - compatible
@@ -37,6 +38,7 @@ properties:
               - sifive,ccache0
               - sifive,fu540-c000-ccache
               - sifive,fu740-c000-ccache
+              - starfive,jh7100-ccache
           - const: cache
       - items:
           - const: starfive,jh7110-ccache
@@ -70,7 +72,13 @@ properties:
       - description: DirFail interrupt
 
   reg:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2
+
+  reg-names:
+    items:
+      - const: control
+      - const: sideband
 
   next-level-cache: true
 
@@ -89,6 +97,7 @@ allOf:
           contains:
             enum:
               - sifive,fu740-c000-ccache
+              - starfive,jh7100-ccache
               - starfive,jh7110-ccache
               - microchip,mpfs-ccache
 
@@ -106,12 +115,29 @@ allOf:
             Must contain entries for DirError, DataError and DataFail signals.
           maxItems: 3
 
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: starfive,jh7100-ccache
+
+    then:
+      properties:
+        reg:
+          maxItems: 2
+
+    else:
+      properties:
+        reg:
+          maxItems: 1
+
   - if:
       properties:
         compatible:
           contains:
             enum:
               - sifive,fu740-c000-ccache
+              - starfive,jh7100-ccache
               - starfive,jh7110-ccache
 
     then:
-- 
2.39.1

Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

[Krzysztof Kozlowski]

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Document the compatible for the SiFive Composable Cache Controller found
> on the StarFive JH7100 SoC.
> 
> This also requires extending the 'reg' property to handle distinct
> ranges, as specified via 'reg-names'.


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]

Hey all,

On Sat, Feb 11, 2023 at 05:18:10AM +0200, Cristian Ciocaltea wrote:
> Document the compatible for the SiFive Composable Cache Controller found
> on the StarFive JH7100 SoC.
> 
> This also requires extending the 'reg' property to handle distinct
> ranges, as specified via 'reg-names'.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../bindings/riscv/sifive,ccache0.yaml        | 28 ++++++++++++++++++-
>  1 file changed, 27 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> index 31d20efaa6d3..2b864b2f12c9 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> @@ -25,6 +25,7 @@ select:
>            - sifive,ccache0
>            - sifive,fu540-c000-ccache
>            - sifive,fu740-c000-ccache
> +          - starfive,jh7100-ccache
>  
>    required:
>      - compatible
> @@ -37,6 +38,7 @@ properties:
>                - sifive,ccache0
>                - sifive,fu540-c000-ccache
>                - sifive,fu740-c000-ccache
> +              - starfive,jh7100-ccache
>            - const: cache
>        - items:
>            - const: starfive,jh7110-ccache
> @@ -70,7 +72,13 @@ properties:
>        - description: DirFail interrupt
>  
>    reg:
> -    maxItems: 1
> +    minItems: 1
> +    maxItems: 2
> +
> +  reg-names:
> +    items:
> +      - const: control
> +      - const: sideband

So why is this called "sideband"?
In the docs for the JH7100 it is called LIM & it's called LIM in our
docs for the PolarFire SoC (at the same address btw) and we run the HSS
out of it! LIM being "loosely integrated memory", which by the limit
hits on Google may be a SiFive-ism?

I'm not really sure if adding it as a "reg" section is the right thing
to do as it's not "just" a register bank.
Perhaps Rob/Krzysztof have a take on that one?

>  
>    next-level-cache: true
>  
> @@ -89,6 +97,7 @@ allOf:
>            contains:
>              enum:
>                - sifive,fu740-c000-ccache
> +              - starfive,jh7100-ccache
>                - starfive,jh7110-ccache
>                - microchip,mpfs-ccache
>  
> @@ -106,12 +115,29 @@ allOf:
>              Must contain entries for DirError, DataError and DataFail signals.
>            maxItems: 3
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: starfive,jh7100-ccache
> +
> +    then:
> +      properties:
> +        reg:
> +          maxItems: 2
> +
> +    else:
> +      properties:
> +        reg:
> +          maxItems: 1
> +
>    - if:
>        properties:
>          compatible:
>            contains:
>              enum:
>                - sifive,fu740-c000-ccache
> +              - starfive,jh7100-ccache
>                - starfive,jh7110-ccache
>  
>      then:
> -- 
> 2.39.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

[Emil Renner Berthing]

On Tue, 14 Feb 2023 at 21:42, Conor Dooley <conor@kernel.org> wrote:
>
> Hey all,
>
> On Sat, Feb 11, 2023 at 05:18:10AM +0200, Cristian Ciocaltea wrote:
> > Document the compatible for the SiFive Composable Cache Controller found
> > on the StarFive JH7100 SoC.
> >
> > This also requires extending the 'reg' property to handle distinct
> > ranges, as specified via 'reg-names'.
> >
> > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > ---
> >  .../bindings/riscv/sifive,ccache0.yaml        | 28 ++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> > index 31d20efaa6d3..2b864b2f12c9 100644
> > --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> > @@ -25,6 +25,7 @@ select:
> >            - sifive,ccache0
> >            - sifive,fu540-c000-ccache
> >            - sifive,fu740-c000-ccache
> > +          - starfive,jh7100-ccache
> >
> >    required:
> >      - compatible
> > @@ -37,6 +38,7 @@ properties:
> >                - sifive,ccache0
> >                - sifive,fu540-c000-ccache
> >                - sifive,fu740-c000-ccache
> > +              - starfive,jh7100-ccache
> >            - const: cache
> >        - items:
> >            - const: starfive,jh7110-ccache
> > @@ -70,7 +72,13 @@ properties:
> >        - description: DirFail interrupt
> >
> >    reg:
> > -    maxItems: 1
> > +    minItems: 1
> > +    maxItems: 2
> > +
> > +  reg-names:
> > +    items:
> > +      - const: control
> > +      - const: sideband
>
> So why is this called "sideband"?
> In the docs for the JH7100 it is called LIM & it's called LIM in our
> docs for the PolarFire SoC (at the same address btw) and we run the HSS
> out of it! LIM being "loosely integrated memory", which by the limit
> hits on Google may be a SiFive-ism?
>
> I'm not really sure if adding it as a "reg" section is the right thing
> to do as it's not "just" a register bank.
> Perhaps Rob/Krzysztof have a take on that one?

Yes, this seems to be a leftover I didn't manage to clean up yet. The
"sideband" range is called L2 LIM in the datasheet and seems to be a
way to use the cache directly. The Sifive docs read "When cache ways
are disabled, they are addressable in the L2 Loosely-Integrated Memory
(L2 LIM) address space [..]". This feature is not used by Linux on the
JH7100, so can just be removed here.

/Emil

> >
> >    next-level-cache: true
> >
> > @@ -89,6 +97,7 @@ allOf:
> >            contains:
> >              enum:
> >                - sifive,fu740-c000-ccache
> > +              - starfive,jh7100-ccache
> >                - starfive,jh7110-ccache
> >                - microchip,mpfs-ccache
> >
> > @@ -106,12 +115,29 @@ allOf:
> >              Must contain entries for DirError, DataError and DataFail signals.
> >            maxItems: 3
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: starfive,jh7100-ccache
> > +
> > +    then:
> > +      properties:
> > +        reg:
> > +          maxItems: 2
> > +
> > +    else:
> > +      properties:
> > +        reg:
> > +          maxItems: 1
> > +
> >    - if:
> >        properties:
> >          compatible:
> >            contains:
> >              enum:
> >                - sifive,fu740-c000-ccache
> > +              - starfive,jh7100-ccache
> >                - starfive,jh7110-ccache
> >
> >      then:
> > --
> > 2.39.1
> >
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 01/12] dt-bindings: riscv: sifive-ccache: Add compatible for StarFive JH7100 SoC

[Palmer Dabbelt]

On Tue, 14 Feb 2023 12:40:35 PST (-0800), Conor Dooley wrote:
> Hey all,
>
> On Sat, Feb 11, 2023 at 05:18:10AM +0200, Cristian Ciocaltea wrote:
>> Document the compatible for the SiFive Composable Cache Controller found
>> on the StarFive JH7100 SoC.
>> 
>> This also requires extending the 'reg' property to handle distinct
>> ranges, as specified via 'reg-names'.
>> 
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  .../bindings/riscv/sifive,ccache0.yaml        | 28 ++++++++++++++++++-
>>  1 file changed, 27 insertions(+), 1 deletion(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
>> index 31d20efaa6d3..2b864b2f12c9 100644
>> --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
>> @@ -25,6 +25,7 @@ select:
>>            - sifive,ccache0
>>            - sifive,fu540-c000-ccache
>>            - sifive,fu740-c000-ccache
>> +          - starfive,jh7100-ccache
>>  
>>    required:
>>      - compatible
>> @@ -37,6 +38,7 @@ properties:
>>                - sifive,ccache0
>>                - sifive,fu540-c000-ccache
>>                - sifive,fu740-c000-ccache
>> +              - starfive,jh7100-ccache
>>            - const: cache
>>        - items:
>>            - const: starfive,jh7110-ccache
>> @@ -70,7 +72,13 @@ properties:
>>        - description: DirFail interrupt
>>  
>>    reg:
>> -    maxItems: 1
>> +    minItems: 1
>> +    maxItems: 2
>> +
>> +  reg-names:
>> +    items:
>> +      - const: control
>> +      - const: sideband
>
> So why is this called "sideband"?
> In the docs for the JH7100 it is called LIM & it's called LIM in our
> docs for the PolarFire SoC (at the same address btw) and we run the HSS

IIRC it's both: "LIM" is the memory, "sideband" is the port.  I can't 
find any proper documentation of "sideband" outside of DT and errata, 
but there's a hanful of references to it in the bootloader for the 
fu540: 
<https://github.com/sifive/freedom-u540-c000-bootloader/search?q=sideband>.

It's not really clear which is more correct here: sideband accesses are 
only useful when the cache is configured as an LIM, at least for general 
software.  IIRC the accesses to the LIM only go through the sideband 
port for the E core, but I might be wrong about that.

> out of it! LIM being "loosely integrated memory", which by the limit
> hits on Google may be a SiFive-ism?

Yep: TIM is the SiFive version of Arm's TCM (tightly coupled memory), 
and LIM is the flavor that's farther away (L2 instead of L1).

> I'm not really sure if adding it as a "reg" section is the right thing
> to do as it's not "just" a register bank.
> Perhaps Rob/Krzysztof have a take on that one?
>
>>  
>>    next-level-cache: true
>>  
>> @@ -89,6 +97,7 @@ allOf:
>>            contains:
>>              enum:
>>                - sifive,fu740-c000-ccache
>> +              - starfive,jh7100-ccache
>>                - starfive,jh7110-ccache
>>                - microchip,mpfs-ccache
>>  
>> @@ -106,12 +115,29 @@ allOf:
>>              Must contain entries for DirError, DataError and DataFail signals.
>>            maxItems: 3
>>  
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            const: starfive,jh7100-ccache
>> +
>> +    then:
>> +      properties:
>> +        reg:
>> +          maxItems: 2
>> +
>> +    else:
>> +      properties:
>> +        reg:
>> +          maxItems: 1
>> +
>>    - if:
>>        properties:
>>          compatible:
>>            contains:
>>              enum:
>>                - sifive,fu740-c000-ccache
>> +              - starfive,jh7100-ccache
>>                - starfive,jh7110-ccache
>>  
>>      then:
>> -- 
>> 2.39.1
>> 

[PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

[Cristian Ciocaltea]

Add the 'uncached-offset' property to be used for specifying the
uncached memory offset required for handling non-coherent DMA
transactions.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
index 2b864b2f12c9..60cd87a2810a 100644
--- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
+++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
@@ -82,6 +82,11 @@ properties:
 
   next-level-cache: true
 
+  uncached-offset:
+    $ref: /schemas/types.yaml#/definitions/uint64
+    description: |
+      Uncached memory offset for handling non-coherent DMA transactions.
+
   memory-region:
     maxItems: 1
     description: |
-- 
2.39.1

Re: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

[Krzysztof Kozlowski]

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Add the 'uncached-offset' property to be used for specifying the
> uncached memory offset required for handling non-coherent DMA
> transactions.

Only one offset can be non-coherent? If this is for DMA, why
dma-noncoherent cannot be used?

Best regards,
Krzysztof

Re: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

[Cristian Ciocaltea]

On 2/13/23 11:23, Krzysztof Kozlowski wrote:
> On 11/02/2023 04:18, Cristian Ciocaltea wrote:
>> Add the 'uncached-offset' property to be used for specifying the
>> uncached memory offset required for handling non-coherent DMA
>> transactions.
> 
> Only one offset can be non-coherent? If this is for DMA, why
> dma-noncoherent cannot be used?

As Conor already mentioned in [1], the handling of non-coherent DMA on 
RISC-V is currently being worked on, so I expect this patch will be dropped.

[1] https://lore.kernel.org/lkml/Y+d36nz0xdfXmDI1@spud/

Thanks for reviewing,
Cristian

> 
> Best regards,
> Krzysztof
> 

Re: [PATCH 02/12] dt-bindings: riscv: sifive-ccache: Add 'uncached-offset' property

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4079 bytes --]

Hey all,

On Sat, Feb 11, 2023 at 05:18:11AM +0200, Cristian Ciocaltea wrote:
> Add the 'uncached-offset' property to be used for specifying the
> uncached memory offset required for handling non-coherent DMA
> transactions.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> index 2b864b2f12c9..60cd87a2810a 100644
> --- a/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> +++ b/Documentation/devicetree/bindings/riscv/sifive,ccache0.yaml
> @@ -82,6 +82,11 @@ properties:
>  
>    next-level-cache: true
>  
> +  uncached-offset:
> +    $ref: /schemas/types.yaml#/definitions/uint64
> +    description: |
> +      Uncached memory offset for handling non-coherent DMA transactions.

Firstly, this pretty tied to the StarFive stuff, where there is only one
"bank" of memory that neatly maps to one bank of non-cached memory.
On PolarFire SoC, where we would also like to make use of non-coherent
DMA for some transfers using the FPGA fabric, things are a bit more
complex.
Instead of a region & a non-cached alias, we have 2 regions and 2
non-cached regions.
These regions lie at 0x8000_0000 & 0x10_0000_0000 and the non-cached
regions are at 0xc000_0000 & 0x14_0000_0000. As you can tell, one fixed
offset isn't going to work there!

The other bit of a problem is that there is no fixed concept of aliases,
as seems to be the case on the jh7100. Instead, where the regions
"point" to in physical DDR is something that is configurable at runtime.
Practically speaking, it is set by firmware very early on in boot & is
fixed from there out, but will vary between boards and FPGA fabric
configuration. Effectively that means that from the PoV of a Devicetree
it is constant, but a good bit of flexibility is going to be needed.

What we have been doing on PolarFire SoC (although mostly internally at
this point) is, rather than creating a property like uncached-offset, we
instead are using the dma-ranges properties to induce the same affect.

In an example configuration with memory at:
	reg = <0x0 0x80000000 0x0 0x4000000>;
	reg = <0x0 0x8a000000 0x0 0x8000000>;
	reg = <0x0 0xc4000000 0x0 0x6000000>;
	reg = <0x10 0x22000000 0x0 0x5e000000>;
	reg = <0x14 0x12000000 0x0 0x10000000>;

a reserved memory section then covering the non-cached region at
0x14_0000_0000:
	dma_non_cached_high: non-cached-high-buffer {
		compatible = "shared-dma-pool";
		size = <0x0 0x10000000>;
		no-map;
		linux,dma-default;
		alloc-ranges = <0x14 0x12000000 0x0 0x10000000>;
	};

and dma-ranges:
	dma-ranges = <0x14 0x0 0x0 0x80000000 0x0 0x4000000>,
		     <0x14 0x4000000 0x0 0xc4000000 0x0 0x6000000>,
		     <0x14 0xa000000 0x0 0x8a000000 0x0 0x8000000>,

In this configuration, 0x8000_0000, 0x10_0000_0000, 0xc000_0000 &
0x14_0000_0000 are all aliases of the same address.
With this setup, we're able to do non-coherent DMA to the FPGA fabric,
to the PCI for example.
The DTS does grow a bit of complexity, with reserved memory regions and
dma-ranges - but at least they're standard properties!

Emil, if you want to take a look at that it is here:
https://github.com/linux4microchip/linux linux-5.15-mchp
I think I said to you before that it was based on one of Atish's early
approaches, the one from the 5.15 development cycle IIRC since we're
using that LTS.
Obviously it'll need changes to be upstreamable so we're not wedded to
this approach. For instance, it's being controlled by a compile time
option at the moment, so that clearly needs to become runtime for
upstream (and realistically needs to be one in our vendor tree too...)

I'll try to hack that approach into the visionfive v1 soonTM and see how
it goes, but it'll not be this side of March before I have time to do
that.

Cheers,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 03/12] soc: sifive: ccache: Add StarFive JH7100 support

[Cristian Ciocaltea]

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>
---
 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
 	help
 	  Support for the composable cache controller on SiFive platforms.
 
diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
index 3684f5b40a80..676468c35859 100644
--- a/drivers/soc/sifive/sifive_ccache.c
+++ b/drivers/soc/sifive/sifive_ccache.c
@@ -106,6 +106,7 @@ static void ccache_config_read(void)
 static const struct of_device_id sifive_ccache_ids[] = {
 	{ .compatible = "sifive,fu540-c000-ccache" },
 	{ .compatible = "sifive,fu740-c000-ccache" },
+	{ .compatible = "starfive,jh7100-ccache", .data = (void *)BIT(DATA_UNCORR) },
 	{ .compatible = "sifive,ccache0" },
 	{ /* end of table */ }
 };
@@ -210,11 +211,15 @@ static int __init sifive_ccache_init(void)
 	struct device_node *np;
 	struct resource res;
 	int i, rc, intr_num;
+	const struct of_device_id *match;
+	unsigned long broken_irqs;
 
-	np = of_find_matching_node(NULL, sifive_ccache_ids);
+	np = of_find_matching_node_and_match(NULL, sifive_ccache_ids, &match);
 	if (!np)
 		return -ENODEV;
 
+	broken_irqs = (uintptr_t)match->data;
+
 	if (of_address_to_resource(np, 0, &res)) {
 		rc = -ENODEV;
 		goto err_node_put;
@@ -240,6 +245,10 @@ static int __init sifive_ccache_init(void)
 
 	for (i = 0; i < intr_num; i++) {
 		g_irq[i] = irq_of_parse_and_map(np, i);
+
+		if (broken_irqs & BIT(i))
+			continue;
+
 		rc = request_irq(g_irq[i], ccache_int_handler, 0, "ccache_ecc",
 				 NULL);
 		if (rc) {
-- 
2.39.1

Re: [PATCH 03/12] soc: sifive: ccache: Add StarFive JH7100 support

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1533 bytes --]

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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 03/12] soc: sifive: ccache: Add StarFive JH7100 support

[Cristian Ciocaltea]

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.

[PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

[Cristian Ciocaltea]

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>
---
 drivers/soc/sifive/sifive_ccache.c | 60 +++++++++++++++++++++++++++++-
 include/soc/sifive/sifive_ccache.h | 21 +++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/soc/sifive/sifive_ccache.c
index 676468c35859..0062635d845f 100644
--- a/drivers/soc/sifive/sifive_ccache.c
+++ b/drivers/soc/sifive/sifive_ccache.c
@@ -8,13 +8,16 @@
 
 #define pr_fmt(fmt) "CCACHE: " fmt
 
+#include <linux/align.h>
 #include <linux/debugfs.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/device.h>
 #include <linux/bitfield.h>
+#include <linux/cacheflush.h>
 #include <asm/cacheinfo.h>
+#include <asm/page.h>
 #include <soc/sifive/sifive_ccache.h>
 
 #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
@@ -39,10 +42,14 @@
 #define SIFIVE_CCACHE_CONFIG_SETS_MASK GENMASK_ULL(23, 16)
 #define SIFIVE_CCACHE_CONFIG_BLKS_MASK GENMASK_ULL(31, 24)
 
+#define SIFIVE_CCACHE_FLUSH64 0x200
+#define SIFIVE_CCACHE_FLUSH32 0x240
+
 #define SIFIVE_CCACHE_WAYENABLE 0x08
 #define SIFIVE_CCACHE_ECCINJECTERR 0x40
 
 #define SIFIVE_CCACHE_MAX_ECCINTR 4
+#define SIFIVE_CCACHE_LINE_SIZE 64
 
 static void __iomem *ccache_base;
 static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
@@ -125,6 +132,47 @@ int unregister_sifive_ccache_error_notifier(struct notifier_block *nb)
 }
 EXPORT_SYMBOL_GPL(unregister_sifive_ccache_error_notifier);
 
+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+static phys_addr_t uncached_offset;
+DEFINE_STATIC_KEY_FALSE(sifive_ccache_handle_noncoherent_key);
+
+void sifive_ccache_flush_range(phys_addr_t start, size_t len)
+{
+	phys_addr_t end = start + len;
+	phys_addr_t line;
+
+	if (!len)
+		return;
+
+	mb();
+	for (line = ALIGN_DOWN(start, SIFIVE_CCACHE_LINE_SIZE); line < end;
+			line += SIFIVE_CCACHE_LINE_SIZE) {
+#ifdef CONFIG_32BIT
+		writel(line >> 4, ccache_base + SIFIVE_CCACHE_FLUSH32);
+#else
+		writeq(line, ccache_base + SIFIVE_CCACHE_FLUSH64);
+#endif
+		mb();
+	}
+}
+EXPORT_SYMBOL_GPL(sifive_ccache_flush_range);
+
+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;
+}
+EXPORT_SYMBOL_GPL(sifive_ccache_set_uncached);
+#endif /* CONFIG_RISCV_DMA_NONCOHERENT */
+
 static int ccache_largest_wayenabled(void)
 {
 	return readl(ccache_base + SIFIVE_CCACHE_WAYENABLE) & 0xFF;
@@ -213,6 +261,7 @@ static int __init sifive_ccache_init(void)
 	int i, rc, intr_num;
 	const struct of_device_id *match;
 	unsigned long broken_irqs;
+	u64 __maybe_unused offset;
 
 	np = of_find_matching_node_and_match(NULL, sifive_ccache_ids, &match);
 	if (!np)
@@ -258,6 +307,15 @@ static int __init sifive_ccache_init(void)
 	}
 	of_node_put(np);
 
+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+	if (!of_property_read_u64(np, "uncached-offset", &offset)) {
+		uncached_offset = offset;
+		static_branch_enable(&sifive_ccache_handle_noncoherent_key);
+		riscv_cbom_block_size = SIFIVE_CCACHE_LINE_SIZE;
+		riscv_noncoherent_supported();
+	}
+#endif
+
 	ccache_config_read();
 
 	ccache_cache_ops.get_priv_group = ccache_get_priv_group;
@@ -278,4 +336,4 @@ static int __init sifive_ccache_init(void)
 	return rc;
 }
 
-device_initcall(sifive_ccache_init);
+arch_initcall(sifive_ccache_init);
diff --git a/include/soc/sifive/sifive_ccache.h b/include/soc/sifive/sifive_ccache.h
index 4d4ed49388a0..d349ccb3969b 100644
--- a/include/soc/sifive/sifive_ccache.h
+++ b/include/soc/sifive/sifive_ccache.h
@@ -7,10 +7,31 @@
 #ifndef __SOC_SIFIVE_CCACHE_H
 #define __SOC_SIFIVE_CCACHE_H
 
+#include <linux/io.h>
+#include <linux/jump_label.h>
+
 extern int register_sifive_ccache_error_notifier(struct notifier_block *nb);
 extern int unregister_sifive_ccache_error_notifier(struct notifier_block *nb);
 
 #define SIFIVE_CCACHE_ERR_TYPE_CE 0
 #define SIFIVE_CCACHE_ERR_TYPE_UE 1
 
+DECLARE_STATIC_KEY_FALSE(sifive_ccache_handle_noncoherent_key);
+
+static inline bool sifive_ccache_handle_noncoherent(void)
+{
+#ifdef CONFIG_SIFIVE_CCACHE
+	return static_branch_unlikely(&sifive_ccache_handle_noncoherent_key);
+#else
+	return false;
+#endif
+}
+
+void sifive_ccache_flush_range(phys_addr_t start, size_t len);
+void *sifive_ccache_set_uncached(void *addr, size_t size);
+static inline void sifive_ccache_clear_uncached(void *addr, size_t size)
+{
+	memunmap(addr);
+}
+
 #endif /* __SOC_SIFIVE_CCACHE_H */
-- 
2.39.1

Re: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

[Emil Renner Berthing]

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

Re: [PATCH 04/12] soc: sifive: ccache: Add non-coherent DMA handling

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

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!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

[Cristian Ciocaltea]

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;
+	}
 
+	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);
 }
 
-- 
2.39.1

Re: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

[Ben Dooks]

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);
>   }
>   

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

[Cristian Ciocaltea]

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);
>>   }
> 

Re: [PATCH 05/12] riscv: Implement non-coherent DMA support via SiFive cache flushing

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2211 bytes --]

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.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 06/12] dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible

[Cristian Ciocaltea]

From: Emil Renner Berthing <kernel@esmil.dk>

Document StarFive JH7100 SoC compatible for sysmain registers.

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mfd/syscon.yaml b/Documentation/devicetree/bindings/mfd/syscon.yaml
index c828c4f5e4a7..43f564be709f 100644
--- a/Documentation/devicetree/bindings/mfd/syscon.yaml
+++ b/Documentation/devicetree/bindings/mfd/syscon.yaml
@@ -67,6 +67,7 @@ properties:
               - rockchip,rk3568-qos
               - rockchip,rk3588-qos
               - rockchip,rv1126-qos
+              - starfive,jh7100-sysmain
 
           - const: syscon
 
-- 
2.39.1

Re: [PATCH 06/12] dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible

[Krzysztof Kozlowski]

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> From: Emil Renner Berthing <kernel@esmil.dk>
> 
> Document StarFive JH7100 SoC compatible for sysmain registers.
> 


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 06/12] dt-bindings: mfd: syscon: Add StarFive JH7100 sysmain compatible

[Lee Jones]

On Sat, 11 Feb 2023, Cristian Ciocaltea wrote:

> From: Emil Renner Berthing <kernel@esmil.dk>
> 
> Document StarFive JH7100 SoC compatible for sysmain registers.
> 
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  Documentation/devicetree/bindings/mfd/syscon.yaml | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks

-- 
Lee Jones [李琼斯]

[PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Cristian Ciocaltea]

Add DT bindings documentation for the Synopsys DesignWare MAC found on
the StarFive JH7100 SoC.

Adjust 'reset' and 'reset-names' properties to allow using 'ahb' instead
of the 'stmmaceth' reset signal, as required by JH7100.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   |  15 ++-
 .../bindings/net/starfive,jh7100-dwmac.yaml   | 106 ++++++++++++++++++
 MAINTAINERS                                   |   5 +
 3 files changed, 122 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e88a86623fce..71522a2cd7a4 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -89,6 +89,7 @@ properties:
         - snps,dwmac-5.10a
         - snps,dwxgmac
         - snps,dwxgmac-2.10
+        - starfive,jh7100-dwmac
 
   reg:
     minItems: 1
@@ -131,12 +132,17 @@ properties:
         - ptp_ref
 
   resets:
-    maxItems: 1
-    description:
-      MAC Reset signal.
+    minItems: 1
+    items:
+      - description: MAC Reset signal
+      - description: AHB Reset signal
 
   reset-names:
-    const: stmmaceth
+    minItems: 1
+    contains:
+      enum:
+        - stmmaceth
+        - ahb
 
   power-domains:
     maxItems: 1
@@ -578,6 +584,7 @@ allOf:
               - snps,dwxgmac
               - snps,dwxgmac-2.10
               - st,spear600-gmac
+              - starfive,jh7100-dwmac
 
     then:
       properties:
diff --git a/Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
new file mode 100644
index 000000000000..6afe30690172
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
@@ -0,0 +1,106 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 StarFive Technology Co., Ltd.
+# Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk>
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/starfive,jh7100-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH7100 DWMAC Ethernet Controller
+
+maintainers:
+  - Emil Renner Berthing <kernel@esmil.dk>
+
+# We need a select here so we don't match all nodes with 'snps,dwmac'
+select:
+  properties:
+    compatible:
+      contains:
+        const: starfive,jh7100-dwmac
+  required:
+    - compatible
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+properties:
+  compatible:
+    items:
+      - const: starfive,jh7100-dwmac
+      - const: snps,dwmac
+
+  clocks:
+    items:
+      - description: GMAC main clock
+      - description: GMAC AHB clock
+      - description: PTP clock
+      - description: GTX clock
+      - description: TX clock
+
+  clock-names:
+    items:
+      - const: stmmaceth
+      - const: pclk
+      - const: ptp_ref
+      - const: gtxc
+      - const: tx
+
+  resets:
+    description: AHB Reset signal
+
+  reset-names:
+    const: ahb
+
+  starfive,syscon:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: phandle to the syscon node
+
+  starfive,gtxclk-dlychain:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: GTX clock delay chain setting
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/starfive-jh7100.h>
+    #include <dt-bindings/reset/starfive-jh7100.h>
+
+    gmac: ethernet@10020000 {
+      compatible = "starfive,jh7100-dwmac", "snps,dwmac";
+      reg = <0x0 0x10020000 0x0 0x10000>;
+      clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
+               <&clkgen JH7100_CLK_GMAC_AHB>,
+               <&clkgen JH7100_CLK_GMAC_PTP_REF>,
+               <&clkgen JH7100_CLK_GMAC_GTX>,
+               <&clkgen JH7100_CLK_GMAC_TX_INV>;
+      clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
+      resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
+      reset-names = "ahb";
+      interrupts = <6>, <7>;
+      interrupt-names = "macirq", "eth_wake_irq";
+      max-frame-size = <9000>;
+      phy-mode = "rgmii-txid";
+      snps,multicast-filter-bins = <32>;
+      snps,perfect-filter-entries = <128>;
+      starfive,syscon = <&sysmain>;
+      rx-fifo-depth = <32768>;
+      tx-fifo-depth = <16384>;
+      snps,axi-config = <&stmmac_axi_setup>;
+      snps,fixed-burst;
+      snps,force_thresh_dma_mode;
+      snps,no-pbl-x8;
+
+      stmmac_axi_setup: stmmac-axi-config {
+        snps,wr_osr_lmt = <0xf>;
+        snps,rd_osr_lmt = <0xf>;
+        snps,blen = <256 128 64 32 0 0 0>;
+      };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index abed40db41f0..d48468b81b94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19816,6 +19816,11 @@ M:	Emil Renner Berthing <kernel@esmil.dk>
 S:	Maintained
 F:	arch/riscv/boot/dts/starfive/
 
+STARFIVE DWMAC GLUE LAYER
+M:	Emil Renner Berthing <kernel@esmil.dk>
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/starfive,jh7100-dwmac.yaml
+
 STARFIVE JH7100 CLOCK DRIVERS
 M:	Emil Renner Berthing <kernel@esmil.dk>
 S:	Maintained
-- 
2.39.1

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Andrew Lunn]

> +  starfive,gtxclk-dlychain:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: GTX clock delay chain setting

Please could you add more details to this. Is this controlling the
RGMII delays? 0ns or 2ns?

> +    gmac: ethernet@10020000 {
> +      compatible = "starfive,jh7100-dwmac", "snps,dwmac";
> +      reg = <0x0 0x10020000 0x0 0x10000>;
> +      clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
> +               <&clkgen JH7100_CLK_GMAC_AHB>,
> +               <&clkgen JH7100_CLK_GMAC_PTP_REF>,
> +               <&clkgen JH7100_CLK_GMAC_GTX>,
> +               <&clkgen JH7100_CLK_GMAC_TX_INV>;
> +      clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
> +      resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
> +      reset-names = "ahb";
> +      interrupts = <6>, <7>;
> +      interrupt-names = "macirq", "eth_wake_irq";
> +      max-frame-size = <9000>;
> +      phy-mode = "rgmii-txid";

This is unusual. Does your board have a really long RX clock line to
insert the 2ns delay needed on the RX side?

       Andrew

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Cristian Ciocaltea]

On 2/11/23 18:01, Andrew Lunn wrote:
>> +  starfive,gtxclk-dlychain:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: GTX clock delay chain setting
> 
> Please could you add more details to this. Is this controlling the
> RGMII delays? 0ns or 2ns?

This is what gets written to JH7100_SYSMAIN_REGISTER49 and it's 
currently set to 4 in patch 12/12. As already mentioned, I don't have 
the register information in the datasheet, but I'll update this as soon 
as we get some details.

>> +    gmac: ethernet@10020000 {
>> +      compatible = "starfive,jh7100-dwmac", "snps,dwmac";
>> +      reg = <0x0 0x10020000 0x0 0x10000>;
>> +      clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
>> +               <&clkgen JH7100_CLK_GMAC_AHB>,
>> +               <&clkgen JH7100_CLK_GMAC_PTP_REF>,
>> +               <&clkgen JH7100_CLK_GMAC_GTX>,
>> +               <&clkgen JH7100_CLK_GMAC_TX_INV>;
>> +      clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
>> +      resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
>> +      reset-names = "ahb";
>> +      interrupts = <6>, <7>;
>> +      interrupt-names = "macirq", "eth_wake_irq";
>> +      max-frame-size = <9000>;
>> +      phy-mode = "rgmii-txid";
> 
> This is unusual. Does your board have a really long RX clock line to
> insert the 2ns delay needed on the RX side?

Just tested with "rgmii" and didn't notice any issues. If I'm not 
missing anything, I'll do the change in the next revision.

>         Andrew

Thanks,
Cristian

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Andrew Lunn]

On Wed, Feb 15, 2023 at 02:34:23AM +0200, Cristian Ciocaltea wrote:
> On 2/11/23 18:01, Andrew Lunn wrote:
> > > +  starfive,gtxclk-dlychain:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    description: GTX clock delay chain setting
> > 
> > Please could you add more details to this. Is this controlling the
> > RGMII delays? 0ns or 2ns?
> 
> This is what gets written to JH7100_SYSMAIN_REGISTER49 and it's currently
> set to 4 in patch 12/12. As already mentioned, I don't have the register
> information in the datasheet, but I'll update this as soon as we get some
> details.

I have seen what happens to this value, but i have no idea what it
actually means. And without knowing what it means, i cannot say if it
is being used correctly or not. And it could be related to the next
part of my comment...

> 
> > > +    gmac: ethernet@10020000 {
> > > +      compatible = "starfive,jh7100-dwmac", "snps,dwmac";
> > > +      reg = <0x0 0x10020000 0x0 0x10000>;
> > > +      clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
> > > +               <&clkgen JH7100_CLK_GMAC_AHB>,
> > > +               <&clkgen JH7100_CLK_GMAC_PTP_REF>,
> > > +               <&clkgen JH7100_CLK_GMAC_GTX>,
> > > +               <&clkgen JH7100_CLK_GMAC_TX_INV>;
> > > +      clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
> > > +      resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
> > > +      reset-names = "ahb";
> > > +      interrupts = <6>, <7>;
> > > +      interrupt-names = "macirq", "eth_wake_irq";
> > > +      max-frame-size = <9000>;
> > > +      phy-mode = "rgmii-txid";
> > 
> > This is unusual. Does your board have a really long RX clock line to
> > insert the 2ns delay needed on the RX side?
> 
> Just tested with "rgmii" and didn't notice any issues. If I'm not missing
> anything, I'll do the change in the next revision.

rgmii-id is generally the value to be used. That indicates the board
needs 2ns delays adding by something, either the MAC or the PHY. And
then i always recommend the MAC driver does nothing, pass the value to
the PHY and let the PHY add the delays.

So try both rgmii and rgmii-id and do a lot of bi directional
transfers. Then look at the reported ethernet frame check sum error
counts, both local and the link peer. I would expect one setting gives
you lots of errors, and the other works much better.

    Andrew

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Cristian Ciocaltea]

On 2/15/23 15:01, Andrew Lunn wrote:
> On Wed, Feb 15, 2023 at 02:34:23AM +0200, Cristian Ciocaltea wrote:
>> On 2/11/23 18:01, Andrew Lunn wrote:
>>>> +  starfive,gtxclk-dlychain:
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    description: GTX clock delay chain setting
>>>
>>> Please could you add more details to this. Is this controlling the
>>> RGMII delays? 0ns or 2ns?
>>
>> This is what gets written to JH7100_SYSMAIN_REGISTER49 and it's currently
>> set to 4 in patch 12/12. As already mentioned, I don't have the register
>> information in the datasheet, but I'll update this as soon as we get some
>> details.
> 
> I have seen what happens to this value, but i have no idea what it
> actually means. And without knowing what it means, i cannot say if it
> is being used correctly or not. And it could be related to the next
> part of my comment...
> 
>>
>>>> +    gmac: ethernet@10020000 {
>>>> +      compatible = "starfive,jh7100-dwmac", "snps,dwmac";
>>>> +      reg = <0x0 0x10020000 0x0 0x10000>;
>>>> +      clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
>>>> +               <&clkgen JH7100_CLK_GMAC_AHB>,
>>>> +               <&clkgen JH7100_CLK_GMAC_PTP_REF>,
>>>> +               <&clkgen JH7100_CLK_GMAC_GTX>,
>>>> +               <&clkgen JH7100_CLK_GMAC_TX_INV>;
>>>> +      clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
>>>> +      resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
>>>> +      reset-names = "ahb";
>>>> +      interrupts = <6>, <7>;
>>>> +      interrupt-names = "macirq", "eth_wake_irq";
>>>> +      max-frame-size = <9000>;
>>>> +      phy-mode = "rgmii-txid";
>>>
>>> This is unusual. Does your board have a really long RX clock line to
>>> insert the 2ns delay needed on the RX side?
>>
>> Just tested with "rgmii" and didn't notice any issues. If I'm not missing
>> anything, I'll do the change in the next revision.
> 
> rgmii-id is generally the value to be used. That indicates the board
> needs 2ns delays adding by something, either the MAC or the PHY. And
> then i always recommend the MAC driver does nothing, pass the value to
> the PHY and let the PHY add the delays.
> 
> So try both rgmii and rgmii-id and do a lot of bi directional
> transfers. Then look at the reported ethernet frame check sum error
> counts, both local and the link peer. I would expect one setting gives
> you lots of errors, and the other works much better.

I gave "rgmii-id" a try and it's not usable, I get too many errors. So 
"rgmii" should be the right choice here.

Thanks,
Cristian

>      Andrew
> 

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Andrew Lunn]

> I gave "rgmii-id" a try and it's not usable, I get too many errors. So
> "rgmii" should be the right choice here.

I would actually say it shows we don't understand what is going on
with delays. "rgmii" is not every often the correct value. The fact it
works suggests the MAC is adding delays.

What value are you using for starfive,gtxclk-dlychain ? Try 0 and then
"rgmii-id"

	Andrew			 

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Cristian Ciocaltea]

On 2/16/23 19:54, Andrew Lunn wrote:
>> I gave "rgmii-id" a try and it's not usable, I get too many errors. So
>> "rgmii" should be the right choice here.
> 
> I would actually say it shows we don't understand what is going on
> with delays. "rgmii" is not every often the correct value. The fact it
> works suggests the MAC is adding delays.
> 
> What value are you using for starfive,gtxclk-dlychain ? 

This is set to '4' in patch 12/12.

> Try 0 and then "rgmii-id"

I made some more tests and it seems the only stable configuration is 
"rgmii" with "starfive,gtxclk-dlychain" set to 4:

phy-mode | dlychain | status
---------+----------+--------------------------------------------
rgmii    |        4 | OK (no issues observed)
rgmii-id |        4 | BROKEN (errors reported [1])
rgmii    |        0 | UNRELIABLE (no errors, but frequent stalls)
rgmii-id |        0 | BROKEN (errors reported)

[1] Reported errors in case of BROKEN status:
$ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'

/sys/class/net/eth0/statistics/rx_crc_errors:6
/sys/class/net/eth0/statistics/rx_errors:6
/sys/class/net/eth0/statistics/tx_bytes:10836
/sys/class/net/eth0/statistics/tx_packets:46

> 	Andrew			
> 

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Andrew Lunn]

> > I would actually say it shows we don't understand what is going on
> > with delays. "rgmii" is not every often the correct value. The fact it
> > works suggests the MAC is adding delays.
> > 
> > What value are you using for starfive,gtxclk-dlychain ?
> 
> This is set to '4' in patch 12/12.
> 
> > Try 0 and then "rgmii-id"
> 
> I made some more tests and it seems the only stable configuration is "rgmii"
> with "starfive,gtxclk-dlychain" set to 4:
> 
> phy-mode | dlychain | status
> ---------+----------+--------------------------------------------
> rgmii    |        4 | OK (no issues observed)
> rgmii-id |        4 | BROKEN (errors reported [1])
> rgmii    |        0 | UNRELIABLE (no errors, but frequent stalls)
> rgmii-id |        0 | BROKEN (errors reported)
> 
> [1] Reported errors in case of BROKEN status:
> $ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'

Thanks for the testing.

So it seems like something is adding delays when it probably should
not. Ideally we want to know what.

There is a danger here, something which has happened in the past. A
PHY which ignored "rgmii" and actually did power on defaults which was
"rgmii-id". As a result, lots of boards put "rmgii" in there DT blob,
which 'worked'. Until a board came along which really did need
"rgmii". The developer bringing that board up debugged the PHY, found
the problem and made it respect "rgmii" so their board worked. And the
fix broke a number of 'working' boards which had the wrong "rgmii"
instead of "rgmii-id".

So you have a choice. Go with 4 and "rgmii", but put in a big fat
warning, "Works somehow but is technically wrong and will probably
break sometime in the future". Or try to understand what is really
going on here, were are the delays coming from, and fix the issue.

      Andrew

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Cristian Ciocaltea]

On 2/17/23 15:30, Andrew Lunn wrote:
>>> I would actually say it shows we don't understand what is going on
>>> with delays. "rgmii" is not every often the correct value. The fact it
>>> works suggests the MAC is adding delays.
>>>
>>> What value are you using for starfive,gtxclk-dlychain ?
>>
>> This is set to '4' in patch 12/12.
>>
>>> Try 0 and then "rgmii-id"
>>
>> I made some more tests and it seems the only stable configuration is "rgmii"
>> with "starfive,gtxclk-dlychain" set to 4:
>>
>> phy-mode | dlychain | status
>> ---------+----------+--------------------------------------------
>> rgmii    |        4 | OK (no issues observed)
>> rgmii-id |        4 | BROKEN (errors reported [1])
>> rgmii    |        0 | UNRELIABLE (no errors, but frequent stalls)
>> rgmii-id |        0 | BROKEN (errors reported)
>>
>> [1] Reported errors in case of BROKEN status:
>> $ grep '' /sys/class/net/eth0/statistics/* | grep -v ':0$'
> 
> Thanks for the testing.
> 
> So it seems like something is adding delays when it probably should
> not. Ideally we want to know what.
> 
> There is a danger here, something which has happened in the past. A
> PHY which ignored "rgmii" and actually did power on defaults which was
> "rgmii-id". As a result, lots of boards put "rmgii" in there DT blob,
> which 'worked'. Until a board came along which really did need
> "rgmii". The developer bringing that board up debugged the PHY, found
> the problem and made it respect "rgmii" so their board worked. And the
> fix broke a number of 'working' boards which had the wrong "rgmii"
> instead of "rgmii-id".

Thanks for the heads-up.

> So you have a choice. Go with 4 and "rgmii", but put in a big fat
> warning, "Works somehow but is technically wrong and will probably
> break sometime in the future". Or try to understand what is really
> going on here, were are the delays coming from, and fix the issue.
> 
>        Andrew

I will try to analyze this further.

Regards,
Cristian

Re: [PATCH 07/12] dt-bindings: net: Add StarFive JH7100 SoC

[Krzysztof Kozlowski]

On 11/02/2023 04:18, Cristian Ciocaltea wrote:
> Add DT bindings documentation for the Synopsys DesignWare MAC found on
> the StarFive JH7100 SoC.
> 
> Adjust 'reset' and 'reset-names' properties to allow using 'ahb' instead
> of the 'stmmaceth' reset signal, as required by JH7100.
> 
> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |  15 ++-
>  .../bindings/net/starfive,jh7100-dwmac.yaml   | 106 ++++++++++++++++++


FYI, there is conflicting work:

https://lore.kernel.org/all/20230118061701.30047-5-yanhong.wang@starfivetech.com/

It's almost the same, thus this should be dropped.

Best regards,
Krzysztof

[PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Cristian Ciocaltea]

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"
+	default m if SOC_STARFIVE
+	depends on SOC_STARFIVE || COMPILE_TEST
+	select MFD_SYSCON
+	help
+	  Support for ethernet controller on StarFive SOCs.
+
+	  This selects StarFive SoC glue layer support for the stmmac device
+	  driver. This driver is used for the JH71x0 series GMAC ethernet
+	  controller.
+
 config DWMAC_STI
 	tristate "STi GMAC support"
 	default ARCH_STI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 057e4bab5c08..8738fdbb4b2d 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS)	+= dwmac-oxnas.o
 obj-$(CONFIG_DWMAC_QCOM_ETHQOS)	+= dwmac-qcom-ethqos.o
 obj-$(CONFIG_DWMAC_ROCKCHIP)	+= dwmac-rk.o
 obj-$(CONFIG_DWMAC_SOCFPGA)	+= dwmac-altr-socfpga.o
+obj-$(CONFIG_DWMAC_STARFIVE)	+= dwmac-starfive.o
 obj-$(CONFIG_DWMAC_STI)		+= dwmac-sti.o
 obj-$(CONFIG_DWMAC_STM32)	+= dwmac-stm32.o
 obj-$(CONFIG_DWMAC_SUNXI)	+= dwmac-sunxi.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
new file mode 100644
index 000000000000..d4c81f1a5482
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dwmac-starfive.c - DWMAC glue layer for StarFive JH7100 SoC
+ *
+ * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk>
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#include "stmmac.h"
+#include "stmmac_platform.h"
+
+#define JH7100_SYSMAIN_REGISTER28 0x70
+/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
+#define JH7100_SYSMAIN_REGISTER49 0xc8
+
+struct dwmac_starfive {
+	struct device *dev;
+	struct clk *gtxc;
+};
+
+static int dwmac_starfive_jh7100_syscon_init(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct regmap *sysmain;
+	u32 gtxclk_dlychain;
+	int ret;
+
+	sysmain = syscon_regmap_lookup_by_phandle(np, "starfive,syscon");
+	if (IS_ERR(sysmain))
+		return dev_err_probe(dev, PTR_ERR(sysmain),
+				     "error getting sysmain registers\n");
+
+	/* Choose RGMII interface to the phy.
+	 * TODO: support other interfaces once we know the meaning of other
+	 * values in the register
+	 */
+	ret = regmap_update_bits(sysmain, JH7100_SYSMAIN_REGISTER28, 0x7, 1);
+	if (ret)
+		return dev_err_probe(dev, ret, "error selecting gmac interface\n");
+
+	if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
+		ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
+		if (ret)
+			return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
+	}
+
+	return 0;
+}
+
+static void dwmac_starfive_fix_mac_speed(void *data, unsigned int speed)
+{
+	struct dwmac_starfive *dwmac = data;
+	unsigned long rate;
+	int ret;
+
+	switch (speed) {
+	case SPEED_1000:
+		rate = 125000000;
+		break;
+	case SPEED_100:
+		rate = 25000000;
+		break;
+	case SPEED_10:
+		rate = 2500000;
+		break;
+	default:
+		dev_warn(dwmac->dev, "unsupported link speed %u\n", speed);
+		return;
+	}
+
+	ret = clk_set_rate(dwmac->gtxc, rate);
+	if (ret)
+		dev_err(dwmac->dev, "error setting gtx clock rate: %d\n", ret);
+}
+
+static int dwmac_starfive_probe(struct platform_device *pdev)
+{
+	struct stmmac_resources stmmac_res;
+	struct plat_stmmacenet_data *plat;
+	struct dwmac_starfive *dwmac;
+	struct clk *txclk;
+	int (*syscon_init)(struct device *dev);
+	int ret;
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (ret)
+		return ret;
+
+	syscon_init = of_device_get_match_data(&pdev->dev);
+	if (syscon_init) {
+		ret = syscon_init(&pdev->dev);
+		if (ret)
+			return ret;
+	}
+
+	dwmac->gtxc = devm_clk_get_enabled(&pdev->dev, "gtxc");
+	if (IS_ERR(dwmac->gtxc))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->gtxc),
+				     "error getting/enabling gtxc clock\n");
+
+	txclk = devm_clk_get_enabled(&pdev->dev, "tx");
+	if (IS_ERR(txclk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(txclk),
+				     "error getting/enabling tx clock\n");
+
+	plat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat))
+		return dev_err_probe(&pdev->dev, PTR_ERR(plat),
+				     "dt configuration failed\n");
+
+	dwmac->dev = &pdev->dev;
+	plat->bsp_priv = dwmac;
+	plat->fix_mac_speed = dwmac_starfive_fix_mac_speed;
+
+	ret = stmmac_dvr_probe(&pdev->dev, plat, &stmmac_res);
+	if (ret) {
+		stmmac_remove_config_dt(pdev, plat);
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id dwmac_starfive_match[] = {
+	{
+		.compatible = "starfive,jh7100-dwmac",
+		.data = dwmac_starfive_jh7100_syscon_init,
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, dwmac_starfive_match);
+
+static struct platform_driver dwmac_starfive_driver = {
+	.probe  = dwmac_starfive_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name           = "dwmac-starfive",
+		.pm		= &stmmac_pltfr_pm_ops,
+		.of_match_table = dwmac_starfive_match,
+	},
+};
+module_platform_driver(dwmac_starfive_driver);
+
+MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
+MODULE_DESCRIPTION("StarFive DWMAC Glue Layer");
+MODULE_LICENSE("GPL");
-- 
2.39.1

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Andrew Lunn]

> +
> +#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", &gtxclk_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

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Cristian Ciocaltea]

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", &gtxclk_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

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Emil Renner Berthing]

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", &gtxclk_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

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Cristian Ciocaltea]

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", &gtxclk_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

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Andrew Lunn]

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

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Krzysztof Kozlowski]

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

Re: [PATCH 08/12] net: stmmac: Add glue layer for StarFive JH7100 SoC

[Cristian Ciocaltea]

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
> 
> 

[PATCH 09/12] riscv: dts: starfive: Add dma-noncoherent for JH7100 SoC

[Cristian Ciocaltea]

The RISC-V architecture is by default coherent, as indicated by
CONFIG_OF_DMA_DEFAULT_COHERENT, but the StarFive JH7100 is not, hence
provide the dma-noncoherent property to the soc DT node.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 arch/riscv/boot/dts/starfive/jh7100.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 000447482aca..7109e70fdab8 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -114,6 +114,7 @@ soc {
 		#address-cells = <2>;
 		#size-cells = <2>;
 		ranges;
+		dma-noncoherent;
 
 		clint: clint@2000000 {
 			compatible = "starfive,jh7100-clint", "sifive,clint0";
-- 
2.39.1

[PATCH 10/12] riscv: dts: starfive: jh7100: Add ccache DT node

[Cristian Ciocaltea]

Provide a DT node for the Sifive Composable Cache controller found on
the StarFive JH7100 SoC.

Note this is also used to support non-coherent DMA.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 arch/riscv/boot/dts/starfive/jh7100.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100.dtsi b/arch/riscv/boot/dts/starfive/jh7100.dtsi
index 7109e70fdab8..88f91bc5753b 100644
--- a/arch/riscv/boot/dts/starfive/jh7100.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100.dtsi
@@ -32,6 +32,7 @@ U74_0: cpu@0 {
 			i-tlb-sets = <1>;
 			i-tlb-size = <32>;
 			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
 			riscv,isa = "rv64imafdc";
 			tlb-split;
 
@@ -57,6 +58,7 @@ U74_1: cpu@1 {
 			i-tlb-sets = <1>;
 			i-tlb-size = <32>;
 			mmu-type = "riscv,sv39";
+			next-level-cache = <&ccache>;
 			riscv,isa = "rv64imafdc";
 			tlb-split;
 
@@ -116,6 +118,20 @@ soc {
 		ranges;
 		dma-noncoherent;
 
+		ccache: cache-controller@2010000 {
+			compatible = "starfive,jh7100-ccache", "cache";
+			reg = <0x0 0x2010000 0x0 0x1000>,
+			      <0x0 0x8000000 0x0 0x2000000>;
+			reg-names = "control", "sideband";
+			interrupts = <128>, <130>, <131>, <129>;
+			cache-block-size = <64>;
+			cache-level = <2>;
+			cache-sets = <2048>;
+			cache-size = <2097152>;
+			cache-unified;
+			uncached-offset = <0xf 0x80000000>;
+		};
+
 		clint: clint@2000000 {
 			compatible = "starfive,jh7100-clint", "sifive,clint0";
 			reg = <0x0 0x2000000 0x0 0x10000>;
-- 
2.39.1

[PATCH 11/12] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

[Cristian Ciocaltea]

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 {
+			compatible = "starfive,jh7100-dwmac", "snps,dwmac";
+			reg = <0x0 0x10020000 0x0 0x10000>;
+			clocks = <&clkgen JH7100_CLK_GMAC_ROOT_DIV>,
+				 <&clkgen JH7100_CLK_GMAC_AHB>,
+				 <&clkgen JH7100_CLK_GMAC_PTP_REF>,
+				 <&clkgen JH7100_CLK_GMAC_GTX>,
+				 <&clkgen JH7100_CLK_GMAC_TX_INV>;
+			clock-names = "stmmaceth", "pclk", "ptp_ref", "gtxc", "tx";
+			resets = <&rstgen JH7100_RSTN_GMAC_AHB>;
+			reset-names = "ahb";
+			interrupts = <6>, <7>;
+			interrupt-names = "macirq", "eth_wake_irq";
+			max-frame-size = <9000>;
+			phy-mode = "rgmii-txid";
+			snps,multicast-filter-bins = <32>;
+			snps,perfect-filter-entries = <128>;
+			starfive,syscon = <&sysmain>;
+			rx-fifo-depth = <32768>;
+			tx-fifo-depth = <16384>;
+			snps,axi-config = <&stmmac_axi_setup>;
+			snps,fixed-burst;
+			snps,force_thresh_dma_mode;
+			snps,no-pbl-x8;
+			status = "disabled";
+
+			stmmac_axi_setup: stmmac-axi-config {
+				snps,wr_osr_lmt = <0xf>;
+				snps,rd_osr_lmt = <0xf>;
+				snps,blen = <256 128 64 32 0 0 0>;
+			};
+		};
+
 		i2c0: i2c@118b0000 {
 			compatible = "snps,designware-i2c";
 			reg = <0x0 0x118b0000 0x0 0x10000>;
-- 
2.39.1

Re: [PATCH 11/12] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

[Krzysztof Kozlowski]

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

Re: [PATCH 11/12] riscv: dts: starfive: jh7100: Add sysmain and gmac DT nodes

[Cristian Ciocaltea]

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
> 
> 

[PATCH 12/12] riscv: dts: starfive: jh7100-common: Setup pinmux and enable gmac

[Cristian Ciocaltea]

Add pinmux configuration for the DWMAC found on the JH7100 based boards
and enable the gmac DT node.

Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
---
 .../boot/dts/starfive/jh7100-common.dtsi      | 78 +++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
index b93ce351a90f..9927e7462e9f 100644
--- a/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7100-common.dtsi
@@ -41,7 +41,85 @@ led-ack {
 	};
 };
 
+&gmac {
+	starfive,gtxclk-dlychain = <4>;
+	pinctrl-names = "default";
+	pinctrl-0 = <&gmac_pins>;
+	status = "okay";
+};
+
 &gpio {
+	gmac_pins: gmac-0 {
+		gtxclk-pins {
+			pins = <PAD_FUNC_SHARE(115)>;
+			bias-pull-up;
+			drive-strength = <35>;
+			input-enable;
+			input-schmitt-enable;
+			slew-rate = <0>;
+		};
+		miitxclk-pins {
+			pins = <PAD_FUNC_SHARE(116)>;
+			bias-pull-up;
+			drive-strength = <14>;
+			input-enable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+		tx-pins {
+			pins = <PAD_FUNC_SHARE(117)>,
+			       <PAD_FUNC_SHARE(119)>,
+			       <PAD_FUNC_SHARE(120)>,
+			       <PAD_FUNC_SHARE(121)>,
+			       <PAD_FUNC_SHARE(122)>,
+			       <PAD_FUNC_SHARE(123)>,
+			       <PAD_FUNC_SHARE(124)>,
+			       <PAD_FUNC_SHARE(125)>,
+			       <PAD_FUNC_SHARE(126)>;
+			bias-pull-up;
+			drive-strength = <35>;
+			input-disable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+		rxclk-pins {
+			pins = <PAD_FUNC_SHARE(127)>;
+			bias-pull-up;
+			drive-strength = <14>;
+			input-enable;
+			input-schmitt-disable;
+			slew-rate = <6>;
+		};
+		rxer-pins {
+			pins = <PAD_FUNC_SHARE(129)>;
+			bias-pull-up;
+			drive-strength = <14>;
+			input-enable;
+			input-schmitt-disable;
+			slew-rate = <0>;
+		};
+		rx-pins {
+			pins = <PAD_FUNC_SHARE(128)>,
+			       <PAD_FUNC_SHARE(130)>,
+			       <PAD_FUNC_SHARE(131)>,
+			       <PAD_FUNC_SHARE(132)>,
+			       <PAD_FUNC_SHARE(133)>,
+			       <PAD_FUNC_SHARE(134)>,
+			       <PAD_FUNC_SHARE(135)>,
+			       <PAD_FUNC_SHARE(136)>,
+			       <PAD_FUNC_SHARE(137)>,
+			       <PAD_FUNC_SHARE(138)>,
+			       <PAD_FUNC_SHARE(139)>,
+			       <PAD_FUNC_SHARE(140)>,
+			       <PAD_FUNC_SHARE(141)>;
+			bias-pull-up;
+			drive-strength = <14>;
+			input-enable;
+			input-schmitt-enable;
+			slew-rate = <0>;
+		};
+	};
+
 	i2c0_pins: i2c0-0 {
 		i2c-pins {
 			pinmux = <GPIOMUX(62, GPO_LOW,
-- 
2.39.1

Re: [PATCH 00/12] Enable networking support for StarFive JH7100 SoC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3865 bytes --]

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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 00/12] Enable networking support for StarFive JH7100 SoC

[Cristian Ciocaltea]

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
>>