Re: [PATCH v9 02/24] dt-bindings: introduce silabs,wfx.yaml

On Tuesday 11 January 2022 23:19:56 CET Rob Herring wrote:

On Tue, Jan 11, 2022 at 06:14:02PM +0100, Jerome Pouiller wrote:

quoted

From: Jérôme Pouiller <jerome.pouiller@silabs.com>

Prepare the inclusion of the wfx driver in the kernel.

Signed-off-by: Jérôme Pouiller <jerome.pouiller@silabs.com>
---
 .../bindings/net/wireless/silabs,wfx.yaml     | 138 ++++++++++++++++++
 1 file changed, 138 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..d12f262868cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/silabs,wfx.yaml

[...]

quoted

+properties:
+  compatible:
+    anyOf:
+      - const: silabs,wf200    # Chip alone without antenna
+      - const: silabs,brd4001a # WGM160P Evaluation Board
+      - const: silabs,brd8022a # WF200 Evaluation Board
+      - const: silabs,brd8023a # WFM200 Evaluation Board

This still defines that compatible is a single entry. You need something
like:

items:
  - enum:
      - silabs,brd4001a
      - silabs,brd8022a
      - silabs,brd8023a
  - const: silabs,wf200

You need a separate 'items' list for different number of compatible
entries (e.g. if a single string is valid) and that is when you need to
use 'oneOf'. Plenty of examples in the tree.

Ok.

[...]

quoted

+  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

Unless there is a mode you can configure, supporting both is wrong even
though edge will mostly work for a device that is really level.

What a driver supports is not relevant to the binding.

hmm... right.

quoted

+      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")

What legacy? This is a new binding.

This driver already exist in staging/. But, it is probably the right moment
to drop this legacy binding.

[...]

-- 
Jérôme Pouiller