2024-03-27 01:58:32

by ChunHau Tan

[permalink] [raw]
Subject: [PATCH v5 0/1] Add StarFive JH8100 dwmac support

Add StarFive JH8100 dwmac support.
The JH8100 dwmac shares the same driver code as the JH7110 dwmac
and has only one reset signal.

Please refer to below:

JH8100: reset-names = "stmmaceth";
JH7110: reset-names = "stmmaceth", "ahb";
JH7100: reset-names = "ahb";

Example usage of JH8100 in the device tree:

gmac0: ethernet@16030000 {
compatible = "starfive,jh8100-dwmac",
"starfive,jh7110-dwmac",
"snps,dwmac-5.20";
...
};

Changes in v5:
- Addressed duplicated interrupts and interrupt-names.

Tan Chun Hau (1):
dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support

.../devicetree/bindings/net/snps,dwmac.yaml | 1 +
.../bindings/net/starfive,jh7110-dwmac.yaml | 29 +++++++++++++++----
2 files changed, 25 insertions(+), 5 deletions(-)

--
2.25.1



2024-03-27 01:58:53

by ChunHau Tan

[permalink] [raw]
Subject: [PATCH v5 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support

Add StarFive JH8100 dwmac support.
The JH8100 dwmac shares the same driver code as the JH7110 dwmac
and has only one reset signal.

Please refer to below:

JH8100: reset-names = "stmmaceth";
JH7110: reset-names = "stmmaceth", "ahb";
JH7100: reset-names = "ahb";

Example usage of JH8100 in the device tree:

gmac0: ethernet@16030000 {
compatible = "starfive,jh8100-dwmac",
"starfive,jh7110-dwmac",
"snps,dwmac-5.20";
...
};

Signed-off-by: Tan Chun Hau <[email protected]>
---
.../devicetree/bindings/net/snps,dwmac.yaml | 1 +
.../bindings/net/starfive,jh7110-dwmac.yaml | 29 +++++++++++++++----
2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 6b0341a8e0ea..a6d596b7dcf4 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -97,6 +97,7 @@ properties:
- snps,dwxgmac-2.10
- starfive,jh7100-dwmac
- starfive,jh7110-dwmac
+ - starfive,jh8100-dwmac

reg:
minItems: 1
diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
index 0d1962980f57..5805a58c55d1 100644
--- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
@@ -18,6 +18,7 @@ select:
enum:
- starfive,jh7100-dwmac
- starfive,jh7110-dwmac
+ - starfive,jh8100-dwmac
required:
- compatible

@@ -30,6 +31,10 @@ properties:
- items:
- const: starfive,jh7110-dwmac
- const: snps,dwmac-5.20
+ - items:
+ - const: starfive,jh8100-dwmac
+ - const: starfive,jh7110-dwmac
+ - const: snps,dwmac-5.20

reg:
maxItems: 1
@@ -116,11 +121,25 @@ allOf:
minItems: 3
maxItems: 3

- resets:
- minItems: 2
-
- reset-names:
- minItems: 2
+ if:
+ properties:
+ compatible:
+ contains:
+ const: starfive,jh8100-dwmac
+ then:
+ properties:
+ resets:
+ maxItems: 1
+
+ reset-names:
+ const: stmmaceth
+ else:
+ properties:
+ resets:
+ minItems: 2
+
+ reset-names:
+ minItems: 2

unevaluatedProperties: false

--
2.25.1


2024-03-27 07:55:33

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support

On 27/03/2024 02:57, Tan Chun Hau wrote:
> Add StarFive JH8100 dwmac support.
> The JH8100 dwmac shares the same driver code as the JH7110 dwmac
> and has only one reset signal.
>
> Please refer to below:
>
> JH8100: reset-names = "stmmaceth";
> JH7110: reset-names = "stmmaceth", "ahb";
> JH7100: reset-names = "ahb";
>
> Example usage of JH8100 in the device tree:
>
> gmac0: ethernet@16030000 {
> compatible = "starfive,jh8100-dwmac",
> "starfive,jh7110-dwmac",
> "snps,dwmac-5.20";
> ...
> };
>
> Signed-off-by: Tan Chun Hau <[email protected]>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,jh7110-dwmac.yaml | 29 +++++++++++++++----
> 2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 6b0341a8e0ea..a6d596b7dcf4 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -97,6 +97,7 @@ properties:
> - snps,dwxgmac-2.10
> - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
> + - starfive,jh8100-dwmac

I think that's not needed. You have there already your fallback.

>
> reg:
> minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> index 0d1962980f57..5805a58c55d1 100644
> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> @@ -18,6 +18,7 @@ select:
> enum:
> - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
> + - starfive,jh8100-dwmac

Same here, even more obvious.

> required:
> - compatible
>
> @@ -30,6 +31,10 @@ properties:
> - items:
> - const: starfive,jh7110-dwmac
> - const: snps,dwmac-5.20
> + - items:
> + - const: starfive,jh8100-dwmac
> + - const: starfive,jh7110-dwmac
> + - const: snps,dwmac-5.20
>
> reg:
> maxItems: 1
> @@ -116,11 +121,25 @@ allOf:
> minItems: 3
> maxItems: 3
>
> - resets:
> - minItems: 2
> -
> - reset-names:
> - minItems: 2
> + if:

I would personally avoid nesting if within if. It gets unreadable.
Although Rob did not comment on this one, so I guess it is fine.

Best regards,
Krzysztof


2024-03-28 20:43:26

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v5 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support

On Wed, Mar 27, 2024 at 08:54:30AM +0100, Krzysztof Kozlowski wrote:
> On 27/03/2024 02:57, Tan Chun Hau wrote:
> > Add StarFive JH8100 dwmac support.
> > The JH8100 dwmac shares the same driver code as the JH7110 dwmac
> > and has only one reset signal.
> >
> > Please refer to below:
> >
> > JH8100: reset-names = "stmmaceth";
> > JH7110: reset-names = "stmmaceth", "ahb";
> > JH7100: reset-names = "ahb";
> >
> > Example usage of JH8100 in the device tree:
> >
> > gmac0: ethernet@16030000 {
> > compatible = "starfive,jh8100-dwmac",
> > "starfive,jh7110-dwmac",
> > "snps,dwmac-5.20";
> > ...
> > };
> >
> > Signed-off-by: Tan Chun Hau <[email protected]>
> > ---
> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > .../bindings/net/starfive,jh7110-dwmac.yaml | 29 +++++++++++++++----
> > 2 files changed, 25 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > index 6b0341a8e0ea..a6d596b7dcf4 100644
> > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > @@ -97,6 +97,7 @@ properties:
> > - snps,dwxgmac-2.10
> > - starfive,jh7100-dwmac
> > - starfive,jh7110-dwmac
> > + - starfive,jh8100-dwmac
>
> I think that's not needed. You have there already your fallback.
>
> >
> > reg:
> > minItems: 1
> > diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > index 0d1962980f57..5805a58c55d1 100644
> > --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > @@ -18,6 +18,7 @@ select:
> > enum:
> > - starfive,jh7100-dwmac
> > - starfive,jh7110-dwmac
> > + - starfive,jh8100-dwmac
>
> Same here, even more obvious.

Agreed.

>
> > required:
> > - compatible
> >
> > @@ -30,6 +31,10 @@ properties:
> > - items:
> > - const: starfive,jh7110-dwmac
> > - const: snps,dwmac-5.20
> > + - items:
> > + - const: starfive,jh8100-dwmac
> > + - const: starfive,jh7110-dwmac
> > + - const: snps,dwmac-5.20
> >
> > reg:
> > maxItems: 1
> > @@ -116,11 +121,25 @@ allOf:
> > minItems: 3
> > maxItems: 3
> >
> > - resets:
> > - minItems: 2
> > -
> > - reset-names:
> > - minItems: 2
> > + if:
>
> I would personally avoid nesting if within if. It gets unreadable.
> Although Rob did not comment on this one, so I guess it is fine.

I normally agree, but here I suggested it as it looked to be the
simplest option.

With the 2 other comments addressed,

Reviewed-by: Rob Herring <[email protected]>

2024-04-03 10:58:18

by ChunHau Tan

[permalink] [raw]
Subject: RE: [PATCH v5 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support



> -----Original Message-----
> From: Rob Herring <[email protected]>
> Sent: Friday, 29 March, 2024 4:42 AM
> To: Krzysztof Kozlowski <[email protected]>
> Cc: ChunHau Tan <[email protected]>; David S . Miller
> <[email protected]>; Eric Dumazet <[email protected]>; Jakub
> Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Emil Renner
> Berthing <[email protected]>; Krzysztof Kozlowski
> <[email protected]>; Conor Dooley <[email protected]>;
> Maxime Coquelin <[email protected]>; Alexandre Torgue
> <[email protected]>; Simon Horman <[email protected]>; Bartosz
> Golaszewski <[email protected]>; Andrew Halaney
> <[email protected]>; Jisheng Zhang <[email protected]>; Uwe
> Kleine-K?nig <[email protected]>; Russell King
> <[email protected]>; Leyfoon Tan <[email protected]>;
> JeeHeng Sia <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected];
> [email protected]; [email protected]
> Subject: Re: [PATCH v5 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add
> StarFive JH8100 support
>
> On Wed, Mar 27, 2024 at 08:54:30AM +0100, Krzysztof Kozlowski wrote:
> > On 27/03/2024 02:57, Tan Chun Hau wrote:
> > > Add StarFive JH8100 dwmac support.
> > > The JH8100 dwmac shares the same driver code as the JH7110 dwmac and
> > > has only one reset signal.
> > >
> > > Please refer to below:
> > >
> > > JH8100: reset-names = "stmmaceth";
> > > JH7110: reset-names = "stmmaceth", "ahb";
> > > JH7100: reset-names = "ahb";
> > >
> > > Example usage of JH8100 in the device tree:
> > >
> > > gmac0: ethernet@16030000 {
> > > compatible = "starfive,jh8100-dwmac",
> > > "starfive,jh7110-dwmac",
> > > "snps,dwmac-5.20";
> > > ...
> > > };
> > >
> > > Signed-off-by: Tan Chun Hau <[email protected]>
> > > ---
> > > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > > .../bindings/net/starfive,jh7110-dwmac.yaml | 29 +++++++++++++++----
> > > 2 files changed, 25 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > index 6b0341a8e0ea..a6d596b7dcf4 100644
> > > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> > > @@ -97,6 +97,7 @@ properties:
> > > - snps,dwxgmac-2.10
> > > - starfive,jh7100-dwmac
> > > - starfive,jh7110-dwmac
> > > + - starfive,jh8100-dwmac
> >
> > I think that's not needed. You have there already your fallback.

Okay, I will remove it.

> >
> > >
> > > reg:
> > > minItems: 1
> > > diff --git
> > > a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > > b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > > index 0d1962980f57..5805a58c55d1 100644
> > > ---
> > > a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> > > +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.ya
> > > +++ ml
> > > @@ -18,6 +18,7 @@ select:
> > > enum:
> > > - starfive,jh7100-dwmac
> > > - starfive,jh7110-dwmac
> > > + - starfive,jh8100-dwmac
> >
> > Same here, even more obvious.
>
> Agreed.

Okay, I will remove it.
>
> >
> > > required:
> > > - compatible
> > >
> > > @@ -30,6 +31,10 @@ properties:
> > > - items:
> > > - const: starfive,jh7110-dwmac
> > > - const: snps,dwmac-5.20
> > > + - items:
> > > + - const: starfive,jh8100-dwmac
> > > + - const: starfive,jh7110-dwmac
> > > + - const: snps,dwmac-5.20
> > >
> > > reg:
> > > maxItems: 1
> > > @@ -116,11 +121,25 @@ allOf:
> > > minItems: 3
> > > maxItems: 3
> > >
> > > - resets:
> > > - minItems: 2
> > > -
> > > - reset-names:
> > > - minItems: 2
> > > + if:
> >
> > I would personally avoid nesting if within if. It gets unreadable.
> > Although Rob did not comment on this one, so I guess it is fine.
>
> I normally agree, but here I suggested it as it looked to be the simplest option.
>
> With the 2 other comments addressed,
>
> Reviewed-by: Rob Herring <[email protected]>