Sashiko discussions
 help / color / mirror / Atom feed
* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding
       [not found] ` <20260508213255.60491C2BCB0@smtp.kernel.org>
@ 2026-05-09 15:56   ` Conor Dooley
  2026-05-09 16:31     ` Roman Gushchin
  0 siblings, 1 reply; 3+ 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] 3+ messages in thread

* Re: [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding
  2026-05-09 15:56   ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Conor Dooley
@ 2026-05-09 16:31     ` Roman Gushchin
  2026-05-09 18:08       ` Conor Dooley
  0 siblings, 1 reply; 3+ 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] 3+ 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; 3+ 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] 3+ messages in thread

end of thread, other threads:[~2026-05-09 18:08 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260507212451.3333185-2-arnd@kernel.org>
     [not found] ` <20260508213255.60491C2BCB0@smtp.kernel.org>
2026-05-09 15:56   ` [PATCH 1/3] [v5 net-next] dt-bindings: net: add st,stlc4560/p54spi binding Conor Dooley
2026-05-09 16:31     ` Roman Gushchin
2026-05-09 18:08       ` Conor Dooley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox