* [PATCH 0/3] [v5 net-next] wireless: p54 devicetree conversion
@ 2026-05-07 21:24 Arnd Bergmann
2026-05-07 21:24 ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Arnd Bergmann
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Arnd Bergmann @ 2026-05-07 21:24 UTC (permalink / raw)
To: netdev
Cc: Arnd Bergmann, Aaro Koskinen, Andreas Kemnade,
Bartosz Golaszewski, Benoît Cousson, David S. Miller,
Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski,
Johannes Berg, Kevin Hilman, Krzysztof Kozlowski, Linus Walleij,
Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren,
linux-wireless, devicetree, linux-kernel, linux-arm-kernel,
linux-gpio, linux-omap
From: Arnd Bergmann <arnd@arndb.de>
This is an older patch of mine that I lost track of. We already decided
a while ago that the OMAP2 platform should probably be removed entirely,
and this is the only known user, but it's probably a good idea to still
get the driver changes in, in case there are other out-of-tree users.
We probably don't have to worry about bisectability any more though,
so the devicetree and driver changes can just get merged independently
through the OMAP and wireless trees, respectively.
Arnd
---
v4 Link: https://lore.kernel.org/all/20260430081242.3686993-1-arnd@kernel.org/
v3 Link: https://lore.kernel.org/all/20260427142355.2532714-1-arnd@kernel.org/
v2 Link: https://lore.kernel.org/all/20230404082401.1087835-1-arnd@kernel.org/
Cc: "Aaro Koskinen" <aaro.koskinen@iki.fi>
Cc: "Andreas Kemnade" <andreas@kemnade.info>
Cc: "Arnd Bergmann" <arnd@arndb.de>
Cc: "Bartosz Golaszewski" <brgl@kernel.org>
Cc: "Benoît Cousson" <bcousson@baylibre.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: "Dmitry Torokhov" <dmitry.torokhov@gmail.com>
Cc: "Eric Dumazet" <edumazet@google.com>
Cc: "Felipe Balbi" <balbi@kernel.org>
Cc: "Jakub Kicinski" <kuba@kernel.org>
Cc: "Johannes Berg" <johannes@sipsolutions.net>
Cc: "Kevin Hilman" <khilman@baylibre.com>
Cc: "Krzysztof Kozlowski" <krzk+dt@kernel.org>
Cc: "Linus Walleij" <linusw@kernel.org>
Cc: "Paolo Abeni" <pabeni@redhat.com>
Cc: "Rob Herring" <robh+dt@kernel.org>
Cc: "Roger Quadros" <rogerq@kernel.org>
Cc: "Tony Lindgren" <tony@atomide.com>
Cc: linux-wireless@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-omap@vger.kernel.org
Arnd Bergmann (3):
[v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding
[v5 net-next] p54spi: convert to devicetree
[v5 omap] ARM: dts: omap2: add stlc4560 spi-wireless node
.../bindings/net/wireless/st,stlc4560.yaml | 61 +++++++++++++++++
MAINTAINERS | 1 +
arch/arm/boot/dts/ti/omap/omap2.dtsi | 4 ++
.../dts/ti/omap/omap2420-n8x0-common.dtsi | 12 ++++
arch/arm/mach-omap2/board-n8x0.c | 18 -----
drivers/net/wireless/intersil/p54/p54spi.c | 67 +++++++------------
drivers/net/wireless/intersil/p54/p54spi.h | 3 +
7 files changed, 104 insertions(+), 62 deletions(-)
create mode 100644 Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml
--
2.39.5
^ permalink raw reply [flat|nested] 15+ messages in thread* [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding 2026-05-07 21:24 [PATCH 0/3] [v5 net-next] wireless: p54 devicetree conversion Arnd Bergmann @ 2026-05-07 21:24 ` Arnd Bergmann 2026-05-08 15:05 ` Conor Dooley 2026-05-08 21:32 ` sashiko-bot 2026-05-07 21:24 ` [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Arnd Bergmann 2026-05-07 21:24 ` [PATCH 3/3] [v5 omap] ARM: dts: omap2: add stlc4560 spi-wireless node Arnd Bergmann 2 siblings, 2 replies; 15+ messages in thread From: Arnd Bergmann @ 2026-05-07 21:24 UTC (permalink / raw) To: netdev Cc: Arnd Bergmann, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S. Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, Krzysztof Kozlowski, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, linux-gpio, linux-omap, Christian Lamparter From: Arnd Bergmann <arnd@arndb.de> The SPI version of Prism54 was sold under a couple of different names and supported by the Linux p54spi driver, but there was never a DT binding for it. Document the four known names of this device and the properties that are sufficient for its use on the Nokia N8x0 tablet. As I don't have this hardware or documentation for it, this is purely based on existing usage in the driver. Link: https://lore.kernel.org/all/e8dc9acb-6f85-e0a9-a145-d101ca6da201@gmail.com/ Acked-by: Christian Lamparter <chunkeey@gmail.com> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- v5: fix name in MAINTAINERS file v4: renamed file to st,stlc4560, matching the primary compatible string require st,stlc4560 string --- .../bindings/net/wireless/st,stlc4560.yaml | 61 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 62 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml diff --git a/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml new file mode 100644 index 000000000000..a32265c07350 --- /dev/null +++ b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/wireless/st,stlc4560.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ST/Intersil/Conexant stlc45xx/p54spi/cx3110x SPI wireless device + +maintainers: + - Christian Lamparter <chunkeey@gmail.com> + +description: + The SPI variant of the Intersil Prism54 wireless device was sold + under a variety of names, including Conexant CX3110x and + ST Microelectronics STLC5460. + +allOf: + - $ref: ieee80211.yaml# + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + oneOf: + - const: st,stlc4560 + - items: + - enum: + - cnxt,3110x + - isil,p54spi + - st,stlc4550 + - const: st,stlc4560 + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + powerdown-gpios: + maxItems: 1 + +required: + - compatible + - reg + - interrupts + +unevaluatedProperties: false + +examples: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + wifi@0 { + compatible = "st,stlc4560"; + reg = <0>; + spi-max-frequency = <48000000>; + interrupts-extended = <&gpio 23>; + powerdown-gpios = <&gpio 1>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 21c0ef0b9ce5..7defcc6e2072 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20142,6 +20142,7 @@ M: Christian Lamparter <chunkeey@googlemail.com> L: linux-wireless@vger.kernel.org S: Maintained W: https://wireless.wiki.kernel.org/en/users/Drivers/p54 +F: Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml F: drivers/net/wireless/intersil/ PACKET SOCKETS -- 2.39.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding 2026-05-07 21:24 ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Arnd Bergmann @ 2026-05-08 15:05 ` Conor Dooley 2026-05-08 21:32 ` sashiko-bot 1 sibling, 0 replies; 15+ messages in thread From: Conor Dooley @ 2026-05-08 15:05 UTC (permalink / raw) To: Arnd Bergmann Cc: netdev, Arnd Bergmann, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S. Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, Krzysztof Kozlowski, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, linux-gpio, linux-omap, Christian Lamparter [-- Attachment #1: Type: text/plain, Size: 766 bytes --] On Thu, May 07, 2026 at 11:24:49PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The SPI version of Prism54 was sold under a couple of different > names and supported by the Linux p54spi driver, but there was > never a DT binding for it. > > Document the four known names of this device and the properties > that are sufficient for its use on the Nokia N8x0 tablet. > > As I don't have this hardware or documentation for it, this is > purely based on existing usage in the driver. > > Link: https://lore.kernel.org/all/e8dc9acb-6f85-e0a9-a145-d101ca6da201@gmail.com/ > Acked-by: Christian Lamparter <chunkeey@gmail.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Conor Dooley <conor.dooley@microchip.com> [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding 2026-05-07 21:24 ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Arnd Bergmann 2026-05-08 15:05 ` Conor Dooley @ 2026-05-08 21:32 ` sashiko-bot 2026-05-09 15:56 ` Conor Dooley 1 sibling, 1 reply; 15+ messages in thread From: sashiko-bot @ 2026-05-08 21:32 UTC (permalink / raw) To: Arnd Bergmann; +Cc: conor+dt, krzk+dt, devicetree, robh Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Low] Typo in the binding description: 'STLC5460' is used instead of 'STLC4560'. - [Low] The `interrupts-extended` and `powerdown-gpios` specifiers in the example lack standard secondary cells (flags/triggers). -- commit b803e7cb976e6f9f5ea1b9d2c76cdffbb8fe0d67 Author: Arnd Bergmann <arnd@arndb.de> dt-bindings: net: add st,stlc4560/p54spi binding This commit adds a device tree binding for the SPI version of the Prism54 wireless device. It documents the known names of this device and the properties sufficient for its use. Link: https://lore.kernel.org/all/e8dc9acb-6f85-e0a9-a145-d101ca6da201@gmail.com/ > diff --git a/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml > new file mode 100644 > index 0000000000000..a32265c07350d > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml > @@ -0,0 +1,61 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/wireless/st,stlc4560.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ST/Intersil/Conexant stlc45xx/p54spi/cx3110x SPI wireless device > + > +maintainers: > + - Christian Lamparter <chunkeey@gmail.com> > + > +description: > + The SPI variant of the Intersil Prism54 wireless device was sold > + under a variety of names, including Conexant CX3110x and > + ST Microelectronics STLC5460. This isn't a bug, but there's a typo in the description (STLC5460 instead of STLC4560). [ ... ] > +examples: > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + wifi@0 { > + compatible = "st,stlc4560"; > + reg = <0>; > + spi-max-frequency = <48000000>; > + interrupts-extended = <&gpio 23>; > + powerdown-gpios = <&gpio 1>; Do these specifiers need standard secondary cells for flags and triggers? Real-world GPIO controllers typically require #gpio-cells = <2> or #interrupt-cells = <2> to specify trigger types and active levels. While dt_binding_check accepts this since the phandle cell sizes are unknown, copying this example into a real device tree will result in a compilation error. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260507212451.3333185-1-arnd@kernel.org?part=1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding 2026-05-08 21:32 ` sashiko-bot @ 2026-05-09 15:56 ` Conor Dooley 2026-05-09 16:31 ` Roman Gushchin 0 siblings, 1 reply; 15+ messages in thread From: Conor Dooley @ 2026-05-09 15:56 UTC (permalink / raw) To: sashiko; +Cc: Arnd Bergmann, conor+dt, krzk+dt, devicetree, robh [-- Attachment #1: Type: text/plain, Size: 2863 bytes --] On Fri, May 08, 2026 at 09:32:54PM +0000, sashiko-bot@kernel.org wrote: > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > - [Low] Typo in the binding description: 'STLC5460' is used instead of 'STLC4560'. > - [Low] The `interrupts-extended` and `powerdown-gpios` specifiers in the example lack standard secondary cells (flags/triggers). > -- > > commit b803e7cb976e6f9f5ea1b9d2c76cdffbb8fe0d67 > Author: Arnd Bergmann <arnd@arndb.de> > > dt-bindings: net: add st,stlc4560/p54spi binding > > This commit adds a device tree binding for the SPI version of the Prism54 > wireless device. It documents the known names of this device and the > properties sufficient for its use. > > Link: https://lore.kernel.org/all/e8dc9acb-6f85-e0a9-a145-d101ca6da201@gmail.com/ > > > diff --git a/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml > > new file mode 100644 > > index 0000000000000..a32265c07350d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml > > @@ -0,0 +1,61 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/net/wireless/st,stlc4560.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: ST/Intersil/Conexant stlc45xx/p54spi/cx3110x SPI wireless device > > + > > +maintainers: > > + - Christian Lamparter <chunkeey@gmail.com> > > + > > +description: > > + The SPI variant of the Intersil Prism54 wireless device was sold > > + under a variety of names, including Conexant CX3110x and > > + ST Microelectronics STLC5460. > > This isn't a bug, but there's a typo in the description (STLC5460 instead > of STLC4560). > > [ ... ] > > > +examples: > > + - | > > + spi { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + wifi@0 { > > + compatible = "st,stlc4560"; > > + reg = <0>; > > + spi-max-frequency = <48000000>; > > + interrupts-extended = <&gpio 23>; > > + powerdown-gpios = <&gpio 1>; > > Do these specifiers need standard secondary cells for flags and triggers? > > Real-world GPIO controllers typically require #gpio-cells = <2> or > #interrupt-cells = <2> to specify trigger types and active levels. While > dt_binding_check accepts this since the phandle cell sizes are unknown, > copying this example into a real device tree will result in a compilation > error. Can we make this thing not whinge about stuff like this? It's an /example/ and these fields cannot be copy-pasted anyway. > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260507212451.3333185-1-arnd@kernel.org?part=1 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding 2026-05-09 15:56 ` Conor Dooley @ 2026-05-09 16:31 ` Roman Gushchin 2026-05-09 18:08 ` Conor Dooley 0 siblings, 1 reply; 15+ messages in thread From: Roman Gushchin @ 2026-05-09 16:31 UTC (permalink / raw) To: Conor Dooley; +Cc: sashiko, Arnd Bergmann, conor+dt, krzk+dt, devicetree, robh Conor Dooley <conor@kernel.org> writes: > On Fri, May 08, 2026 at 09:32:54PM +0000, sashiko-bot@kernel.org wrote: >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: >> - [Low] Typo in the binding description: 'STLC5460' is used instead of 'STLC4560'. >> - [Low] The `interrupts-extended` and `powerdown-gpios` specifiers in the example lack standard secondary cells (flags/triggers). >> -- >> >> commit b803e7cb976e6f9f5ea1b9d2c76cdffbb8fe0d67 >> Author: Arnd Bergmann <arnd@arndb.de> >> >> dt-bindings: net: add st,stlc4560/p54spi binding >> >> This commit adds a device tree binding for the SPI version of the Prism54 >> wireless device. It documents the known names of this device and the >> properties sufficient for its use. >> >> Link: https://lore.kernel.org/all/e8dc9acb-6f85-e0a9-a145-d101ca6da201@gmail.com/ >> >> > diff --git a/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml >> > new file mode 100644 >> > index 0000000000000..a32265c07350d >> > --- /dev/null >> > +++ b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml >> > @@ -0,0 +1,61 @@ >> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> > +%YAML 1.2 >> > +--- >> > +$id: http://devicetree.org/schemas/net/wireless/st,stlc4560.yaml# >> > +$schema: http://devicetree.org/meta-schemas/core.yaml# >> > + >> > +title: ST/Intersil/Conexant stlc45xx/p54spi/cx3110x SPI wireless device >> > + >> > +maintainers: >> > + - Christian Lamparter <chunkeey@gmail.com> >> > + >> > +description: >> > + The SPI variant of the Intersil Prism54 wireless device was sold >> > + under a variety of names, including Conexant CX3110x and >> > + ST Microelectronics STLC5460. >> >> This isn't a bug, but there's a typo in the description (STLC5460 instead >> of STLC4560). >> >> [ ... ] >> >> > +examples: >> > + - | >> > + spi { >> > + #address-cells = <1>; >> > + #size-cells = <0>; >> > + >> > + wifi@0 { >> > + compatible = "st,stlc4560"; >> > + reg = <0>; >> > + spi-max-frequency = <48000000>; >> > + interrupts-extended = <&gpio 23>; >> > + powerdown-gpios = <&gpio 1>; >> >> Do these specifiers need standard secondary cells for flags and triggers? >> >> Real-world GPIO controllers typically require #gpio-cells = <2> or >> #interrupt-cells = <2> to specify trigger types and active levels. While >> dt_binding_check accepts this since the phandle cell sizes are unknown, >> copying this example into a real device tree will result in a compilation >> error. > > Can we make this thing not whinge about stuff like this? It's an > /example/ and these fields cannot be copy-pasted anyway. If you have an idea how to convert it into a rule, I'm happy to merge it into prompts. Something like "fields X and Y are not required for example dt records". Sorry, it's really far from my area of expertise, so I don't know what's right here and what's not. Thanks! ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding 2026-05-09 16:31 ` Roman Gushchin @ 2026-05-09 18:08 ` Conor Dooley 0 siblings, 0 replies; 15+ messages in thread From: Conor Dooley @ 2026-05-09 18:08 UTC (permalink / raw) To: Roman Gushchin Cc: sashiko, Arnd Bergmann, conor+dt, krzk+dt, devicetree, robh [-- Attachment #1: Type: text/plain, Size: 3704 bytes --] On Sat, May 09, 2026 at 09:31:24AM -0700, Roman Gushchin wrote: > Conor Dooley <conor@kernel.org> writes: > > > On Fri, May 08, 2026 at 09:32:54PM +0000, sashiko-bot@kernel.org wrote: > >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: > >> - [Low] Typo in the binding description: 'STLC5460' is used instead of 'STLC4560'. > >> - [Low] The `interrupts-extended` and `powerdown-gpios` specifiers in the example lack standard secondary cells (flags/triggers). > >> -- > >> > >> commit b803e7cb976e6f9f5ea1b9d2c76cdffbb8fe0d67 > >> Author: Arnd Bergmann <arnd@arndb.de> > >> > >> dt-bindings: net: add st,stlc4560/p54spi binding > >> > >> This commit adds a device tree binding for the SPI version of the Prism54 > >> wireless device. It documents the known names of this device and the > >> properties sufficient for its use. > >> > >> Link: https://lore.kernel.org/all/e8dc9acb-6f85-e0a9-a145-d101ca6da201@gmail.com/ > >> > >> > diff --git a/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml > >> > new file mode 100644 > >> > index 0000000000000..a32265c07350d > >> > --- /dev/null > >> > +++ b/Documentation/devicetree/bindings/net/wireless/st,stlc4560.yaml > >> > @@ -0,0 +1,61 @@ > >> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >> > +%YAML 1.2 > >> > +--- > >> > +$id: http://devicetree.org/schemas/net/wireless/st,stlc4560.yaml# > >> > +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> > + > >> > +title: ST/Intersil/Conexant stlc45xx/p54spi/cx3110x SPI wireless device > >> > + > >> > +maintainers: > >> > + - Christian Lamparter <chunkeey@gmail.com> > >> > + > >> > +description: > >> > + The SPI variant of the Intersil Prism54 wireless device was sold > >> > + under a variety of names, including Conexant CX3110x and > >> > + ST Microelectronics STLC5460. > >> > >> This isn't a bug, but there's a typo in the description (STLC5460 instead > >> of STLC4560). > >> > >> [ ... ] > >> > >> > +examples: > >> > + - | > >> > + spi { > >> > + #address-cells = <1>; > >> > + #size-cells = <0>; > >> > + > >> > + wifi@0 { > >> > + compatible = "st,stlc4560"; > >> > + reg = <0>; > >> > + spi-max-frequency = <48000000>; > >> > + interrupts-extended = <&gpio 23>; > >> > + powerdown-gpios = <&gpio 1>; > >> > >> Do these specifiers need standard secondary cells for flags and triggers? > >> > >> Real-world GPIO controllers typically require #gpio-cells = <2> or > >> #interrupt-cells = <2> to specify trigger types and active levels. While > >> dt_binding_check accepts this since the phandle cell sizes are unknown, > >> copying this example into a real device tree will result in a compilation > >> error. > > > > Can we make this thing not whinge about stuff like this? It's an > > /example/ and these fields cannot be copy-pasted anyway. > > If you have an idea how to convert it into a rule, I'm happy to merge it > into prompts. Something like "fields X and Y are not required for > example dt records". Sorry, it's really far from my area of expertise, so > I don't know what's right here and what's not. "Ignore phandle arguments in examples, when the phandle doesn't reference a node in the example"? Although, probably can go as far as "ignore phandle arguments in examples", since Rob's bot does validation on examples, and should catch a mismatch where the target of phandle exists. I dunno if "argument" is the correct word for the RHS of the property. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-07 21:24 [PATCH 0/3] [v5 net-next] wireless: p54 devicetree conversion Arnd Bergmann 2026-05-07 21:24 ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Arnd Bergmann @ 2026-05-07 21:24 ` Arnd Bergmann 2026-05-08 21:32 ` sashiko-bot ` (2 more replies) 2026-05-07 21:24 ` [PATCH 3/3] [v5 omap] ARM: dts: omap2: add stlc4560 spi-wireless node Arnd Bergmann 2 siblings, 3 replies; 15+ messages in thread From: Arnd Bergmann @ 2026-05-07 21:24 UTC (permalink / raw) To: netdev Cc: Arnd Bergmann, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S. Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, Krzysztof Kozlowski, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, linux-gpio, linux-omap, Christian Lamparter From: Arnd Bergmann <arnd@arndb.de> The Prism54 SPI driver hardcodes GPIO numbers and expects users to pass them as module parameters, apparently a relic from its life as a staging driver. This works because there is only one user, the Nokia N8x0 tablet. Convert this to the gpio descriptor interface and DT based probing to improve this and simplify the code at the same time. Acked-by: Christian Lamparter <chunkeey@gmail.com> Reviewed-by: Linus Walleij <linusw@kernel.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- v5: - fix irq assignment # https://sashiko.dev/#/patchset/20260430081242.3686993-1-arnd%40kernel.org v4: - make gpio line optional again - fix EPROBE_DEFER handling - match only st,stlc4560 compatible value, with binding change v3: - rebase an older patch --- arch/arm/mach-omap2/board-n8x0.c | 18 ------ drivers/net/wireless/intersil/p54/p54spi.c | 67 ++++++++-------------- drivers/net/wireless/intersil/p54/p54spi.h | 3 + 3 files changed, 26 insertions(+), 62 deletions(-) diff --git a/arch/arm/mach-omap2/board-n8x0.c b/arch/arm/mach-omap2/board-n8x0.c index 969265d5d5c6..d9acd32c5457 100644 --- a/arch/arm/mach-omap2/board-n8x0.c +++ b/arch/arm/mach-omap2/board-n8x0.c @@ -20,7 +20,6 @@ #include <linux/spi/spi.h> #include <linux/usb/musb.h> #include <linux/mmc/host.h> -#include <linux/platform_data/spi-omap2-mcspi.h> #include <linux/platform_data/mmc-omap.h> #include <linux/mfd/menelaus.h> @@ -106,21 +105,6 @@ static void __init n8x0_usb_init(void) {} #endif /*CONFIG_USB_MUSB_TUSB6010 */ - -static struct omap2_mcspi_device_config p54spi_mcspi_config = { - .turbo_mode = 0, -}; - -static struct spi_board_info n800_spi_board_info[] __initdata = { - { - .modalias = "p54spi", - .bus_num = 2, - .chip_select = 0, - .max_speed_hz = 48000000, - .controller_data = &p54spi_mcspi_config, - }, -}; - #if defined(CONFIG_MENELAUS) && IS_ENABLED(CONFIG_MMC_OMAP) /* @@ -524,7 +508,5 @@ omap_late_initcall(n8x0_late_initcall); void * __init n8x0_legacy_init(void) { board_check_revision(); - spi_register_board_info(n800_spi_board_info, - ARRAY_SIZE(n800_spi_board_info)); return &mmc1_data; } diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c index 9d66dcae54e0..d18be2545028 100644 --- a/drivers/net/wireless/intersil/p54/p54spi.c +++ b/drivers/net/wireless/intersil/p54/p54spi.c @@ -8,6 +8,7 @@ */ #include <linux/module.h> +#include <linux/mod_devicetable.h> #include <linux/platform_device.h> #include <linux/interrupt.h> #include <linux/firmware.h> @@ -15,7 +16,7 @@ #include <linux/irq.h> #include <linux/spi/spi.h> #include <linux/etherdevice.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/slab.h> #include "p54spi.h" @@ -30,19 +31,6 @@ MODULE_FIRMWARE("3826.arm"); MODULE_FIRMWARE("3826.eeprom"); -/* gpios should be handled in board files and provided via platform data, - * but because it's currently impossible for p54spi to have a header file - * in include/linux, let's use module parameters for now - */ - -static int p54spi_gpio_power = 97; -module_param(p54spi_gpio_power, int, 0444); -MODULE_PARM_DESC(p54spi_gpio_power, "gpio number for power line"); - -static int p54spi_gpio_irq = 87; -module_param(p54spi_gpio_irq, int, 0444); -MODULE_PARM_DESC(p54spi_gpio_irq, "gpio number for irq line"); - static void p54spi_spi_read(struct p54s_priv *priv, u8 address, void *buf, size_t len) { @@ -262,14 +250,14 @@ static int p54spi_upload_firmware(struct ieee80211_hw *dev) static void p54spi_power_off(struct p54s_priv *priv) { - disable_irq(gpio_to_irq(p54spi_gpio_irq)); - gpio_set_value(p54spi_gpio_power, 0); + disable_irq(priv->irq); + gpiod_set_value(priv->gpio_powerdown, 1); } static void p54spi_power_on(struct p54s_priv *priv) { - gpio_set_value(p54spi_gpio_power, 1); - enable_irq(gpio_to_irq(p54spi_gpio_irq)); + gpiod_set_value(priv->gpio_powerdown, 0); + enable_irq(priv->irq); /* need to wait a while before device can be accessed, the length * is just a guess @@ -608,31 +596,20 @@ static int p54spi_probe(struct spi_device *spi) goto err_free; } - ret = gpio_request(p54spi_gpio_power, "p54spi power"); - if (ret < 0) { - dev_err(&priv->spi->dev, "power GPIO request failed: %d", ret); + priv->gpio_powerdown = gpiod_get_optional(&spi->dev, "powerdown", GPIOD_OUT_HIGH); + if (IS_ERR(priv->gpio_powerdown)) { + ret = dev_err_probe(&priv->spi->dev, PTR_ERR(priv->gpio_powerdown), + "powerdown GPIO request failed\n"); goto err_free; } - ret = gpio_request(p54spi_gpio_irq, "p54spi irq"); - if (ret < 0) { - dev_err(&priv->spi->dev, "irq GPIO request failed: %d", ret); - goto err_free_gpio_power; - } - - gpio_direction_output(p54spi_gpio_power, 0); - gpio_direction_input(p54spi_gpio_irq); - - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", - priv->spi); + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); if (ret < 0) { dev_err(&priv->spi->dev, "request_irq() failed"); - goto err_free_gpio_irq; + goto err_free_gpio_power; } - irq_set_irq_type(gpio_to_irq(p54spi_gpio_irq), IRQ_TYPE_EDGE_RISING); - + priv->irq = spi->irq; INIT_WORK(&priv->work, p54spi_work); init_completion(&priv->fw_comp); INIT_LIST_HEAD(&priv->tx_pending); @@ -659,11 +636,9 @@ static int p54spi_probe(struct spi_device *spi) err_free_common: release_firmware(priv->firmware); - free_irq(gpio_to_irq(p54spi_gpio_irq), spi); -err_free_gpio_irq: - gpio_free(p54spi_gpio_irq); + free_irq(priv->irq, spi); err_free_gpio_power: - gpio_free(p54spi_gpio_power); + gpiod_put(priv->gpio_powerdown); err_free: p54_free_common(priv->hw); return ret; @@ -675,10 +650,8 @@ static void p54spi_remove(struct spi_device *spi) p54_unregister_common(priv->hw); - free_irq(gpio_to_irq(p54spi_gpio_irq), spi); - - gpio_free(p54spi_gpio_power); - gpio_free(p54spi_gpio_irq); + free_irq(priv->irq, spi); + gpiod_put(priv->gpio_powerdown); release_firmware(priv->firmware); mutex_destroy(&priv->mutex); @@ -686,10 +659,16 @@ static void p54spi_remove(struct spi_device *spi) p54_free_common(priv->hw); } +static const struct of_device_id p54spi_of_ids[] = { + { .compatible = "st,stlc4560", }, + { }, +}; +MODULE_DEVICE_TABLE(of, p54spi_of_ids); static struct spi_driver p54spi_driver = { .driver = { .name = "p54spi", + .of_match_table = p54spi_of_ids, }, .probe = p54spi_probe, diff --git a/drivers/net/wireless/intersil/p54/p54spi.h b/drivers/net/wireless/intersil/p54/p54spi.h index e5619a13fd61..118785cc635a 100644 --- a/drivers/net/wireless/intersil/p54/p54spi.h +++ b/drivers/net/wireless/intersil/p54/p54spi.h @@ -107,6 +107,9 @@ struct p54s_priv { enum fw_state fw_state; const struct firmware *firmware; + + struct gpio_desc *gpio_powerdown; + int irq; }; #endif /* P54SPI_H */ -- 2.39.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-07 21:24 ` [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Arnd Bergmann @ 2026-05-08 21:32 ` sashiko-bot 2026-05-09 17:08 ` Arnd Bergmann 2026-05-11 11:30 ` Bartosz Golaszewski 2026-05-11 16:12 ` Simon Horman 2 siblings, 1 reply; 15+ messages in thread From: sashiko-bot @ 2026-05-08 21:32 UTC (permalink / raw) To: Arnd Bergmann; +Cc: devicetree, krzk+dt, robh, conor+dt Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The `p54spi_spi_read` function performs an SPI transfer using a stack-allocated buffer, which violates DMA mapping rules. - [High] Missing validation for `spi->irq` before calling `request_irq()`. -- commit fa2ad50656ff32f99005056eb774d07bdf3f221c Author: Arnd Bergmann <arnd@arndb.de> p54spi: convert to devicetree This commit converts the Prism54 SPI driver from using hardcoded GPIO module parameters to using the GPIO descriptor interface and device tree probing. > diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c > index 9d66dcae54e02..d18be25450285 100644 > --- a/drivers/net/wireless/intersil/p54/p54spi.c > +++ b/drivers/net/wireless/intersil/p54/p54spi.c [ ... ] > @@ -30,19 +31,6 @@ > MODULE_FIRMWARE("3826.arm"); > MODULE_FIRMWARE("3826.eeprom"); > > -/* gpios should be handled in board files and provided via platform data, > - * but because it's currently impossible for p54spi to have a header file > - * in include/linux, let's use module parameters for now > - */ > - > -static int p54spi_gpio_power = 97; > -module_param(p54spi_gpio_power, int, 0444); > -MODULE_PARM_DESC(p54spi_gpio_power, "gpio number for power line"); > - > -static int p54spi_gpio_irq = 87; > -module_param(p54spi_gpio_irq, int, 0444); > -MODULE_PARM_DESC(p54spi_gpio_irq, "gpio number for irq line"); > - > static void p54spi_spi_read(struct p54s_priv *priv, u8 address, > void *buf, size_t len) > { This problem wasn't introduced by this patch, but I noticed a pre-existing issue in the context here. Does the p54spi_spi_read() function perform an SPI transfer using a stack-allocated buffer? Looking at p54spi_spi_read(), the addr variable is allocated on the stack and passed as a transfer buffer to spi_sync(). The SPI framework maps transfer buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and cannot be safely DMA-mapped. Could this cause memory corruption or a kernel panic during the transfer? [ ... ] > @@ -608,31 +596,20 @@ static int p54spi_probe(struct spi_device *spi) > goto err_free; > } > > - ret = gpio_request(p54spi_gpio_power, "p54spi power"); > - if (ret < 0) { > - dev_err(&priv->spi->dev, "power GPIO request failed: %d", ret); > + priv->gpio_powerdown = gpiod_get_optional(&spi->dev, "powerdown", GPIOD_OUT_HIGH); > + if (IS_ERR(priv->gpio_powerdown)) { > + ret = dev_err_probe(&priv->spi->dev, PTR_ERR(priv->gpio_powerdown), > + "powerdown GPIO request failed\n"); > goto err_free; > } > > - ret = gpio_request(p54spi_gpio_irq, "p54spi irq"); > - if (ret < 0) { > - dev_err(&priv->spi->dev, "irq GPIO request failed: %d", ret); > - goto err_free_gpio_power; > - } > - > - gpio_direction_output(p54spi_gpio_power, 0); > - gpio_direction_input(p54spi_gpio_irq); > - > - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), > - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", > - priv->spi); > + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); Is there missing validation for spi->irq before calling request_irq()? If the device tree node lacks an interrupts property, the SPI core may set spi->irq to 0 or a negative error code. Passing this directly to request_irq() without validation can successfully hijack a core system interrupt like the system timer on some architectures, leading to system instability or deadlocks. Should there be a check like if (spi->irq <= 0) to fail the probe gracefully? > if (ret < 0) { > dev_err(&priv->spi->dev, "request_irq() failed"); > - goto err_free_gpio_irq; > + goto err_free_gpio_power; > } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260507212451.3333185-1-arnd@kernel.org?part=2 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-08 21:32 ` sashiko-bot @ 2026-05-09 17:08 ` Arnd Bergmann 0 siblings, 0 replies; 15+ messages in thread From: Arnd Bergmann @ 2026-05-09 17:08 UTC (permalink / raw) To: sashiko; +Cc: devicetree, krzk+dt, Rob Herring, Conor Dooley On Fri, May 8, 2026, at 23:32, sashiko-bot@kernel.org wrote: >> -static int p54spi_gpio_irq = 87; >> -module_param(p54spi_gpio_irq, int, 0444); >> -MODULE_PARM_DESC(p54spi_gpio_irq, "gpio number for irq line"); >> - >> static void p54spi_spi_read(struct p54s_priv *priv, u8 address, >> void *buf, size_t len) >> { > > This problem wasn't introduced by this patch, but I noticed a pre-existing > issue in the context here. Does the p54spi_spi_read() function perform an SPI > transfer using a stack-allocated buffer? > > Looking at p54spi_spi_read(), the addr variable is allocated on the stack and > passed as a transfer buffer to spi_sync(). The SPI framework maps transfer > buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with > CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and > cannot be safely DMA-mapped. > > Could this cause memory corruption or a kernel panic during the transfer? In theory it could, as passing long buffers on stack is not allowed for SPI drivers. In practice this seems to work because all SPI drivers that perform DMA check for short transfers and fall back to MMIO if possible. In this case, the spi-omap2-mcspi driver uses DMA_MIN_BYTES=160, while the on-stack transfers are only two or four bytes and will always be done through MMIO. The driver has other flaws that I'm not trying to address either. >> - >> - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), >> - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", >> - priv->spi); >> + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); > > Is there missing validation for spi->irq before calling request_irq()? > > If the device tree node lacks an interrupts property, the SPI core may set > spi->irq to 0 or a negative error code. > > Passing this directly to request_irq() without validation can successfully > hijack a core system interrupt like the system timer on some architectures, > leading to system instability or deadlocks. > > Should there be a check like if (spi->irq <= 0) to fail the probe gracefully? request_irq() fails with -EINVAL in this case, which is an appropriate return code. This is again the same behavior as before, if an invalid GPIO, or a GPIO that is not connected to an irqchip gets passed. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-07 21:24 ` [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Arnd Bergmann 2026-05-08 21:32 ` sashiko-bot @ 2026-05-11 11:30 ` Bartosz Golaszewski 2026-05-11 16:12 ` Simon Horman 2 siblings, 0 replies; 15+ messages in thread From: Bartosz Golaszewski @ 2026-05-11 11:30 UTC (permalink / raw) To: Arnd Bergmann Cc: netdev, Arnd Bergmann, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S. Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, Krzysztof Kozlowski, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, linux-gpio, linux-omap, Christian Lamparter On Thu, 7 May 2026 23:24:50 +0200, Arnd Bergmann <arnd@kernel.org> said: > From: Arnd Bergmann <arnd@arndb.de> > > The Prism54 SPI driver hardcodes GPIO numbers and expects users to > pass them as module parameters, apparently a relic from its life as a > staging driver. This works because there is only one user, the Nokia > N8x0 tablet. > > Convert this to the gpio descriptor interface and DT based probing > to improve this and simplify the code at the same time. > > Acked-by: Christian Lamparter <chunkeey@gmail.com> > Reviewed-by: Linus Walleij <linusw@kernel.org> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-07 21:24 ` [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Arnd Bergmann 2026-05-08 21:32 ` sashiko-bot 2026-05-11 11:30 ` Bartosz Golaszewski @ 2026-05-11 16:12 ` Simon Horman 2026-05-11 19:45 ` Arnd Bergmann 2 siblings, 1 reply; 15+ messages in thread From: Simon Horman @ 2026-05-11 16:12 UTC (permalink / raw) To: arnd Cc: 'Simon Horman', netdev, arnd, aaro.koskinen, andreas, brgl, bcousson, davem, dmitry.torokhov, edumazet, balbi, kuba, johannes, khilman, krzk+dt, linusw, pabeni, robh+dt, rogerq, tony, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, linux-gpio, linux-omap, krzk From: 'Simon Horman' <horms@kernel.org> This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- p54spi: convert to devicetree This commit converts the Prism54 SPI driver from using hardcoded GPIO module parameters to using the GPIO descriptor interface and device tree probing. > diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c > index 9d66dcae54e02..d18be25450285 100644 > --- a/drivers/net/wireless/intersil/p54/p54spi.c > +++ b/drivers/net/wireless/intersil/p54/p54spi.c [ ... ] > @@ -30,19 +31,6 @@ > MODULE_FIRMWARE("3826.arm"); > MODULE_FIRMWARE("3826.eeprom"); > > -/* gpios should be handled in board files and provided via platform data, > - * but because it's currently impossible for p54spi to have a header file > - * in include/linux, let's use module parameters for now > - */ > - > -static int p54spi_gpio_power = 97; > -module_param(p54spi_gpio_power, int, 0444); > -MODULE_PARM_DESC(p54spi_gpio_power, "gpio number for power line"); > - > -static int p54spi_gpio_irq = 87; > -module_param(p54spi_gpio_irq, int, 0444); > -MODULE_PARM_DESC(p54spi_gpio_irq, "gpio number for irq line"); > - > static void p54spi_spi_read(struct p54s_priv *priv, u8 address, > void *buf, size_t len) > { Simon says: As a pre-existing issue I don't think we need to be concerned about this in the scope of this patch-set. But it may be a potential for follow-up. This problem wasn't introduced by this patch, but I noticed a pre-existing issue in the context here. Does the p54spi_spi_read() function perform an SPI transfer using a stack-allocated buffer? Looking at p54spi_spi_read(), the addr variable is allocated on the stack and passed as a transfer buffer to spi_sync(). The SPI framework maps transfer buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and cannot be safely DMA-mapped. Could this cause memory corruption or a kernel panic during the transfer? [ ... ] > @@ -608,31 +596,20 @@ static int p54spi_probe(struct spi_device *spi) > goto err_free; > } > > - ret = gpio_request(p54spi_gpio_power, "p54spi power"); > - if (ret < 0) { > - dev_err(&priv->spi->dev, "power GPIO request failed: %d", ret); > + priv->gpio_powerdown = gpiod_get_optional(&spi->dev, "powerdown", GPIOD_OUT_HIGH); > + if (IS_ERR(priv->gpio_powerdown)) { > + ret = dev_err_probe(&priv->spi->dev, PTR_ERR(priv->gpio_powerdown), > + "powerdown GPIO request failed\n"); > goto err_free; > } > > - ret = gpio_request(p54spi_gpio_irq, "p54spi irq"); > - if (ret < 0) { > - dev_err(&priv->spi->dev, "irq GPIO request failed: %d", ret); > - goto err_free_gpio_power; > - } > - > - gpio_direction_output(p54spi_gpio_power, 0); > - gpio_direction_input(p54spi_gpio_irq); > - > - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), > - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", > - priv->spi); > + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); Simon says: This one seems more concerning in the context of this patch-set. Is there missing validation for spi->irq before calling request_irq()? If the device tree node lacks an interrupts property, the SPI core may set spi->irq to 0 or a negative error code. Passing this directly to request_irq() without validation can successfully hijack a core system interrupt like the system timer on some architectures, leading to system instability or deadlocks. Should there be a check like if (spi->irq <= 0) to fail the probe gracefully? > if (ret < 0) { > dev_err(&priv->spi->dev, "request_irq() failed"); > - goto err_free_gpio_irq; > + goto err_free_gpio_power; > } ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-11 16:12 ` Simon Horman @ 2026-05-11 19:45 ` Arnd Bergmann 2026-05-12 9:40 ` Simon Horman 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2026-05-11 19:45 UTC (permalink / raw) To: Simon Horman, Arnd Bergmann Cc: Netdev, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S . Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, krzk+dt, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, open list:GPIO SUBSYSTEM, Linux-OMAP, Krzysztof Kozlowski On Mon, May 11, 2026, at 18:12, Simon Horman wrote: > > Simon says: As a pre-existing issue I don't think we need to be concerned > about this in the scope of this patch-set. But it may > be a potential for follow-up. > > This problem wasn't introduced by this patch, but I noticed a pre-existing > issue in the context here. Does the p54spi_spi_read() function perform an SPI > transfer using a stack-allocated buffer? > > Looking at p54spi_spi_read(), the addr variable is allocated on the stack and > passed as a transfer buffer to spi_sync(). The SPI framework maps transfer > buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with > CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and > cannot be safely DMA-mapped. > > Could this cause memory corruption or a kernel panic during the transfer? I already commented on this earlier: This is indeed a theoretical bug, but it works in practice because DMA-capable SPI controllers tend to all use MMIO for short transfers instead of DMA. SPI is very fragile this way, and I do have some patches to improve that overall, but it seems out of scope for this one driver. >> - >> - gpio_direction_output(p54spi_gpio_power, 0); >> - gpio_direction_input(p54spi_gpio_irq); >> - >> - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), >> - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", >> - priv->spi); >> + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); > > Simon says: This one seems more concerning in the context of this patch-set. > > Is there missing validation for spi->irq before calling request_irq()? > > If the device tree node lacks an interrupts property, the SPI core may set > spi->irq to 0 or a negative error code. > > Passing this directly to request_irq() without validation can successfully > hijack a core system interrupt like the system timer on some architectures, > leading to system instability or deadlocks. > > Should there be a check like if (spi->irq <= 0) to fail the probe gracefully? I also commented on this: request_irq() already fails gracefully with -EINVAL when presented with an invalid IRQ. IRQ 0 is guaranteed to be invalid on any target that uses devicetree. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree 2026-05-11 19:45 ` Arnd Bergmann @ 2026-05-12 9:40 ` Simon Horman 0 siblings, 0 replies; 15+ messages in thread From: Simon Horman @ 2026-05-12 9:40 UTC (permalink / raw) To: Arnd Bergmann Cc: Arnd Bergmann, Netdev, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S . Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, krzk+dt, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, open list:GPIO SUBSYSTEM, Linux-OMAP, Krzysztof Kozlowski On Mon, May 11, 2026 at 09:45:18PM +0200, Arnd Bergmann wrote: > On Mon, May 11, 2026, at 18:12, Simon Horman wrote: > > > > Simon says: As a pre-existing issue I don't think we need to be concerned > > about this in the scope of this patch-set. But it may > > be a potential for follow-up. > > > > This problem wasn't introduced by this patch, but I noticed a pre-existing > > issue in the context here. Does the p54spi_spi_read() function perform an SPI > > transfer using a stack-allocated buffer? > > > > Looking at p54spi_spi_read(), the addr variable is allocated on the stack and > > passed as a transfer buffer to spi_sync(). The SPI framework maps transfer > > buffers for DMA. Passing a stack-allocated buffer is illegal, particularly with > > CONFIG_VMAP_STACK enabled, because stack memory resides in vmalloc space and > > cannot be safely DMA-mapped. > > > > Could this cause memory corruption or a kernel panic during the transfer? > > I already commented on this earlier: This is indeed a theoretical > bug, but it works in practice because DMA-capable SPI controllers > tend to all use MMIO for short transfers instead of DMA. > > SPI is very fragile this way, and I do have some patches to improve > that overall, but it seems out of scope for this one driver. Thanks, I agree this is out of scope. > >> - > >> - gpio_direction_output(p54spi_gpio_power, 0); > >> - gpio_direction_input(p54spi_gpio_irq); > >> - > >> - ret = request_irq(gpio_to_irq(p54spi_gpio_irq), > >> - p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", > >> - priv->spi); > >> + ret = request_irq(spi->irq, p54spi_interrupt, IRQF_NO_AUTOEN, "p54spi", priv->spi); > > > > Simon says: This one seems more concerning in the context of this patch-set. > > > > Is there missing validation for spi->irq before calling request_irq()? > > > > If the device tree node lacks an interrupts property, the SPI core may set > > spi->irq to 0 or a negative error code. > > > > Passing this directly to request_irq() without validation can successfully > > hijack a core system interrupt like the system timer on some architectures, > > leading to system instability or deadlocks. > > > > Should there be a check like if (spi->irq <= 0) to fail the probe gracefully? > > I also commented on this: request_irq() already fails gracefully > with -EINVAL when presented with an invalid IRQ. IRQ 0 is guaranteed > to be invalid on any target that uses devicetree. Thanks, and sorry for not seeing your earlier comment. Or realising it is a false positive. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/3] [v5 omap] ARM: dts: omap2: add stlc4560 spi-wireless node 2026-05-07 21:24 [PATCH 0/3] [v5 net-next] wireless: p54 devicetree conversion Arnd Bergmann 2026-05-07 21:24 ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Arnd Bergmann 2026-05-07 21:24 ` [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Arnd Bergmann @ 2026-05-07 21:24 ` Arnd Bergmann 2 siblings, 0 replies; 15+ messages in thread From: Arnd Bergmann @ 2026-05-07 21:24 UTC (permalink / raw) To: netdev Cc: Arnd Bergmann, Aaro Koskinen, Andreas Kemnade, Bartosz Golaszewski, Benoît Cousson, David S. Miller, Dmitry Torokhov, Eric Dumazet, Felipe Balbi, Jakub Kicinski, Johannes Berg, Kevin Hilman, Krzysztof Kozlowski, Linus Walleij, Paolo Abeni, Rob Herring, Roger Quadros, Tony Lindgren, linux-wireless, devicetree, linux-kernel, linux-arm-kernel, linux-gpio, linux-omap, Krzysztof Kozlowski From: Arnd Bergmann <arnd@arndb.de> Converted from the platform_device creation in board-n8x0.c. Link: https://lore.kernel.org/all/20230314163201.955689-1-arnd@kernel.org/ Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org> Reviewed-by: Linus Walleij <linusw@kernel.org> Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- arch/arm/boot/dts/ti/omap/omap2.dtsi | 4 ++++ arch/arm/boot/dts/ti/omap/omap2420-n8x0-common.dtsi | 12 ++++++++++++ 2 files changed, 16 insertions(+) diff --git a/arch/arm/boot/dts/ti/omap/omap2.dtsi b/arch/arm/boot/dts/ti/omap/omap2.dtsi index afabb36a8ac1..fdc1790adf43 100644 --- a/arch/arm/boot/dts/ti/omap/omap2.dtsi +++ b/arch/arm/boot/dts/ti/omap/omap2.dtsi @@ -129,6 +129,8 @@ i2c2: i2c@48072000 { }; mcspi1: spi@48098000 { + #address-cells = <1>; + #size-cells = <0>; compatible = "ti,omap2-mcspi"; ti,hwmods = "mcspi1"; reg = <0x48098000 0x100>; @@ -140,6 +142,8 @@ mcspi1: spi@48098000 { }; mcspi2: spi@4809a000 { + #address-cells = <1>; + #size-cells = <0>; compatible = "ti,omap2-mcspi"; ti,hwmods = "mcspi2"; reg = <0x4809a000 0x100>; diff --git a/arch/arm/boot/dts/ti/omap/omap2420-n8x0-common.dtsi b/arch/arm/boot/dts/ti/omap/omap2420-n8x0-common.dtsi index 63b0b4921e4e..fe9dd8bbfc85 100644 --- a/arch/arm/boot/dts/ti/omap/omap2420-n8x0-common.dtsi +++ b/arch/arm/boot/dts/ti/omap/omap2420-n8x0-common.dtsi @@ -109,3 +109,15 @@ partition@5 { }; }; }; + +&mcspi2 { + status = "okay"; + + wifi@0 { + reg = <0>; + compatible = "st,stlc4560"; + spi-max-frequency = <48000000>; + interrupts-extended = <&gpio3 23 IRQ_TYPE_EDGE_RISING>; + powerdown-gpios = <&gpio4 1 GPIO_ACTIVE_LOW>; /* gpio 97 */ + }; +}; -- 2.39.5 ^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2026-05-12 9:40 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-07 21:24 [PATCH 0/3] [v5 net-next] wireless: p54 devicetree conversion Arnd Bergmann 2026-05-07 21:24 ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Arnd Bergmann 2026-05-08 15:05 ` Conor Dooley 2026-05-08 21:32 ` sashiko-bot 2026-05-09 15:56 ` Conor Dooley 2026-05-09 16:31 ` Roman Gushchin 2026-05-09 18:08 ` Conor Dooley 2026-05-07 21:24 ` [PATCH 2/3] [v5 net-next] p54spi: convert to devicetree Arnd Bergmann 2026-05-08 21:32 ` sashiko-bot 2026-05-09 17:08 ` Arnd Bergmann 2026-05-11 11:30 ` Bartosz Golaszewski 2026-05-11 16:12 ` Simon Horman 2026-05-11 19:45 ` Arnd Bergmann 2026-05-12 9:40 ` Simon Horman 2026-05-07 21:24 ` [PATCH 3/3] [v5 omap] ARM: dts: omap2: add stlc4560 spi-wireless node Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox