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

["Jérôme Pouiller" <jerome.pouiller@silabs.com>]

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:
> > 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

[...]

> > +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.

[...]

> > +  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.

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