| Message ID | 20240806170018.638585-18-michael.nemanov@ti.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: cc33xx: Add driver for new TI CC33xx wireless device family | expand |
On 06/08/2024 19:00, Michael Nemanov wrote: Thank you for your patch. There is something to discuss/improve. > +properties: > + compatible: > + enum: > + - ti,cc3300 > + - ti,cc3301 > + - ti,cc3350 > + - ti,cc3351 > + > + reg: > + description: > + must be set to 2 Then just const: 2 and drop free form text. > + maxItems: 1 > + > + interrupts: > + description: > + The out-of-band interrupt line. > + Can be IRQ_TYPE_EDGE_RISING or IRQ_TYPE_LEVEL_HIGH. > + If property is omitted, SDIO in-band IRQ will be used. > + maxItems: 1 > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/irq.h> > + > + // SDIO example: Drop, obvious. > + mmc { > + #address-cells = <1>; > + #size-cells = <0>; > + > + wifi@1{ Missing space. Also, this does not match reg. Test your DTS with W=1 and FIX ALL warnings. Best regards, Krzysztof
On 06/08/2024 19:00, Michael Nemanov wrote: > Add device-tree bindings for the CC33xx family. > > Signed-off-by: Michael Nemanov <michael.nemanov@ti.com> > --- > .../bindings/net/wireless/ti,cc33xx.yaml | 56 +++++++++++++++++++ Also, bindings always come before users, but maybe the maintainers will re-order things since you expect here some squashing. Best regards, Krzysztof
On 8/7/2024 10:06 AM, Krzysztof Kozlowski wrote: > On 06/08/2024 19:00, Michael Nemanov wrote: > > Thank you for your patch. There is something to discuss/improve. > >> +properties: >> + compatible: >> + enum: >> + - ti,cc3300 >> + - ti,cc3301 >> + - ti,cc3350 >> + - ti,cc3351 >> + >> + reg: >> + description: >> + must be set to 2 > > Then just const: 2 and drop free form text. > >> + maxItems: 1 >> + >> + interrupts: >> + description: >> + The out-of-band interrupt line. >> + Can be IRQ_TYPE_EDGE_RISING or IRQ_TYPE_LEVEL_HIGH. >> + If property is omitted, SDIO in-band IRQ will be used. >> + maxItems: 1 >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/irq.h> >> + >> + // SDIO example: > > Drop, obvious. > >> + mmc { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + wifi@1{ > > Missing space. > > Also, this does not match reg. Test your DTS with W=1 and FIX ALL warnings. > > Best regards, > Krzysztof > Will fix all above. I'm currently testing my .yaml with: make dt_binding_check DT_CHECKER_FLAGS=-m \ DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/wireless/ti,cc33xx.yaml It reports no warnings. Adding W=1 doesn't seem to change anything. Am I missing something? Thanks and regards, Michael.
On 8/7/2024 10:06 AM, Krzysztof Kozlowski wrote: > On 06/08/2024 19:00, Michael Nemanov wrote: >> Add device-tree bindings for the CC33xx family. >> >> Signed-off-by: Michael Nemanov <michael.nemanov@ti.com> >> --- >> .../bindings/net/wireless/ti,cc33xx.yaml | 56 +++++++++++++++++++ > > Also, bindings always come before users, but maybe the maintainers will > re-order things since you expect here some squashing. > > Best regards, > Krzysztof > Will hoist the bindings to be 1st. Thanks and regards, Michael.
On 07/08/2024 17:51, Nemanov, Michael wrote: > On 8/7/2024 10:06 AM, Krzysztof Kozlowski wrote: >> On 06/08/2024 19:00, Michael Nemanov wrote: >> >> Thank you for your patch. There is something to discuss/improve. >> >>> +properties: >>> + compatible: >>> + enum: >>> + - ti,cc3300 >>> + - ti,cc3301 >>> + - ti,cc3350 >>> + - ti,cc3351 >>> + >>> + reg: >>> + description: >>> + must be set to 2 >> >> Then just const: 2 and drop free form text. >> >>> + maxItems: 1 >>> + >>> + interrupts: >>> + description: >>> + The out-of-band interrupt line. >>> + Can be IRQ_TYPE_EDGE_RISING or IRQ_TYPE_LEVEL_HIGH. >>> + If property is omitted, SDIO in-band IRQ will be used. >>> + maxItems: 1 >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + >>> + // SDIO example: >> >> Drop, obvious. >> >>> + mmc { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + wifi@1{ >> >> Missing space. >> >> Also, this does not match reg. Test your DTS with W=1 and FIX ALL warnings. >> >> Best regards, >> Krzysztof >> > > Will fix all above. > > I'm currently testing my .yaml with: > make dt_binding_check DT_CHECKER_FLAGS=-m \ > DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/wireless/ti,cc33xx.yaml > > It reports no warnings. Adding W=1 doesn't seem to change anything. Am I > missing something? I said test your DTS, not bindings. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/net/wireless/ti,cc33xx.yaml b/Documentation/devicetree/bindings/net/wireless/ti,cc33xx.yaml new file mode 100644 index 000000000000..402ff7ec9239 --- /dev/null +++ b/Documentation/devicetree/bindings/net/wireless/ti,cc33xx.yaml @@ -0,0 +1,56 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/wireless/ti,cc33xx.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments CC33xx Wireless LAN Controller + +maintainers: + - Michael Nemanov <michael.nemanov@ti.com> + +description: + The CC33xx is a family of IEEE 802.11ax chips from Texas Instruments. + These chips must be connected via SDIO and support in-band / out-of-band IRQ. + +properties: + compatible: + enum: + - ti,cc3300 + - ti,cc3301 + - ti,cc3350 + - ti,cc3351 + + reg: + description: + must be set to 2 + maxItems: 1 + + interrupts: + description: + The out-of-band interrupt line. + Can be IRQ_TYPE_EDGE_RISING or IRQ_TYPE_LEVEL_HIGH. + If property is omitted, SDIO in-band IRQ will be used. + maxItems: 1 + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/irq.h> + + // SDIO example: + mmc { + #address-cells = <1>; + #size-cells = <0>; + + wifi@1{ + compatible = "ti,cc3300"; + reg = <2>; + interrupts = <19 IRQ_TYPE_EDGE_RISING>; + }; + };
Add device-tree bindings for the CC33xx family. Signed-off-by: Michael Nemanov <michael.nemanov@ti.com> --- .../bindings/net/wireless/ti,cc33xx.yaml | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/wireless/ti,cc33xx.yaml