* [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode @ 2020-02-27 13:03 Geert Uytterhoeven 2020-02-27 13:09 ` Maxime Ripard 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2020-02-27 13:03 UTC (permalink / raw) To: Mark Brown, Rob Herring, Maxime Ripard Cc: Yoshihiro Shimoda, linux-spi-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA, Linux-renesas, Geert Uytterhoeven Currently, the DT bindings for an SPI controller specify that "#address-cells" must be fixed to one. However, that applies to an SPI controller in master mode only. When running in SPI slave mode, "#address-cells" should be zero. Fix this making the value of "#address-cells" dependent on the presence of "spi-slave". Fixes: 0a1b929356830257 ("spi: Add YAML schemas for the generic SPI options") Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> --- Is this the right way to specify this? I couldn't find an example, so this needed some trial-and-error. As of dtc commit 403cc79f06a135ae ("checks: Update SPI bus check for 'spi-slave'") and Linux commit c2e7075ca8303631 ("scripts/dtc: Update to upstream version v1.4.7-57-gf267e674d145"), dtc knows about SPI slave. However, when using "#address-cells = <0>" with W=1: Warning (avoid_unnecessary_addr_size): /soc/spi@e6e10000: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property Just removing #address-cells (using /delete-property/ in the board DTS) to fix this warning causes: Warning (spi_bus_bridge): /soc/spi@e6e10000: incorrect #address-cells for SPI bus as spi_bus_bridge() uses node_addr_cells(), which defaults to 2 (due to dtc's ppc64 heritage? But node_size_cells() defaults to 1, not 2?). How should this be fixed? Should "#address-cells = <0>" be left out or not? Should node_{addr,size}_cells() in dtc default to zero? Thanks! --- .../devicetree/bindings/spi/spi-controller.yaml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml index 1e0ca6ccf64bbd0a..ba2646258b94980e 100644 --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml @@ -20,7 +20,8 @@ properties: pattern: "^spi(@.*|-[0-9a-f])*$" "#address-cells": - const: 1 + minimum: 0 + maximum: 1 "#size-cells": const: 0 @@ -52,6 +53,18 @@ properties: description: The SPI controller acts as a slave, instead of a master. +if: + required: + [ spi-slave ] +then: + properties: + "#address-cells": + const: 0 +else: + properties: + "#address-cells": + const: 1 + patternProperties: "^slave$": type: object -- 2.17.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode 2020-02-27 13:03 [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode Geert Uytterhoeven @ 2020-02-27 13:09 ` Maxime Ripard [not found] ` <20200227130937.qvrjyxcwim7rfum7-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2020-02-27 13:09 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Brown, Rob Herring, Yoshihiro Shimoda, linux-spi, devicetree, Linux-renesas [-- Attachment #1: Type: text/plain, Size: 2652 bytes --] Hi! On Thu, Feb 27, 2020 at 02:03:23PM +0100, Geert Uytterhoeven wrote: > Currently, the DT bindings for an SPI controller specify that > "#address-cells" must be fixed to one. However, that applies to an SPI > controller in master mode only. When running in SPI slave mode, > "#address-cells" should be zero. > > Fix this making the value of "#address-cells" dependent on the presence > of "spi-slave". > > Fixes: 0a1b929356830257 ("spi: Add YAML schemas for the generic SPI options") > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > Is this the right way to specify this? I couldn't find an example, so > this needed some trial-and-error. > > As of dtc commit 403cc79f06a135ae ("checks: Update SPI bus check for > 'spi-slave'") and Linux commit c2e7075ca8303631 ("scripts/dtc: Update to > upstream version v1.4.7-57-gf267e674d145"), dtc knows about SPI slave. > > However, when using "#address-cells = <0>" with W=1: > > Warning (avoid_unnecessary_addr_size): /soc/spi@e6e10000: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property > > Just removing #address-cells (using /delete-property/ in the board DTS) > to fix this warning causes: > > Warning (spi_bus_bridge): /soc/spi@e6e10000: incorrect #address-cells for SPI bus > > as spi_bus_bridge() uses node_addr_cells(), which defaults to 2 (due to > dtc's ppc64 heritage? But node_size_cells() defaults to 1, not 2?). > > How should this be fixed? > Should "#address-cells = <0>" be left out or not? > Should node_{addr,size}_cells() in dtc default to zero? > > Thanks! > --- > .../devicetree/bindings/spi/spi-controller.yaml | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/spi/spi-controller.yaml b/Documentation/devicetree/bindings/spi/spi-controller.yaml > index 1e0ca6ccf64bbd0a..ba2646258b94980e 100644 > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml > @@ -20,7 +20,8 @@ properties: > pattern: "^spi(@.*|-[0-9a-f])*$" > > "#address-cells": > - const: 1 > + minimum: 0 > + maximum: 1 Nit: This would be better described as an enum: [0, 1] > "#size-cells": > const: 0 > @@ -52,6 +53,18 @@ properties: > description: > The SPI controller acts as a slave, instead of a master. > > +if: > + required: > + [ spi-slave ] Nit: Usually, that notation is when you would do it on the same line, if you want to go to a new line, it would make more sense to use - Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20200227130937.qvrjyxcwim7rfum7-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org>]
* Re: [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode [not found] ` <20200227130937.qvrjyxcwim7rfum7-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org> @ 2020-02-27 13:19 ` Geert Uytterhoeven [not found] ` <CAMuHMdUVW_51dqMK=ziz_JVT3LuXX7vOF3-Ei-aHiiBuSepGpg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2020-02-27 13:19 UTC (permalink / raw) To: Maxime Ripard Cc: Mark Brown, Rob Herring, Yoshihiro Shimoda, linux-spi, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-renesas Hi Maxime, On Thu, Feb 27, 2020 at 2:09 PM Maxime Ripard <maxime-R63rPqgGiG5yDzI6CaY1VQ@public.gmane.org> wrote: > On Thu, Feb 27, 2020 at 02:03:23PM +0100, Geert Uytterhoeven wrote: > > Currently, the DT bindings for an SPI controller specify that > > "#address-cells" must be fixed to one. However, that applies to an SPI > > controller in master mode only. When running in SPI slave mode, > > "#address-cells" should be zero. > > > > Fix this making the value of "#address-cells" dependent on the presence > > of "spi-slave". > > > > Fixes: 0a1b929356830257 ("spi: Add YAML schemas for the generic SPI options") > > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> > > Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> > > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml > > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml > > @@ -52,6 +53,18 @@ properties: > > description: > > The SPI controller acts as a slave, instead of a master. > > > > +if: > > + required: > > + [ spi-slave ] > > Nit: Usually, that notation is when you would do it on the same line, > if you want to go to a new line, it would make more sense to use - Sorry, being a poor yaml-by-example programmer, I don't understand what you mean. Which part do you refer to by "that notation"? Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <CAMuHMdUVW_51dqMK=ziz_JVT3LuXX7vOF3-Ei-aHiiBuSepGpg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>]
* Re: [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode [not found] ` <CAMuHMdUVW_51dqMK=ziz_JVT3LuXX7vOF3-Ei-aHiiBuSepGpg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> @ 2020-02-27 13:51 ` Maxime Ripard [not found] ` <20200227135109.l2oal55nwhm5un2c-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2020-02-27 13:51 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Brown, Rob Herring, Yoshihiro Shimoda, linux-spi, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-renesas [-- Attachment #1: Type: text/plain, Size: 1699 bytes --] On Thu, Feb 27, 2020 at 02:19:08PM +0100, Geert Uytterhoeven wrote: > Hi Maxime, > > On Thu, Feb 27, 2020 at 2:09 PM Maxime Ripard <maxime-R63rPqgGiG5yDzI6CaY1VQ@public.gmane.org> wrote: > > On Thu, Feb 27, 2020 at 02:03:23PM +0100, Geert Uytterhoeven wrote: > > > Currently, the DT bindings for an SPI controller specify that > > > "#address-cells" must be fixed to one. However, that applies to an SPI > > > controller in master mode only. When running in SPI slave mode, > > > "#address-cells" should be zero. > > > > > > Fix this making the value of "#address-cells" dependent on the presence > > > of "spi-slave". > > > > > > Fixes: 0a1b929356830257 ("spi: Add YAML schemas for the generic SPI options") > > > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> > > > Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> > > > > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml > > > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml > > > > @@ -52,6 +53,18 @@ properties: > > > description: > > > The SPI controller acts as a slave, instead of a master. > > > > > > +if: > > > + required: > > > + [ spi-slave ] > > > > Nit: Usually, that notation is when you would do it on the same line, > > if you want to go to a new line, it would make more sense to use - > > Sorry, being a poor yaml-by-example programmer, I don't understand what > you mean. Which part do you refer to by "that notation"? I meant that usually the [] notation is used over a single line. If you want to have a list over multiple lines, usually you would have required: - spi-slave Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20200227135109.l2oal55nwhm5un2c-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org>]
* Re: [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode [not found] ` <20200227135109.l2oal55nwhm5un2c-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org> @ 2020-02-27 14:05 ` Geert Uytterhoeven 2020-02-27 14:16 ` Maxime Ripard 0 siblings, 1 reply; 6+ messages in thread From: Geert Uytterhoeven @ 2020-02-27 14:05 UTC (permalink / raw) To: Maxime Ripard Cc: Mark Brown, Rob Herring, Yoshihiro Shimoda, linux-spi, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-renesas Hi Maxime, On Thu, Feb 27, 2020 at 2:51 PM Maxime Ripard <maxime-R63rPqgGiG5yDzI6CaY1VQ@public.gmane.org> wrote: > On Thu, Feb 27, 2020 at 02:19:08PM +0100, Geert Uytterhoeven wrote: > > On Thu, Feb 27, 2020 at 2:09 PM Maxime Ripard <maxime-R63rPqgGiG5yDzI6CaY1VQ@public.gmane.org> wrote: > > > On Thu, Feb 27, 2020 at 02:03:23PM +0100, Geert Uytterhoeven wrote: > > > > Currently, the DT bindings for an SPI controller specify that > > > > "#address-cells" must be fixed to one. However, that applies to an SPI > > > > controller in master mode only. When running in SPI slave mode, > > > > "#address-cells" should be zero. > > > > > > > > Fix this making the value of "#address-cells" dependent on the presence > > > > of "spi-slave". > > > > > > > > Fixes: 0a1b929356830257 ("spi: Add YAML schemas for the generic SPI options") > > > > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org> > > > > > > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml > > > > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml > > > > > > @@ -52,6 +53,18 @@ properties: > > > > description: > > > > The SPI controller acts as a slave, instead of a master. > > > > > > > > +if: > > > > + required: > > > > + [ spi-slave ] > > > > > > Nit: Usually, that notation is when you would do it on the same line, > > > if you want to go to a new line, it would make more sense to use - > > > > Sorry, being a poor yaml-by-example programmer, I don't understand what > > you mean. Which part do you refer to by "that notation"? > > I meant that usually the [] notation is used over a single line. > > If you want to have a list over multiple lines, usually you would have > > required: > - spi-slave IC. So I can also write: if: required: [ spi-slave ] else: ... which is even shorter ;-) Both seem to work fine. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode 2020-02-27 14:05 ` Geert Uytterhoeven @ 2020-02-27 14:16 ` Maxime Ripard 0 siblings, 0 replies; 6+ messages in thread From: Maxime Ripard @ 2020-02-27 14:16 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Mark Brown, Rob Herring, Yoshihiro Shimoda, linux-spi, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-renesas [-- Attachment #1: Type: text/plain, Size: 2177 bytes --] On Thu, Feb 27, 2020 at 03:05:56PM +0100, Geert Uytterhoeven wrote: > Hi Maxime, > > On Thu, Feb 27, 2020 at 2:51 PM Maxime Ripard <maxime@cerno.tech> wrote: > > On Thu, Feb 27, 2020 at 02:19:08PM +0100, Geert Uytterhoeven wrote: > > > On Thu, Feb 27, 2020 at 2:09 PM Maxime Ripard <maxime@cerno.tech> wrote: > > > > On Thu, Feb 27, 2020 at 02:03:23PM +0100, Geert Uytterhoeven wrote: > > > > > Currently, the DT bindings for an SPI controller specify that > > > > > "#address-cells" must be fixed to one. However, that applies to an SPI > > > > > controller in master mode only. When running in SPI slave mode, > > > > > "#address-cells" should be zero. > > > > > > > > > > Fix this making the value of "#address-cells" dependent on the presence > > > > > of "spi-slave". > > > > > > > > > > Fixes: 0a1b929356830257 ("spi: Add YAML schemas for the generic SPI options") > > > > > Reported-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > > > --- a/Documentation/devicetree/bindings/spi/spi-controller.yaml > > > > > +++ b/Documentation/devicetree/bindings/spi/spi-controller.yaml > > > > > > > > @@ -52,6 +53,18 @@ properties: > > > > > description: > > > > > The SPI controller acts as a slave, instead of a master. > > > > > > > > > > +if: > > > > > + required: > > > > > + [ spi-slave ] > > > > > > > > Nit: Usually, that notation is when you would do it on the same line, > > > > if you want to go to a new line, it would make more sense to use - > > > > > > Sorry, being a poor yaml-by-example programmer, I don't understand what > > > you mean. Which part do you refer to by "that notation"? > > > > I meant that usually the [] notation is used over a single line. > > > > If you want to have a list over multiple lines, usually you would have > > > > required: > > - spi-slave > > IC. So I can also write: > > if: > required: [ spi-slave ] > else: > ... > > which is even shorter ;-) > > Both seem to work fine. The rest of the file uses the multi-line syntax, so I wouldn't do it for consistency, but yeah, they are equivalent. Maxime [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-02-27 14:16 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-27 13:03 [PATCH] spi: dt-bindings: spi-controller: Fix #address-cells for slave mode Geert Uytterhoeven 2020-02-27 13:09 ` Maxime Ripard [not found] ` <20200227130937.qvrjyxcwim7rfum7-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org> 2020-02-27 13:19 ` Geert Uytterhoeven [not found] ` <CAMuHMdUVW_51dqMK=ziz_JVT3LuXX7vOF3-Ei-aHiiBuSepGpg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org> 2020-02-27 13:51 ` Maxime Ripard [not found] ` <20200227135109.l2oal55nwhm5un2c-2DbqMqoCcjvhXIiyNabO3w@public.gmane.org> 2020-02-27 14:05 ` Geert Uytterhoeven 2020-02-27 14:16 ` Maxime Ripard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).