| Message ID | 20230106030001.1952-3-yanhong.wang@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Ethernet driver for StarFive JH7110 SoC | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/tree_selection | fail | Failed to apply to next/pending-fixes or riscv/for-next |
On 06/01/2023 03:59, Yanhong Wang wrote: > Some boards(such as StarFive VisionFive v2) require more than one value > which defined by resets property, so the original definition can not > meet the requirements. In order to adapt to different requirements, > adjust the maxitems number definition. > > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> > --- > .../devicetree/bindings/net/snps,dwmac.yaml | 36 ++++++++++++++----- > 1 file changed, 28 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index e26c3e76ebb7..f7693e8c8d6d 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -132,14 +132,6 @@ properties: > - pclk > - ptp_ref > > - resets: > - maxItems: 1 > - description: > - MAC Reset signal. > - > - reset-names: > - const: stmmaceth Do not remove properties from top-level properties. Instead these should have widest constraints which are further constrain in allOf:if:then. Here you should list items with minItems: 1. > - Best regards, Krzysztof
On 06/01/2023 03:59, Yanhong Wang wrote: > Some boards(such as StarFive VisionFive v2) require more than one value > which defined by resets property, so the original definition can not > meet the requirements. In order to adapt to different requirements, > adjust the maxitems number definition. > > Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> > --- > .../devicetree/bindings/net/snps,dwmac.yaml | 36 ++++++++++++++----- > 1 file changed, 28 insertions(+), 8 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > index e26c3e76ebb7..f7693e8c8d6d 100644 > --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml > +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml > @@ -132,14 +132,6 @@ properties: > - pclk > - ptp_ref > > - resets: > - maxItems: 1 > - description: > - MAC Reset signal. > - > - reset-names: > - const: stmmaceth > - > power-domains: > maxItems: 1 > > @@ -463,6 +455,34 @@ allOf: > Enables the TSO feature otherwise it will be managed by > MAC HW capability register. > > + - if: > + properties: > + compatible: > + contains: > + const: starfive,jh7110-dwmac > + Looking at your next binding patch, this seems a bit clearer. First of all, this patch on itself has little sense. It's not usable on its own, because you need the next one. Probably the snps,dwmac should be just split into common parts used by devices. It makes code much less readable and unnecessary complicated to support in one schema both devices and re-usability. Otherwise I propose to make the resets/reset-names just like clocks are made: define here wide constraints and update all other users of this binding to explicitly restrict resets. Best regards, Krzysztof
On 2023/1/6 20:44, Krzysztof Kozlowski wrote: > On 06/01/2023 03:59, Yanhong Wang wrote: >> Some boards(such as StarFive VisionFive v2) require more than one value >> which defined by resets property, so the original definition can not >> meet the requirements. In order to adapt to different requirements, >> adjust the maxitems number definition. >> >> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >> --- >> .../devicetree/bindings/net/snps,dwmac.yaml | 36 ++++++++++++++----- >> 1 file changed, 28 insertions(+), 8 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >> index e26c3e76ebb7..f7693e8c8d6d 100644 >> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >> @@ -132,14 +132,6 @@ properties: >> - pclk >> - ptp_ref >> >> - resets: >> - maxItems: 1 >> - description: >> - MAC Reset signal. >> - >> - reset-names: >> - const: stmmaceth >> - >> power-domains: >> maxItems: 1 >> >> @@ -463,6 +455,34 @@ allOf: >> Enables the TSO feature otherwise it will be managed by >> MAC HW capability register. >> >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: starfive,jh7110-dwmac >> + > > Looking at your next binding patch, this seems a bit clearer. First of > all, this patch on itself has little sense. It's not usable on its own, > because you need the next one. > > Probably the snps,dwmac should be just split into common parts used by > devices. It makes code much less readable and unnecessary complicated to > support in one schema both devices and re-usability. > > Otherwise I propose to make the resets/reset-names just like clocks are > made: define here wide constraints and update all other users of this > binding to explicitly restrict resets. > > Thanks, refer to the definition of clocks. If it is defined as follows, is it OK? properties: resets: minItems: 1 maxItems: 3 additionalItems: true items: - description: MAC Reset signal. reset-names: minItems: 1 maxItems: 3 additionalItems: true contains: enum: - stmmaceth allOf: - if: properties: compatible: contains: const: starfive,jh7110-dwmac then: properties: resets: minItems: 2 maxItems: 2 reset-names: items: - const: stmmaceth - const: ahb required: - resets - reset-names else: properties: resets: maxItems: 1 description: MAC Reset signal. reset-names: const: stmmaceth Do you have any other better suggestions? > Best regards, > Krzysztof >
On 17/01/2023 07:52, yanhong wang wrote: > > > On 2023/1/6 20:44, Krzysztof Kozlowski wrote: >> On 06/01/2023 03:59, Yanhong Wang wrote: >>> Some boards(such as StarFive VisionFive v2) require more than one value >>> which defined by resets property, so the original definition can not >>> meet the requirements. In order to adapt to different requirements, >>> adjust the maxitems number definition. >>> >>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >>> --- >>> .../devicetree/bindings/net/snps,dwmac.yaml | 36 ++++++++++++++----- >>> 1 file changed, 28 insertions(+), 8 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>> index e26c3e76ebb7..f7693e8c8d6d 100644 >>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>> @@ -132,14 +132,6 @@ properties: >>> - pclk >>> - ptp_ref >>> >>> - resets: >>> - maxItems: 1 >>> - description: >>> - MAC Reset signal. >>> - >>> - reset-names: >>> - const: stmmaceth >>> - >>> power-domains: >>> maxItems: 1 >>> >>> @@ -463,6 +455,34 @@ allOf: >>> Enables the TSO feature otherwise it will be managed by >>> MAC HW capability register. >>> >>> + - if: >>> + properties: >>> + compatible: >>> + contains: >>> + const: starfive,jh7110-dwmac >>> + >> >> Looking at your next binding patch, this seems a bit clearer. First of >> all, this patch on itself has little sense. It's not usable on its own, >> because you need the next one. >> >> Probably the snps,dwmac should be just split into common parts used by >> devices. It makes code much less readable and unnecessary complicated to >> support in one schema both devices and re-usability. >> >> Otherwise I propose to make the resets/reset-names just like clocks are >> made: define here wide constraints and update all other users of this >> binding to explicitly restrict resets. >> >> > > Thanks, refer to the definition of clocks. If it is defined as follows, is it OK? > > properties: > resets: > minItems: 1 > maxItems: 3 > additionalItems: true Drop > items: > - description: MAC Reset signal. Drop both > > reset-names: > minItems: 1 > maxItems: 3 > additionalItems: true Drop > contains: > enum: > - stmmaceth Drop all > > > allOf: > - if: > properties: > compatible: > contains: > const: starfive,jh7110-dwmac > then: > properties: > resets: > minItems: 2 > maxItems: 2 > reset-names: > items: > - const: stmmaceth > - const: ahb > required: > - resets > - reset-names > else: > properties: > resets: > maxItems: 1 > description: > MAC Reset signal. > > reset-names: > const: stmmaceth > > Do you have any other better suggestions? More or less like this but the allOf should not be in snps,dwmac schema but in individual schemas. The snps,dwmac is growing unmaintainable... Best regards, Krzysztof
On 2023/1/17 15:46, Krzysztof Kozlowski wrote: > On 17/01/2023 07:52, yanhong wang wrote: >> >> >> On 2023/1/6 20:44, Krzysztof Kozlowski wrote: >>> On 06/01/2023 03:59, Yanhong Wang wrote: >>>> Some boards(such as StarFive VisionFive v2) require more than one value >>>> which defined by resets property, so the original definition can not >>>> meet the requirements. In order to adapt to different requirements, >>>> adjust the maxitems number definition. >>>> >>>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> >>>> --- >>>> .../devicetree/bindings/net/snps,dwmac.yaml | 36 ++++++++++++++----- >>>> 1 file changed, 28 insertions(+), 8 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> index e26c3e76ebb7..f7693e8c8d6d 100644 >>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml >>>> @@ -132,14 +132,6 @@ properties: >>>> - pclk >>>> - ptp_ref >>>> >>>> - resets: >>>> - maxItems: 1 >>>> - description: >>>> - MAC Reset signal. >>>> - >>>> - reset-names: >>>> - const: stmmaceth >>>> - >>>> power-domains: >>>> maxItems: 1 >>>> >>>> @@ -463,6 +455,34 @@ allOf: >>>> Enables the TSO feature otherwise it will be managed by >>>> MAC HW capability register. >>>> >>>> + - if: >>>> + properties: >>>> + compatible: >>>> + contains: >>>> + const: starfive,jh7110-dwmac >>>> + >>> >>> Looking at your next binding patch, this seems a bit clearer. First of >>> all, this patch on itself has little sense. It's not usable on its own, >>> because you need the next one. >>> >>> Probably the snps,dwmac should be just split into common parts used by >>> devices. It makes code much less readable and unnecessary complicated to >>> support in one schema both devices and re-usability. >>> >>> Otherwise I propose to make the resets/reset-names just like clocks are >>> made: define here wide constraints and update all other users of this >>> binding to explicitly restrict resets. >>> >>> >> >> Thanks, refer to the definition of clocks. If it is defined as follows, is it OK? >> >> properties: >> resets: >> minItems: 1 >> maxItems: 3 >> additionalItems: true > > Drop > >> items: >> - description: MAC Reset signal. > > Drop both > >> >> reset-names: >> minItems: 1 >> maxItems: 3 >> additionalItems: true > > Drop > >> contains: >> enum: >> - stmmaceth > > Drop all > >> >> >> allOf: >> - if: >> properties: >> compatible: >> contains: >> const: starfive,jh7110-dwmac >> then: >> properties: >> resets: >> minItems: 2 >> maxItems: 2 >> reset-names: >> items: >> - const: stmmaceth >> - const: ahb >> required: >> - resets >> - reset-names >> else: >> properties: >> resets: >> maxItems: 1 >> description: >> MAC Reset signal. >> >> reset-names: >> const: stmmaceth >> >> Do you have any other better suggestions? > > More or less like this but the allOf should not be in snps,dwmac schema > but in individual schemas. The snps,dwmac is growing unmaintainable... > Thanks, it is defined as follows, is it right? properties: resets: minItems: 1 maxItems: 3 additionalItems: true reset-names: minItems: 1 maxItems: 3 additionalItems: true > Best regards, > Krzysztof >
On 17/01/2023 09:14, yanhong wang wrote: >>> Thanks, refer to the definition of clocks. If it is defined as follows, is it OK? >>> >>> properties: >>> resets: >>> minItems: 1 >>> maxItems: 3 >>> additionalItems: true >> >> Drop >> >>> items: >>> - description: MAC Reset signal. >> >> Drop both >> >>> >>> reset-names: >>> minItems: 1 >>> maxItems: 3 >>> additionalItems: true >> >> Drop >> >>> contains: >>> enum: >>> - stmmaceth >> >> Drop all >> >>> >>> >>> allOf: >>> - if: >>> properties: >>> compatible: >>> contains: >>> const: starfive,jh7110-dwmac >>> then: >>> properties: >>> resets: >>> minItems: 2 >>> maxItems: 2 >>> reset-names: >>> items: >>> - const: stmmaceth >>> - const: ahb >>> required: >>> - resets >>> - reset-names >>> else: >>> properties: >>> resets: >>> maxItems: 1 >>> description: >>> MAC Reset signal. >>> >>> reset-names: >>> const: stmmaceth >>> >>> Do you have any other better suggestions? >> >> More or less like this but the allOf should not be in snps,dwmac schema >> but in individual schemas. The snps,dwmac is growing unmaintainable... >> > > Thanks, it is defined as follows, is it right? > > properties: > resets: > minItems: 1 > maxItems: 3 > additionalItems: true > Read my comments above. Drop. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index e26c3e76ebb7..f7693e8c8d6d 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml @@ -132,14 +132,6 @@ properties: - pclk - ptp_ref - resets: - maxItems: 1 - description: - MAC Reset signal. - - reset-names: - const: stmmaceth - power-domains: maxItems: 1 @@ -463,6 +455,34 @@ allOf: Enables the TSO feature otherwise it will be managed by MAC HW capability register. + - if: + properties: + compatible: + contains: + const: starfive,jh7110-dwmac + + then: + properties: + resets: + minItems: 2 + maxItems: 2 + reset-names: + items: + - const: stmmaceth + - const: ahb + required: + - resets + - reset-names + else: + properties: + resets: + maxItems: 1 + description: + MAC Reset signal. + + reset-names: + const: stmmaceth + additionalProperties: true examples:
Some boards(such as StarFive VisionFive v2) require more than one value which defined by resets property, so the original definition can not meet the requirements. In order to adapt to different requirements, adjust the maxitems number definition. Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com> --- .../devicetree/bindings/net/snps,dwmac.yaml | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-)