| Message ID | 20211108150554.4457-11-conor.dooley@microchip.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Update the icicle kit device tree | expand |
On 08/11/2021 16:05, conor.dooley@microchip.com wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Add device tree bindings for the {q,}spi controller on > the Microchip PolarFire SoC. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > > diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > new file mode 100644 > index 000000000000..efed145ad029 > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/spi/microchip,mpfs-spi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip MPFS {Q,}SPI Controller Device Tree Bindings > + > +maintainers: > + - Conor Dooley <conor.dooley@microchip.com> > + > +description: | > + This {Q,}SPI controller is found on the Microchip PolarFire SoC. > + > +allOf: > + - $ref: "spi-controller.yaml#" No need for quotes. > + > +properties: > + compatible: > + enum: > + - microchip,mpfs-spi > + - microsemi,ms-pf-mss-spi > + - microchip,mpfs-qspi > + - microsemi,ms-pf-mss-qspi > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clock-names: > + maxItems: 1 > + > + clocks: > + maxItems: 2 This does not match clock-names. Describe clocks instead. Are you really sure your hardware can have an optional second clock? > + > + num-cs: > + description: | > + Number of chip selects used. > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 1 > + maximum: 8 > + default: 8 > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +unevaluatedProperties: false > + > +examples: > + - | > + #include "dt-bindings/clock/microchip,mpfs-clock.h" > + #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h" > + soc { > + #address-cells = <2>; > + #size-cells = <2>; > + spi0: spi@20108000 { > + compatible = "microchip,mpfs-spi"; > + reg = <0x0 0x20108000 0x0 0x1000>; > + clocks = <&clkcfg CLK_SPI0>; > + interrupt-parent = <&plic>; > + interrupts = <PLIC_INT_SPI0>; > + spi-max-frequency = <25000000>; > + num-cs = <8>; > + status = "disabled"; > + }; > + }; > +... > Best regards, Krzysztof
On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Add device tree bindings for the {q,}spi controller on > the Microchip PolarFire SoC. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> > --- > .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ > 1 file changed, 72 insertions(+) > create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated. make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs.... make: *** [Makefile:1441: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/patch/1552385 This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit.
Hi Conor, On Mon, Nov 8, 2021 at 4:07 PM <conor.dooley@microchip.com> wrote: > From: Conor Dooley <conor.dooley@microchip.com> > > Add device tree bindings for the {q,}spi controller on > the Microchip PolarFire SoC. > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com> Thanks for your patch! > --- /dev/null > +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml > @@ -0,0 +1,72 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/spi/microchip,mpfs-spi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip MPFS {Q,}SPI Controller Device Tree Bindings > + > +maintainers: > + - Conor Dooley <conor.dooley@microchip.com> > + > +description: | > + This {Q,}SPI controller is found on the Microchip PolarFire SoC. > + > +allOf: > + - $ref: "spi-controller.yaml#" > + > +properties: > + compatible: > + enum: > + - microchip,mpfs-spi > + - microsemi,ms-pf-mss-spi > + - microchip,mpfs-qspi > + - microsemi,ms-pf-mss-qspi Same comment as before: what are the ms-pf-mss entries? > +examples: > + - | > + #include "dt-bindings/clock/microchip,mpfs-clock.h" > + #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h" > + soc { > + #address-cells = <2>; > + #size-cells = <2>; Please drop these two... > + spi0: spi@20108000 { > + compatible = "microchip,mpfs-spi"; > + reg = <0x0 0x20108000 0x0 0x1000>; ... and the zeros here. > + clocks = <&clkcfg CLK_SPI0>; > + interrupt-parent = <&plic>; > + interrupts = <PLIC_INT_SPI0>; > + spi-max-frequency = <25000000>; > + num-cs = <8>; > + status = "disabled"; Please drop this. > + }; > + }; > +... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 09/11/2021 08:45, Geert Uytterhoeven wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > Hi Conor, > >> + >> +properties: >> + compatible: >> + enum: >> + - microchip,mpfs-spi >> + - microsemi,ms-pf-mss-spi >> + - microchip,mpfs-qspi >> + - microsemi,ms-pf-mss-qspi > > Same comment as before: what are the ms-pf-mss entries? > i think i will just drop the microsemi bindings, on reflection i dont think theyre relevant to this version of the device tree.
On 09/11/2021 04:06, Rob Herring wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: >> From: Conor Dooley <conor.dooley@microchip.com> >> >> Add device tree bindings for the {q,}spi controller on >> the Microchip PolarFire SoC. >> >> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >> --- >> .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ >> 1 file changed, 72 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory > 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" Rob, Should I drop the header from the example or is there a way for me specify the dependent patch to pass this check? > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > compilation terminated. > make[1]: *** [scripts/Makefile.lib:385: Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dt.yaml] Error 1 > make[1]: *** Waiting for unfinished jobs.... > make: *** [Makefile:1441: dt_binding_check] Error 2 > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/patch/1552385 > > This check can fail if there are any dependencies. The base for a patch > series is generally the most recent rc1. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit. >
On 09/11/2021 13:16, Conor.Dooley@microchip.com wrote: > On 09/11/2021 04:06, Rob Herring wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: >>> From: Conor Dooley <conor.dooley@microchip.com> >>> >>> Add device tree bindings for the {q,}spi controller on >>> the Microchip PolarFire SoC. >>> >>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>> --- >>> .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ >>> 1 file changed, 72 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>> >> >> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >> on your patch (DT_CHECKER_FLAGS is new in v5.13): >> >> yamllint warnings/errors: >> >> dtschema/dtc warnings/errors: >> Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory >> 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" > Rob, > Should I drop the header from the example or is there a way for me > specify the dependent patch to pass this check? The error has to be fixed, although not necessarily by dropping the header, but by posting it. How this can pass on your system? There is no such file added in this patchset. Best regards, Krzysztof
On 09/11/2021 12:53, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 09/11/2021 13:16, Conor.Dooley@microchip.com wrote: >> On 09/11/2021 04:06, Rob Herring wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: >>>> From: Conor Dooley <conor.dooley@microchip.com> >>>> >>>> Add device tree bindings for the {q,}spi controller on >>>> the Microchip PolarFire SoC. >>>> >>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>>> --- >>>> .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ >>>> 1 file changed, 72 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>> >>> >>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >>> on your patch (DT_CHECKER_FLAGS is new in v5.13): >>> >>> yamllint warnings/errors: >>> >>> dtschema/dtc warnings/errors: >>> Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory >>> 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" >> Rob, >> Should I drop the header from the example or is there a way for me >> specify the dependent patch to pass this check? > > The error has to be fixed, although not necessarily by dropping the > header, but by posting it. How this can pass on your system? There is no > such file added in this patchset. I linked the patch adding the clock as a dependency in the cover letter [1], which is why I was wondering if there was a better way to do so that would get picked up by the checker bot. [1] https://lore.kernel.org/linux-clk/20210818141102.36655-2-daire.mcnamara@microchip.com/ > > Best regards, > Krzysztof >
On 09/11/2021 13:58, Conor.Dooley@microchip.com wrote: > On 09/11/2021 12:53, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 09/11/2021 13:16, Conor.Dooley@microchip.com wrote: >>> On 09/11/2021 04:06, Rob Herring wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>> >>>> On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: >>>>> From: Conor Dooley <conor.dooley@microchip.com> >>>>> >>>>> Add device tree bindings for the {q,}spi controller on >>>>> the Microchip PolarFire SoC. >>>>> >>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>>>> --- >>>>> .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ >>>>> 1 file changed, 72 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>>> >>>> >>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >>>> on your patch (DT_CHECKER_FLAGS is new in v5.13): >>>> >>>> yamllint warnings/errors: >>>> >>>> dtschema/dtc warnings/errors: >>>> Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory >>>> 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" >>> Rob, >>> Should I drop the header from the example or is there a way for me >>> specify the dependent patch to pass this check? >> >> The error has to be fixed, although not necessarily by dropping the >> header, but by posting it. How this can pass on your system? There is no >> such file added in this patchset. > I linked the patch adding the clock as a dependency in the cover letter > [1], which is why I was wondering if there was a better way to do so > that would get picked up by the checker bot. It's not only about the bot, but dependency when applied. If you did not warn clk maintainer that clock bindings should go via Rob's tree or should be provided as a tag, the patches here cannot be applied in this cycle. Best regards, Krzysztof
On 09/11/2021 13:04, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 09/11/2021 13:58, Conor.Dooley@microchip.com wrote: >> On 09/11/2021 12:53, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On 09/11/2021 13:16, Conor.Dooley@microchip.com wrote: >>>> On 09/11/2021 04:06, Rob Herring wrote: >>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>> >>>>> On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: >>>>>> From: Conor Dooley <conor.dooley@microchip.com> >>>>>> >>>>>> Add device tree bindings for the {q,}spi controller on >>>>>> the Microchip PolarFire SoC. >>>>>> >>>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>>>>> --- >>>>>> .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ >>>>>> 1 file changed, 72 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>>>> >>>>> >>>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >>>>> on your patch (DT_CHECKER_FLAGS is new in v5.13): >>>>> >>>>> yamllint warnings/errors: >>>>> >>>>> dtschema/dtc warnings/errors: >>>>> Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory >>>>> 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" >>>> Rob, >>>> Should I drop the header from the example or is there a way for me >>>> specify the dependent patch to pass this check? >>> >>> The error has to be fixed, although not necessarily by dropping the >>> header, but by posting it. How this can pass on your system? There is no >>> such file added in this patchset. >> I linked the patch adding the clock as a dependency in the cover letter >> [1], which is why I was wondering if there was a better way to do so >> that would get picked up by the checker bot. > > It's not only about the bot, but dependency when applied. If you did not > warn clk maintainer that clock bindings should go via Rob's tree or > should be provided as a tag, the patches here cannot be applied in this > cycle. It was not my (our) intention to send the clock patches via rob's tree. And since this is my first time trying to upstream wholescale changes to a device tree I honestly didn't expect this series to get accepted in this cycle anyway. > > Best regards, > Krzysztof >
On 09/11/2021 14:20, Conor.Dooley@microchip.com wrote: > On 09/11/2021 13:04, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 09/11/2021 13:58, Conor.Dooley@microchip.com wrote: >>> On 09/11/2021 12:53, Krzysztof Kozlowski wrote: >>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>> >>>> On 09/11/2021 13:16, Conor.Dooley@microchip.com wrote: >>>>> On 09/11/2021 04:06, Rob Herring wrote: >>>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>>>>> >>>>>> On Mon, 08 Nov 2021 15:05:51 +0000, conor.dooley@microchip.com wrote: >>>>>>> From: Conor Dooley <conor.dooley@microchip.com> >>>>>>> >>>>>>> Add device tree bindings for the {q,}spi controller on >>>>>>> the Microchip PolarFire SoC. >>>>>>> >>>>>>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com> >>>>>>> --- >>>>>>> .../bindings/spi/microchip,mpfs-spi.yaml | 72 +++++++++++++++++++ >>>>>>> 1 file changed, 72 insertions(+) >>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml >>>>>>> >>>>>> >>>>>> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' >>>>>> on your patch (DT_CHECKER_FLAGS is new in v5.13): >>>>>> >>>>>> yamllint warnings/errors: >>>>>> >>>>>> dtschema/dtc warnings/errors: >>>>>> Documentation/devicetree/bindings/spi/microchip,mpfs-spi.example.dts:19:18: fatal error: dt-bindings/clock/microchip,mpfs-clock.h: No such file or directory >>>>>> 19 | #include "dt-bindings/clock/microchip,mpfs-clock.h" >>>>> Rob, >>>>> Should I drop the header from the example or is there a way for me >>>>> specify the dependent patch to pass this check? >>>> >>>> The error has to be fixed, although not necessarily by dropping the >>>> header, but by posting it. How this can pass on your system? There is no >>>> such file added in this patchset. >>> I linked the patch adding the clock as a dependency in the cover letter >>> [1], which is why I was wondering if there was a better way to do so >>> that would get picked up by the checker bot. >> >> It's not only about the bot, but dependency when applied. If you did not >> warn clk maintainer that clock bindings should go via Rob's tree or >> should be provided as a tag, the patches here cannot be applied in this >> cycle. > It was not my (our) intention to send the clock patches via rob's tree. > And since this is my first time trying to upstream wholescale changes to > a device tree I honestly didn't expect this series to get accepted in > this cycle anyway. OK :) Assuming your new bindings pass db_binding_check with DT_CHECKER_FLAGS=-m (on top of clock patch), I propose to keep the header here. Another idea would be to submit without the header and use raw IDs (numbers) and convert it later. I prefer the first- base on clock patches. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml new file mode 100644 index 000000000000..efed145ad029 --- /dev/null +++ b/Documentation/devicetree/bindings/spi/microchip,mpfs-spi.yaml @@ -0,0 +1,72 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/spi/microchip,mpfs-spi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip MPFS {Q,}SPI Controller Device Tree Bindings + +maintainers: + - Conor Dooley <conor.dooley@microchip.com> + +description: | + This {Q,}SPI controller is found on the Microchip PolarFire SoC. + +allOf: + - $ref: "spi-controller.yaml#" + +properties: + compatible: + enum: + - microchip,mpfs-spi + - microsemi,ms-pf-mss-spi + - microchip,mpfs-qspi + - microsemi,ms-pf-mss-qspi + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + clock-names: + maxItems: 1 + + clocks: + maxItems: 2 + + num-cs: + description: | + Number of chip selects used. + $ref: /schemas/types.yaml#/definitions/uint32 + minimum: 1 + maximum: 8 + default: 8 + +required: + - compatible + - reg + - interrupts + - clocks + +unevaluatedProperties: false + +examples: + - | + #include "dt-bindings/clock/microchip,mpfs-clock.h" + #include "dt-bindings/interrupt-controller/microchip,mpfs-plic.h" + soc { + #address-cells = <2>; + #size-cells = <2>; + spi0: spi@20108000 { + compatible = "microchip,mpfs-spi"; + reg = <0x0 0x20108000 0x0 0x1000>; + clocks = <&clkcfg CLK_SPI0>; + interrupt-parent = <&plic>; + interrupts = <PLIC_INT_SPI0>; + spi-max-frequency = <25000000>; + num-cs = <8>; + status = "disabled"; + }; + }; +...