| Message ID | 20240429210131.373487-1-eajames@linux.ibm.com |
|---|---|
| Headers | show |
| Series | ARM: dts: aspeed: Add IBM P11 BMC Boards | expand |
On 29/04/2024 23:01, Eddie James wrote: > IBM Power processors have a SPI controller that can be accessed > over FSI from a service processor. Document it. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- This is an automated instruction, just in case, because many review tags are being ignored. If you know the process, you can skip it (please do not feel offended by me posting it here - no bad intentions intended). If you do not know the process, here is a short explanation: Please add Acked-by/Reviewed-by/Tested-by tags when posting new versions, under or above your Signed-off-by tag. Tag is "received", when provided in a message replied to you on the mailing list. Tools like b4 can help here. However, there's no need to repost patches *only* to add the tags. The upstream maintainer will do that for tags received on the version they apply. https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577 Best regards, Krzysztof
On 29/04/2024 23:01, Eddie James wrote: > + > +patternProperties: > + "cfam@[0-9a-f],[0-9a-f]": > + type: object > + properties: > + chip-id: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: > + Processor index fsi.txt tells a bit more about it, so extend the description. > + > + reg: > + maxItems: 1 > + > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 1 > + > + required: > + - reg > + > + additionalProperties: true > + > +additionalProperties: true > + > +examples: > + - | > + fsi { > + #address-cells = <2>; > + #size-cells = <0>; > + > + cfam@0,0 { > + reg = <0 0>; > + #address-cells = <1>; > + #size-cells = <1>; > + chip-id = <0>; > + }; > + }; Drop the example, it's not being validated/used. Best regards, Krzysztof
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:01, Krzysztof Kozlowski wrote: > On 29/04/2024 23:01, Eddie James wrote: >> + >> +patternProperties: >> + "cfam@[0-9a-f],[0-9a-f]": >> + type: object >> + properties: >> + chip-id: >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + description: >> + Processor index > fsi.txt tells a bit more about it, so extend the description. Ack. Thanks, Eddie > > >> + >> + reg: >> + maxItems: 1 >> + >> + "#address-cells": >> + const: 1 >> + >> + "#size-cells": >> + const: 1 >> + >> + required: >> + - reg >> + >> + additionalProperties: true >> + >> +additionalProperties: true >> + >> +examples: >> + - | >> + fsi { >> + #address-cells = <2>; >> + #size-cells = <0>; >> + >> + cfam@0,0 { >> + reg = <0 0>; >> + #address-cells = <1>; >> + #size-cells = <1>; >> + chip-id = <0>; >> + }; >> + }; > Drop the example, it's not being validated/used. Ack. > > 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 >