diff mbox series

[01/23] dt-bindings: introduce silabs,wfx.yaml

Message ID 20201012104648.985256-2-Jerome.Pouiller@silabs.com
State Changes Requested
Headers show
Series wfx: get out from the staging area | expand

Checks

Context Check Description
robh/dt-meta-schema success
robh/checkpatch success

Commit Message

Jérôme Pouiller Oct. 12, 2020, 10:46 a.m. UTC
From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 .../bindings/net/wireless/silabs,wfx.yaml     | 125 ++++++++++++++++++
 1 file changed, 125 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml

Comments

Rob Herring Oct. 13, 2020, 4:49 p.m. UTC | #1
On Mon, Oct 12, 2020 at 12:46:26PM +0200, Jerome Pouiller wrote:
> From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> 
> Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
> ---
>  .../bindings/net/wireless/silabs,wfx.yaml     | 125 ++++++++++++++++++
>  1 file changed, 125 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
> new file mode 100644
> index 000000000000..43b5630c0407
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
> @@ -0,0 +1,125 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2020, Silicon Laboratories, Inc.
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/net/wireless/silabs,wfx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Silicon Labs WFxxx devicetree bindings
> +
> +maintainers:
> +  - Jérôme Pouiller <jerome.pouiller@silabs.com>
> +
> +description:
> +  The WFxxx chip series can be connected via SPI or via SDIO.

What does this chip do? WiFi or some other wireless?

> +
> +  For SDIO':'
> +
> +    The driver is able to detect a WFxxx chip on SDIO bus by matching its Vendor
> +    ID and Product ID. However, driver will only provide limited features in
> +    this case. Thus declaring WFxxx chip in device tree is recommended (and may
> +    become mandatory in the future).
> +
> +    In addition, it is recommended to declare a mmc-pwrseq on SDIO host above
> +    WFx. Without it, you may encounter issues with warm boot. The mmc-pwrseq
> +    should be compatible with mmc-pwrseq-simple. Please consult
> +    Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt for more
> +    information.
> +
> +  For SPI':'
> +
> +    In add of the properties below, please consult
> +    Documentation/devicetree/bindings/spi/spi-controller.yaml for optional SPI
> +    related properties.
> +
> +  Note that in add of the properties below, the WFx driver also supports
> +  `mac-address` and `local-mac-address` as described in
> +  Documentation/devicetree/bindings/net/ethernet.txt

Note what ethernet.txt contains... This should have a $ref to 
ethernet-controller.yaml to express the above.

You can add 'mac-address: true' if you want to be explicit about what 
properties are used.

> +
> +properties:
> +  compatible:
> +    const: silabs,wf200

blank line between each DT property.

> +  reg:
> +    description:
> +      When used on SDIO bus, <reg> must be set to 1. When used on SPI bus, it is
> +      the chip select address of the device as defined in the SPI devices
> +      bindings.
> +    maxItems: 1
> +  spi-max-frequency:
> +    description: (SPI only) Maximum SPI clocking speed of device in Hz.

No need to redefine a common property.

> +    maxItems: 1

Not an array. Just need:

spi-max-frequency: true

> +  interrupts:
> +    description: The interrupt line. Triggers IRQ_TYPE_LEVEL_HIGH and
> +      IRQ_TYPE_EDGE_RISING are both supported by the chip and the driver. When
> +      SPI is used, this property is required. When SDIO is used, the "in-band"
> +      interrupt provided by the SDIO bus is used unless an interrupt is defined
> +      in the Device Tree.
> +    maxItems: 1
> +  reset-gpios:
> +    description: (SPI only) Phandle of gpio that will be used to reset chip
> +      during probe. Without this property, you may encounter issues with warm
> +      boot. (For legacy purpose, the gpio in inverted when compatible ==
> +      "silabs,wfx-spi")
> +
> +      For SDIO, the reset gpio should declared using a mmc-pwrseq.
> +    maxItems: 1
> +  wakeup-gpios:
> +    description: Phandle of gpio that will be used to wake-up chip. Without this
> +      property, driver will disable most of power saving features.
> +    maxItems: 1
> +  config-file:
> +    description: Use an alternative file as PDS. Default is `wf200.pds`. Only
> +      necessary for development/debug purpose.

'firmware-name' is typically what we'd use here. Though if just for 
debug/dev, perhaps do a debugfs interface for this instead. As DT should 
come from the firmware/bootloader, requiring changing the DT for 
dev/debug is not the easiest workflow compared to doing something from 
userspace.

