| Message ID | 20240429210131.373487-9-eajames@linux.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ARM: dts: aspeed: Add IBM P11 BMC Boards | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 1 warnings, 81 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 29/04/2024 23:01, Eddie James wrote: > Convert to json-schema for the AST2600 FSI master documentation. Please mention all the changes from pure conversion. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > Changes since v3: > - Remove quotes around compatible strings > - Re-order allOf to below required > - Add child node in the example > - Change commit message to match similar commits > > .../fsi/aspeed,ast2600-fsi-master.yaml | 81 +++++++++++++++++++ > .../bindings/fsi/fsi-master-aspeed.txt | 36 --------- > 2 files changed, 81 insertions(+), 36 deletions(-) > create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml > delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt > > diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml > new file mode 100644 > index 000000000000..fcf7c4b93b78 > --- /dev/null > +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml > @@ -0,0 +1,81 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Aspeed FSI master > + > +maintainers: > + - Eddie James <eajames@linux.ibm.com> > + > +description: > + The AST2600 and later contain two identical FSI masters. They share a > + clock and have a separate interrupt line and output pins. > + > +properties: > + compatible: > + enum: > + - aspeed,ast2600-fsi-master > + - aspeed,ast2700-fsi-master There was no such compatible before. How does this even validate? Where is fsi-master? You dropped a compatible without any explanation. > + > + reg: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + Best regards, Krzysztof
On 4/30/24 02:04, Krzysztof Kozlowski wrote: > On 29/04/2024 23:01, Eddie James wrote: >> Convert to json-schema for the AST2600 FSI master documentation. > Please mention all the changes from pure conversion. Sure. > >> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> --- >> Changes since v3: >> - Remove quotes around compatible strings >> - Re-order allOf to below required >> - Add child node in the example >> - Change commit message to match similar commits >> >> .../fsi/aspeed,ast2600-fsi-master.yaml | 81 +++++++++++++++++++ >> .../bindings/fsi/fsi-master-aspeed.txt | 36 --------- >> 2 files changed, 81 insertions(+), 36 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >> delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt >> >> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >> new file mode 100644 >> index 000000000000..fcf7c4b93b78 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >> @@ -0,0 +1,81 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Aspeed FSI master >> + >> +maintainers: >> + - Eddie James <eajames@linux.ibm.com> >> + >> +description: >> + The AST2600 and later contain two identical FSI masters. They share a >> + clock and have a separate interrupt line and output pins. >> + >> +properties: >> + compatible: >> + enum: >> + - aspeed,ast2600-fsi-master >> + - aspeed,ast2700-fsi-master > There was no such compatible before. > > How does this even validate? Where is fsi-master? You dropped a > compatible without any explanation. I can make it a separate change to add ast2700. I suppose I don't understand having two compatibles... Aspeed master shouldn't use "fsi-master" as that is too generic, right? Why wouldn't it validate? Devicetrees using "fsi-master" also use "aspeed,ast2600-fsi-master" so they should be OK... > >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + > > Best regards, > Krzysztof >
On 01/05/2024 18:12, Eddie James wrote: > > On 4/30/24 02:04, Krzysztof Kozlowski wrote: >> On 29/04/2024 23:01, Eddie James wrote: >>> Convert to json-schema for the AST2600 FSI master documentation. >> Please mention all the changes from pure conversion. > > > Sure. > > >> >>> Signed-off-by: Eddie James <eajames@linux.ibm.com> >>> --- >>> Changes since v3: >>> - Remove quotes around compatible strings >>> - Re-order allOf to below required >>> - Add child node in the example >>> - Change commit message to match similar commits >>> >>> .../fsi/aspeed,ast2600-fsi-master.yaml | 81 +++++++++++++++++++ >>> .../bindings/fsi/fsi-master-aspeed.txt | 36 --------- >>> 2 files changed, 81 insertions(+), 36 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >>> delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt >>> >>> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >>> new file mode 100644 >>> index 000000000000..fcf7c4b93b78 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >>> @@ -0,0 +1,81 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Aspeed FSI master >>> + >>> +maintainers: >>> + - Eddie James <eajames@linux.ibm.com> >>> + >>> +description: >>> + The AST2600 and later contain two identical FSI masters. They share a >>> + clock and have a separate interrupt line and output pins. >>> + >>> +properties: >>> + compatible: >>> + enum: >>> + - aspeed,ast2600-fsi-master >>> + - aspeed,ast2700-fsi-master >> There was no such compatible before. >> >> How does this even validate? Where is fsi-master? You dropped a >> compatible without any explanation. > > > I can make it a separate change to add ast2700. > > > I suppose I don't understand having two compatibles... Aspeed master > shouldn't use "fsi-master" as that is too generic, right? Why wouldn't Not necessarily, depends. Dropping it silently is confusing. What about other users? firmware, bootloaders, out-of-tree, other OS? Did you investigate all of them? > it validate? Devicetrees using "fsi-master" also use > "aspeed,ast2600-fsi-master" so they should be OK... No, because the compatibles do not match. Run validation and you will see the errors. I am fine with dropping such compatible, which is not used by current kernel ABI, but first DTS must be fixed and second some explanation and justification is needed. Best regards, Krzysztof
On 5/4/24 06:55, Krzysztof Kozlowski wrote: > On 01/05/2024 18:12, Eddie James wrote: >> On 4/30/24 02:04, Krzysztof Kozlowski wrote: >>> On 29/04/2024 23:01, Eddie James wrote: >>>> Convert to json-schema for the AST2600 FSI master documentation. >>> Please mention all the changes from pure conversion. >> >> Sure. >> >> >>>> Signed-off-by: Eddie James <eajames@linux.ibm.com> >>>> --- >>>> Changes since v3: >>>> - Remove quotes around compatible strings >>>> - Re-order allOf to below required >>>> - Add child node in the example >>>> - Change commit message to match similar commits >>>> >>>> .../fsi/aspeed,ast2600-fsi-master.yaml | 81 +++++++++++++++++++ >>>> .../bindings/fsi/fsi-master-aspeed.txt | 36 --------- >>>> 2 files changed, 81 insertions(+), 36 deletions(-) >>>> create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >>>> delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt >>>> >>>> diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >>>> new file mode 100644 >>>> index 000000000000..fcf7c4b93b78 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml >>>> @@ -0,0 +1,81 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Aspeed FSI master >>>> + >>>> +maintainers: >>>> + - Eddie James <eajames@linux.ibm.com> >>>> + >>>> +description: >>>> + The AST2600 and later contain two identical FSI masters. They share a >>>> + clock and have a separate interrupt line and output pins. >>>> + >>>> +properties: >>>> + compatible: >>>> + enum: >>>> + - aspeed,ast2600-fsi-master >>>> + - aspeed,ast2700-fsi-master >>> There was no such compatible before. >>> >>> How does this even validate? Where is fsi-master? You dropped a >>> compatible without any explanation. >> >> I can make it a separate change to add ast2700. >> >> >> I suppose I don't understand having two compatibles... Aspeed master >> shouldn't use "fsi-master" as that is too generic, right? Why wouldn't > Not necessarily, depends. Dropping it silently is confusing. What about > other users? firmware, bootloaders, out-of-tree, other OS? Did you > investigate all of them? The old format file actually only used fsi-master in the example, not in the actual properties description. So I didn't really drop a compatible. Device trees using "fsi-master" are just buggy and should be fixed eventually, but I don't think it needs to be part of this series. Thanks, Eddie > >> it validate? Devicetrees using "fsi-master" also use >> "aspeed,ast2600-fsi-master" so they should be OK... > No, because the compatibles do not match. Run validation and you will > see the errors. > > I am fine with dropping such compatible, which is not used by current > kernel ABI, but first DTS must be fixed and second some explanation and > justification is needed. > > Best regards, > Krzysztof >
diff --git a/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml new file mode 100644 index 000000000000..fcf7c4b93b78 --- /dev/null +++ b/Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml @@ -0,0 +1,81 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/fsi/aspeed,ast2600-fsi-master.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Aspeed FSI master + +maintainers: + - Eddie James <eajames@linux.ibm.com> + +description: + The AST2600 and later contain two identical FSI masters. They share a + clock and have a separate interrupt line and output pins. + +properties: + compatible: + enum: + - aspeed,ast2600-fsi-master + - aspeed,ast2700-fsi-master + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + cfam-reset-gpios: + maxItems: 1 + description: + Output GPIO pin for CFAM reset + + fsi-routing-gpios: + maxItems: 1 + description: + Output GPIO pin for setting the FSI mux (internal or cabled) + + fsi-mux-gpios: + maxItems: 1 + description: + Input GPIO pin for detecting the desired FSI mux state + + interrupts: + maxItems: 1 + +required: + - compatible + - reg + - clocks + - interrupts + +allOf: + - $ref: fsi-controller.yaml# + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/ast2600-clock.h> + #include <dt-bindings/gpio/aspeed-gpio.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + fsi-master@1e79b000 { + compatible = "aspeed,ast2600-fsi-master"; + reg = <0x1e79b000 0x94>; + #address-cells = <2>; + #size-cells = <0>; + interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>; + pinctrl-names = "default"; + pinctrl-0 = <&pinctrl_fsi1_default>; + clocks = <&syscon ASPEED_CLK_GATE_FSICLK>; + fsi-routing-gpios = <&gpio0 ASPEED_GPIO(Q, 7) GPIO_ACTIVE_HIGH>; + fsi-mux-gpios = <&gpio0 ASPEED_GPIO(B, 0) GPIO_ACTIVE_HIGH>; + cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_LOW>; + + cfam@0,0 { + reg = <0 0>; + #address-cells = <1>; + #size-cells = <1>; + chip-id = <0>; + }; + }; diff --git a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt b/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt deleted file mode 100644 index 9853fefff5d8..000000000000 --- a/Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt +++ /dev/null @@ -1,36 +0,0 @@ -Device-tree bindings for AST2600 FSI master -------------------------------------------- - -The AST2600 contains two identical FSI masters. They share a clock and have a -separate interrupt line and output pins. - -Required properties: - - compatible: "aspeed,ast2600-fsi-master" - - reg: base address and length - - clocks: phandle and clock number - - interrupts: platform dependent interrupt description - - pinctrl-0: phandle to pinctrl node - - pinctrl-names: pinctrl state - -Optional properties: - - cfam-reset-gpios: GPIO for CFAM reset - - - fsi-routing-gpios: GPIO for setting the FSI mux (internal or cabled) - - fsi-mux-gpios: GPIO for detecting the desired FSI mux state - - -Examples: - - fsi-master { - compatible = "aspeed,ast2600-fsi-master", "fsi-master"; - reg = <0x1e79b000 0x94>; - interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>; - pinctrl-names = "default"; - pinctrl-0 = <&pinctrl_fsi1_default>; - clocks = <&syscon ASPEED_CLK_GATE_FSICLK>; - - fsi-routing-gpios = <&gpio0 ASPEED_GPIO(Q, 7) GPIO_ACTIVE_HIGH>; - fsi-mux-gpios = <&gpio0 ASPEED_GPIO(B, 0) GPIO_ACTIVE_HIGH>; - - cfam-reset-gpios = <&gpio0 ASPEED_GPIO(Q, 0) GPIO_ACTIVE_LOW>; - };
Convert to json-schema for the AST2600 FSI master documentation. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- Changes since v3: - Remove quotes around compatible strings - Re-order allOf to below required - Add child node in the example - Change commit message to match similar commits .../fsi/aspeed,ast2600-fsi-master.yaml | 81 +++++++++++++++++++ .../bindings/fsi/fsi-master-aspeed.txt | 36 --------- 2 files changed, 81 insertions(+), 36 deletions(-) create mode 100644 Documentation/devicetree/bindings/fsi/aspeed,ast2600-fsi-master.yaml delete mode 100644 Documentation/devicetree/bindings/fsi/fsi-master-aspeed.txt