* Re: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
@ 2024-03-25 16:22 ` Rob Herring
0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2024-03-25 16:22 UTC (permalink / raw)
To: Tan Chun Hau
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Simon Horman,
Bartosz Golaszewski, Andrew Halaney, Jisheng Zhang,
Uwe Kleine-König, Russell King, Ley Foon Tan, Jee Heng Sia,
netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel,
linux-riscv
On Mon, Mar 25, 2024 at 01:51:31AM -0700, 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";
It's debatable whether JH8100 is compatible with JH7110 if the 2nd reset
was not optional. I guess if the Linux driver treated it that way, we
can get away with it. It would simplify the conditionals in the schema
if the schema also treated the 2nd entry as optional on JH7110 as well.
> 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 <chunhau.tan@starfivetech.com>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,jh7110-dwmac.yaml | 82 +++++++++++++------
> 2 files changed, 58 insertions(+), 25 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..da3cc984fec9 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
> @@ -83,29 +88,13 @@ allOf:
> - if:
> properties:
> compatible:
> - contains:
> - const: starfive,jh7100-dwmac
> - then:
> - properties:
> - interrupts:
> - minItems: 2
> - maxItems: 2
> -
> - interrupt-names:
> - minItems: 2
> - maxItems: 2
> -
> - resets:
> - maxItems: 1
> -
> - reset-names:
> - const: ahb
> -
> - - if:
> - properties:
> - compatible:
> - contains:
> - const: starfive,jh7110-dwmac
> + allOf:
> + - contains:
> + enum:
> + - starfive,jh8100-dwmac
> + - contains:
> + enum:
> + - starfive,jh7110-dwmac
There's no need for the 2nd entry. You just need to check
I would something like this structure:
- if:
properties:
compatible:
contains:
const: starfive,jh7100-dwmac
then:
if:
properties:
compatible:
contains:
const: starfive,jh8100-dwmac
then:
...
else:
...
> then:
> properties:
> interrupts:
> @@ -117,10 +106,53 @@ allOf:
> maxItems: 3
>
> resets:
> - minItems: 2
> + maxItems: 1
>
> reset-names:
> - minItems: 2
> + const: stmmaceth
> +
> + else:
I don't think you need the else. Just do another 'if' entry.
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7100-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 2
> + maxItems: 2
> +
> + interrupt-names:
> + minItems: 2
> + maxItems: 2
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: ahb
> +
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7110-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 3
> + maxItems: 3
> +
> + interrupt-names:
> + minItems: 3
> + maxItems: 3
> +
> + resets:
> + minItems: 2
> +
> + reset-names:
> + minItems: 2
>
> unevaluatedProperties: false
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
@ 2024-03-25 16:22 ` Rob Herring
0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2024-03-25 16:22 UTC (permalink / raw)
To: Tan Chun Hau
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Simon Horman,
Bartosz Golaszewski, Andrew Halaney, Jisheng Zhang,
Uwe Kleine-König, Russell King, Ley Foon Tan, Jee Heng Sia,
netdev, devicetree, linux-kernel, linux-stm32, linux-arm-kernel,
linux-riscv
On Mon, Mar 25, 2024 at 01:51:31AM -0700, 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";
It's debatable whether JH8100 is compatible with JH7110 if the 2nd reset
was not optional. I guess if the Linux driver treated it that way, we
can get away with it. It would simplify the conditionals in the schema
if the schema also treated the 2nd entry as optional on JH7110 as well.
> 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 <chunhau.tan@starfivetech.com>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,jh7110-dwmac.yaml | 82 +++++++++++++------
> 2 files changed, 58 insertions(+), 25 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..da3cc984fec9 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
> @@ -83,29 +88,13 @@ allOf:
> - if:
> properties:
> compatible:
> - contains:
> - const: starfive,jh7100-dwmac
> - then:
> - properties:
> - interrupts:
> - minItems: 2
> - maxItems: 2
> -
> - interrupt-names:
> - minItems: 2
> - maxItems: 2
> -
> - resets:
> - maxItems: 1
> -
> - reset-names:
> - const: ahb
> -
> - - if:
> - properties:
> - compatible:
> - contains:
> - const: starfive,jh7110-dwmac
> + allOf:
> + - contains:
> + enum:
> + - starfive,jh8100-dwmac
> + - contains:
> + enum:
> + - starfive,jh7110-dwmac
There's no need for the 2nd entry. You just need to check
I would something like this structure:
- if:
properties:
compatible:
contains:
const: starfive,jh7100-dwmac
then:
if:
properties:
compatible:
contains:
const: starfive,jh8100-dwmac
then:
...
else:
...
> then:
> properties:
> interrupts:
> @@ -117,10 +106,53 @@ allOf:
> maxItems: 3
>
> resets:
> - minItems: 2
> + maxItems: 1
>
> reset-names:
> - minItems: 2
> + const: stmmaceth
> +
> + else:
I don't think you need the else. Just do another 'if' entry.
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7100-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 2
> + maxItems: 2
> +
> + interrupt-names:
> + minItems: 2
> + maxItems: 2
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: ahb
> +
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh7110-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 3
> + maxItems: 3
> +
> + interrupt-names:
> + minItems: 3
> + maxItems: 3
> +
> + resets:
> + minItems: 2
> +
> + reset-names:
> + minItems: 2
>
> unevaluatedProperties: false
>
> --
> 2.25.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
2024-03-25 16:22 ` Rob Herring
(?)
@ 2024-03-26 5:24 ` ChunHau Tan
-1 siblings, 0 replies; 12+ messages in thread
From: ChunHau Tan @ 2024-03-26 5:24 UTC (permalink / raw)
To: Rob Herring
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Simon Horman,
Bartosz Golaszewski, Andrew Halaney, Jisheng Zhang,
Uwe Kleine-König, Russell King, Leyfoon Tan, JeeHeng Sia,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-riscv@lists.infradead.org
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, 26 March, 2024 12:23 AM
> To: ChunHau Tan <chunhau.tan@starfivetech.com>
> Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Emil Renner Berthing <kernel@esmil.dk>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Alexandre Torgue <alexandre.torgue@foss.st.com>; Simon Horman
> <horms@kernel.org>; Bartosz Golaszewski <bartosz.golaszewski@linaro.org>;
> Andrew Halaney <ahalaney@redhat.com>; Jisheng Zhang <jszhang@kernel.org>;
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de>; Russell King
> <rmk+kernel@armlinux.org.uk>; Leyfoon Tan <leyfoon.tan@starfivetech.com>;
> JeeHeng Sia <jeeheng.sia@starfivetech.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; linux-riscv@lists.infradead.org
> Subject: Re: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add
> StarFive JH8100 support
>
> On Mon, Mar 25, 2024 at 01:51:31AM -0700, 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";
>
> It's debatable whether JH8100 is compatible with JH7110 if the 2nd reset was
> not optional. I guess if the Linux driver treated it that way, we can get away with
> it. It would simplify the conditionals in the schema if the t also treated the
> 2nd entry as optional on JH7110 as well.
Yes, Linux driver and snps,dwmac.yaml treated it that way.
please refer to
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/snps%2Cdwmac.yaml#L143
>
> > 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 <chunhau.tan@starfivetech.com>
> > ---
> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > .../bindings/net/starfive,jh7110-dwmac.yaml | 82 +++++++++++++------
> > 2 files changed, 58 insertions(+), 25 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..da3cc984fec9 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
> > @@ -83,29 +88,13 @@ allOf:
> > - if:
> > properties:
> > compatible:
> > - contains:
> > - const: starfive,jh7100-dwmac
> > - then:
> > - properties:
> > - interrupts:
> > - minItems: 2
> > - maxItems: 2
> > -
> > - interrupt-names:
> > - minItems: 2
> > - maxItems: 2
> > -
> > - resets:
> > - maxItems: 1
> > -
> > - reset-names:
> > - const: ahb
> > -
> > - - if:
> > - properties:
> > - compatible:
> > - contains:
> > - const: starfive,jh7110-dwmac
> > + allOf:
> > + - contains:
> > + enum:
> > + - starfive,jh8100-dwmac
> > + - contains:
> > + enum:
> > + - starfive,jh7110-dwmac
>
> There's no need for the 2nd entry. You just need to check
>
> I would something like this structure:
>
> - if:
> properties:
> compatible:
> contains:
> const: starfive,jh7100-dwmac
>
> then:
>
> if:
> properties:
> compatible:
> contains:
> const: starfive,jh8100-dwmac
> then:
> ...
> else:
> ...
>
Okay, thank you for the feedback.
> > then:
> > properties:
> > interrupts:
> > @@ -117,10 +106,53 @@ allOf:
> > maxItems: 3
> >
> > resets:
> > - minItems: 2
> > + maxItems: 1
> >
> > reset-names:
> > - minItems: 2
> > + const: stmmaceth
> > +
> > + else:
>
> I don't think you need the else. Just do another 'if' entry.
>
> > + if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7100-dwmac
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 2
> > + maxItems: 2
> > +
> > + interrupt-names:
> > + minItems: 2
> > + maxItems: 2
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + reset-names:
> > + const: ahb
> > +
> > + if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7110-dwmac
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 3
> > + maxItems: 3
> > +
> > + interrupt-names:
> > + minItems: 3
> > + maxItems: 3
> > +
> > + resets:
> > + minItems: 2
> > +
> > + reset-names:
> > + minItems: 2
> >
> > unevaluatedProperties: false
> >
> > --
> > 2.25.1
> >
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
@ 2024-03-26 5:24 ` ChunHau Tan
0 siblings, 0 replies; 12+ messages in thread
From: ChunHau Tan @ 2024-03-26 5:24 UTC (permalink / raw)
To: Rob Herring
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Simon Horman,
Bartosz Golaszewski, Andrew Halaney, Jisheng Zhang,
Uwe Kleine-König, Russell King, Leyfoon Tan, JeeHeng Sia,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-riscv@lists.infradead.org
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, 26 March, 2024 12:23 AM
> To: ChunHau Tan <chunhau.tan@starfivetech.com>
> Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Emil Renner Berthing <kernel@esmil.dk>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Alexandre Torgue <alexandre.torgue@foss.st.com>; Simon Horman
> <horms@kernel.org>; Bartosz Golaszewski <bartosz.golaszewski@linaro.org>;
> Andrew Halaney <ahalaney@redhat.com>; Jisheng Zhang <jszhang@kernel.org>;
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de>; Russell King
> <rmk+kernel@armlinux.org.uk>; Leyfoon Tan <leyfoon.tan@starfivetech.com>;
> JeeHeng Sia <jeeheng.sia@starfivetech.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; linux-riscv@lists.infradead.org
> Subject: Re: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add
> StarFive JH8100 support
>
> On Mon, Mar 25, 2024 at 01:51:31AM -0700, 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";
>
> It's debatable whether JH8100 is compatible with JH7110 if the 2nd reset was
> not optional. I guess if the Linux driver treated it that way, we can get away with
> it. It would simplify the conditionals in the schema if the t also treated the
> 2nd entry as optional on JH7110 as well.
Yes, Linux driver and snps,dwmac.yaml treated it that way.
please refer to
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/snps%2Cdwmac.yaml#L143
>
> > 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 <chunhau.tan@starfivetech.com>
> > ---
> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > .../bindings/net/starfive,jh7110-dwmac.yaml | 82 +++++++++++++------
> > 2 files changed, 58 insertions(+), 25 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..da3cc984fec9 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
> > @@ -83,29 +88,13 @@ allOf:
> > - if:
> > properties:
> > compatible:
> > - contains:
> > - const: starfive,jh7100-dwmac
> > - then:
> > - properties:
> > - interrupts:
> > - minItems: 2
> > - maxItems: 2
> > -
> > - interrupt-names:
> > - minItems: 2
> > - maxItems: 2
> > -
> > - resets:
> > - maxItems: 1
> > -
> > - reset-names:
> > - const: ahb
> > -
> > - - if:
> > - properties:
> > - compatible:
> > - contains:
> > - const: starfive,jh7110-dwmac
> > + allOf:
> > + - contains:
> > + enum:
> > + - starfive,jh8100-dwmac
> > + - contains:
> > + enum:
> > + - starfive,jh7110-dwmac
>
> There's no need for the 2nd entry. You just need to check
>
> I would something like this structure:
>
> - if:
> properties:
> compatible:
> contains:
> const: starfive,jh7100-dwmac
>
> then:
>
> if:
> properties:
> compatible:
> contains:
> const: starfive,jh8100-dwmac
> then:
> ...
> else:
> ...
>
Okay, thank you for the feedback.
> > then:
> > properties:
> > interrupts:
> > @@ -117,10 +106,53 @@ allOf:
> > maxItems: 3
> >
> > resets:
> > - minItems: 2
> > + maxItems: 1
> >
> > reset-names:
> > - minItems: 2
> > + const: stmmaceth
> > +
> > + else:
>
> I don't think you need the else. Just do another 'if' entry.
>
> > + if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7100-dwmac
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 2
> > + maxItems: 2
> > +
> > + interrupt-names:
> > + minItems: 2
> > + maxItems: 2
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + reset-names:
> > + const: ahb
> > +
> > + if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7110-dwmac
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 3
> > + maxItems: 3
> > +
> > + interrupt-names:
> > + minItems: 3
> > + maxItems: 3
> > +
> > + resets:
> > + minItems: 2
> > +
> > + reset-names:
> > + minItems: 2
> >
> > unevaluatedProperties: false
> >
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
@ 2024-03-26 5:24 ` ChunHau Tan
0 siblings, 0 replies; 12+ messages in thread
From: ChunHau Tan @ 2024-03-26 5:24 UTC (permalink / raw)
To: Rob Herring
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, Krzysztof Kozlowski, Conor Dooley,
Maxime Coquelin, Alexandre Torgue, Simon Horman,
Bartosz Golaszewski, Andrew Halaney, Jisheng Zhang,
Uwe Kleine-König, Russell King, Leyfoon Tan, JeeHeng Sia,
netdev@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-stm32@st-md-mailman.stormreply.com,
linux-arm-kernel@lists.infradead.org,
linux-riscv@lists.infradead.org
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Tuesday, 26 March, 2024 12:23 AM
> To: ChunHau Tan <chunhau.tan@starfivetech.com>
> Cc: David S . Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Emil Renner Berthing <kernel@esmil.dk>; Krzysztof
> Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley
> <conor+dt@kernel.org>; Maxime Coquelin <mcoquelin.stm32@gmail.com>;
> Alexandre Torgue <alexandre.torgue@foss.st.com>; Simon Horman
> <horms@kernel.org>; Bartosz Golaszewski <bartosz.golaszewski@linaro.org>;
> Andrew Halaney <ahalaney@redhat.com>; Jisheng Zhang <jszhang@kernel.org>;
> Uwe Kleine-König <u.kleine-koenig@pengutronix.de>; Russell King
> <rmk+kernel@armlinux.org.uk>; Leyfoon Tan <leyfoon.tan@starfivetech.com>;
> JeeHeng Sia <jeeheng.sia@starfivetech.com>; netdev@vger.kernel.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org;
> linux-stm32@st-md-mailman.stormreply.com;
> linux-arm-kernel@lists.infradead.org; linux-riscv@lists.infradead.org
> Subject: Re: [PATCH v3 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add
> StarFive JH8100 support
>
> On Mon, Mar 25, 2024 at 01:51:31AM -0700, 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";
>
> It's debatable whether JH8100 is compatible with JH7110 if the 2nd reset was
> not optional. I guess if the Linux driver treated it that way, we can get away with
> it. It would simplify the conditionals in the schema if the t also treated the
> 2nd entry as optional on JH7110 as well.
Yes, Linux driver and snps,dwmac.yaml treated it that way.
please refer to
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/net/snps%2Cdwmac.yaml#L143
>
> > 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 <chunhau.tan@starfivetech.com>
> > ---
> > .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> > .../bindings/net/starfive,jh7110-dwmac.yaml | 82 +++++++++++++------
> > 2 files changed, 58 insertions(+), 25 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..da3cc984fec9 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
> > @@ -83,29 +88,13 @@ allOf:
> > - if:
> > properties:
> > compatible:
> > - contains:
> > - const: starfive,jh7100-dwmac
> > - then:
> > - properties:
> > - interrupts:
> > - minItems: 2
> > - maxItems: 2
> > -
> > - interrupt-names:
> > - minItems: 2
> > - maxItems: 2
> > -
> > - resets:
> > - maxItems: 1
> > -
> > - reset-names:
> > - const: ahb
> > -
> > - - if:
> > - properties:
> > - compatible:
> > - contains:
> > - const: starfive,jh7110-dwmac
> > + allOf:
> > + - contains:
> > + enum:
> > + - starfive,jh8100-dwmac
> > + - contains:
> > + enum:
> > + - starfive,jh7110-dwmac
>
> There's no need for the 2nd entry. You just need to check
>
> I would something like this structure:
>
> - if:
> properties:
> compatible:
> contains:
> const: starfive,jh7100-dwmac
>
> then:
>
> if:
> properties:
> compatible:
> contains:
> const: starfive,jh8100-dwmac
> then:
> ...
> else:
> ...
>
Okay, thank you for the feedback.
> > then:
> > properties:
> > interrupts:
> > @@ -117,10 +106,53 @@ allOf:
> > maxItems: 3
> >
> > resets:
> > - minItems: 2
> > + maxItems: 1
> >
> > reset-names:
> > - minItems: 2
> > + const: stmmaceth
> > +
> > + else:
>
> I don't think you need the else. Just do another 'if' entry.
>
> > + if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7100-dwmac
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 2
> > + maxItems: 2
> > +
> > + interrupt-names:
> > + minItems: 2
> > + maxItems: 2
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + reset-names:
> > + const: ahb
> > +
> > + if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: starfive,jh7110-dwmac
> > + then:
> > + properties:
> > + interrupts:
> > + minItems: 3
> > + maxItems: 3
> > +
> > + interrupt-names:
> > + minItems: 3
> > + maxItems: 3
> > +
> > + resets:
> > + minItems: 2
> > +
> > + reset-names:
> > + minItems: 2
> >
> > unevaluatedProperties: false
> >
> > --
> > 2.25.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread