> -----Original Message----- > From: Rob Herring <robh@xxxxxxxxxx> > Sent: Tuesday, 26 March, 2024 12:23 AM > To: ChunHau Tan <chunhau.tan@xxxxxxxxxxxxxxxx> > Cc: David S . Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet > <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni > <pabeni@xxxxxxxxxx>; Emil Renner Berthing <kernel@xxxxxxxx>; Krzysztof > Kozlowski <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley > <conor+dt@xxxxxxxxxx>; Maxime Coquelin <mcoquelin.stm32@xxxxxxxxx>; > Alexandre Torgue <alexandre.torgue@xxxxxxxxxxx>; Simon Horman > <horms@xxxxxxxxxx>; Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>; > Andrew Halaney <ahalaney@xxxxxxxxxx>; Jisheng Zhang <jszhang@xxxxxxxxxx>; > Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>; Russell King > <rmk+kernel@xxxxxxxxxxxxxxx>; Leyfoon Tan <leyfoon.tan@xxxxxxxxxxxxxxxx>; > JeeHeng Sia <jeeheng.sia@xxxxxxxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; > devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; > linux-stm32@xxxxxxxxxxxxxxxxxxxxxxxxxxxx; > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-riscv@xxxxxxxxxxxxxxxxxxx > 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@xxxxxxxxxxxxxxxx> > > --- > > .../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 > >