> +    maxItems: 1

Looks like a string, not an array.

> +
> +required:
> +  - compatible
> +  - reg

Will need additionalProperties or unevaluatedProperties depending on 
whether you list out properties from ethernet-controller.yaml or not.

Rob
Jérôme Pouiller Oct. 14, 2020, 1:49 p.m. UTC | #2
On Tuesday 13 October 2020 18:49:35 CEST Rob Herring wrote:
> On Mon, Oct 12, 2020 at 12:46:26PM +0200, Jerome Pouiller wrote:
> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
[...]
> > +  Note that in add of the properties below, the WFx driver also supports
> > +  `mac-address` and `local-mac-address` as described in
> > +  Documentation/devicetree/bindings/net/ethernet.txt
> 
> Note what ethernet.txt contains... This should have a $ref to
> ethernet-controller.yaml to express the above.
> 
> You can add 'mac-address: true' if you want to be explicit about what
> properties are used.

Here, only mac-address and local-mac-address are supported. So, would the
code below do the job?

  local-mac-address:
    $ref: ethernet-controller.yaml#/properties/local-mac-address

  mac-address:
    $ref: ethernet-controller.yaml#/properties/mac-address


[...]
> > +  spi-max-frequency:
> > +    description: (SPI only) Maximum SPI clocking speed of device in Hz.
> 
> No need to redefine a common property.

When a property is specific to a bus, I would have like to explicitly
say it. That's why I redefined the description.


[...]
> > +  config-file:
> > +    description: Use an alternative file as PDS. Default is `wf200.pds`. Only
> > +      necessary for development/debug purpose.
> 
> 'firmware-name' is typically what we'd use here. Though if just for
> debug/dev, perhaps do a debugfs interface for this instead. As DT should
> come from the firmware/bootloader, requiring changing the DT for
> dev/debug is not the easiest workflow compared to doing something from
> userspace.

This file is not a firmware. It mainly contains data related to the
antenna. At the beginning, this property has been added for
development. With the time, I think it can be used to  have one disk
image for several devices that differ only in antenna.

I am going to remove the part about development/debug purpose.


[...]
> Will need additionalProperties or unevaluatedProperties depending on
> whether you list out properties from ethernet-controller.yaml or not.

I think I need to specify "additionalProperties: true" since the user can
also use properties defined for the SPI devices.

In fact, I would like to write something like:

    allOf:
        $ref: spi-controller.yaml#/patternProperties/^.*@[0-9a-f]+$/properties
Kalle Valo Nov. 2, 2020, 3:58 p.m. UTC | #3
Jérôme Pouiller <jerome.pouiller@silabs.com> writes:

> On Tuesday 13 October 2020 18:49:35 CEST Rob Herring wrote:
>> On Mon, Oct 12, 2020 at 12:46:26PM +0200, Jerome Pouiller wrote:
>> > From: Jérôme Pouiller <jerome.pouiller@silabs.com>
> [...]
>> > +  Note that in add of the properties below, the WFx driver also supports
>> > +  `mac-address` and `local-mac-address` as described in
>> > +  Documentation/devicetree/bindings/net/ethernet.txt
>> 
>> Note what ethernet.txt contains... This should have a $ref to
>> ethernet-controller.yaml to express the above.
>> 
>> You can add 'mac-address: true' if you want to be explicit about what
>> properties are used.
>
> Here, only mac-address and local-mac-address are supported. So, would the
> code below do the job?
>
>   local-mac-address:
>     $ref: ethernet-controller.yaml#/properties/local-mac-address
>
>   mac-address:
>     $ref: ethernet-controller.yaml#/properties/mac-address
>
>
> [...]
>> > +  spi-max-frequency:
>> > +    description: (SPI only) Maximum SPI clocking speed of device in Hz.
>> 
>> No need to redefine a common property.
>
> When a property is specific to a bus, I would have like to explicitly
> say it. That's why I redefined the description.
>
>
> [...]
>> > +  config-file:
>> > +    description: Use an alternative file as PDS. Default is `wf200.pds`. Only
>> > +      necessary for development/debug purpose.
>> 
>> 'firmware-name' is typically what we'd use here. Though if just for
>> debug/dev, perhaps do a debugfs interface for this instead. As DT should
>> come from the firmware/bootloader, requiring changing the DT for
>> dev/debug is not the easiest workflow compared to doing something from
>> userspace.
>
> This file is not a firmware. It mainly contains data related to the
> antenna. At the beginning, this property has been added for
> development. With the time, I think it can be used to  have one disk
> image for several devices that differ only in antenna.
>
> I am going to remove the part about development/debug purpose.

config-file doesn't sound right either. So what kind of data is this,
calibration data or what?
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
new file mode 100644
index 000000000000..43b5630c0407
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml
@@ -0,0 +1,125 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2020, Silicon Laboratories, Inc.
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/net/wireless/silabs,wfx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Silicon Labs WFxxx devicetree bindings
+
+maintainers:
+  - Jérôme Pouiller <jerome.pouiller@silabs.com>
+
+description:
+  The WFxxx chip series can be connected via SPI or via SDIO.
+
+  For SDIO':'
+
+    The driver is able to detect a WFxxx chip on SDIO bus by matching its Vendor
+    ID and Product ID. However, driver will only provide limited features in
+    this case. Thus declaring WFxxx chip in device tree is recommended (and may
+    become mandatory in the future).
+
+    In addition, it is recommended to declare a mmc-pwrseq on SDIO host above
+    WFx. Without it, you may encounter issues with warm boot. The mmc-pwrseq
+    should be compatible with mmc-pwrseq-simple. Please consult
+    Documentation/devicetree/bindings/mmc/mmc-pwrseq-simple.txt for more
+    information.
+
+  For SPI':'
+
+    In add of the properties below, please consult
+    Documentation/devicetree/bindings/spi/spi-controller.yaml for optional SPI
+    related properties.
+
+  Note that in add of the properties below, the WFx driver also supports
+  `mac-address` and `local-mac-address` as described in
+  Documentation/devicetree/bindings/net/ethernet.txt
+
+properties:
+  compatible:
+    const: silabs,wf200
+  reg:
+    description:
+      When used on SDIO bus, <reg> must be set to 1. When used on SPI bus, it is
+      the chip select address of the device as defined in the SPI devices
+      bindings.
+    maxItems: 1
+  spi-max-frequency:
+    description: (SPI only) Maximum SPI clocking speed of device in Hz.
+    maxItems: 1
+  interrupts:
+    description: The interrupt line. Triggers IRQ_TYPE_LEVEL_HIGH and
+      IRQ_TYPE_EDGE_RISING are both supported by the chip and the driver. When
+      SPI is used, this property is required. When SDIO is used, the "in-band"
+      interrupt provided by the SDIO bus is used unless an interrupt is defined
+      in the Device Tree.
+    maxItems: 1
+  reset-gpios:
+    description: (SPI only) Phandle of gpio that will be used to reset chip
+      during probe. Without this property, you may encounter issues with warm
+      boot. (For legacy purpose, the gpio in inverted when compatible ==
+      "silabs,wfx-spi")
+
+      For SDIO, the reset gpio should declared using a mmc-pwrseq.
+    maxItems: 1
+  wakeup-gpios:
+    description: Phandle of gpio that will be used to wake-up chip. Without this
+      property, driver will disable most of power saving features.
+    maxItems: 1
+  config-file:
+    description: Use an alternative file as PDS. Default is `wf200.pds`. Only
+      necessary for development/debug purpose.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        wfx@0 {
+            compatible = "silabs,wf200";
+            pinctrl-names = "default";
+            pinctrl-0 = <&wfx_irq &wfx_gpios>;
+            reg = <0>;
+            interrupts-extended = <&gpio 16 IRQ_TYPE_EDGE_RISING>;
+            wakeup-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
+            reset-gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
+            spi-max-frequency = <42000000>;
+        };
+    };
+
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    wfx_pwrseq: wfx_pwrseq {
+        compatible = "mmc-pwrseq-simple";
+        pinctrl-names = "default";
+        pinctrl-0 = <&wfx_reset>;
+        reset-gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
+    };
+
+    mmc0 {
+        mmc-pwrseq = <&wfx_pwrseq>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        mmc@1 {
+            compatible = "silabs,wf200";
+            pinctrl-names = "default";
+            pinctrl-0 = <&wfx_wakeup>;
+            reg = <1>;
+            wakeup-gpios = <&gpio 12 GPIO_ACTIVE_HIGH>;
+        };
+    };
+...