2021-12-14 16:33:33

by David Mosberger-Tang

[permalink] [raw]
Subject: [PATCH v4 0/2] Add reset/enable GPIO support to SPI driver

I made a mistake last night when checking whether gpiod_set_value() is
safe to call with a NULL gpiod descriptor (it is). v4 of the patch
just fixes that mistake. It does simplify the code nicely.

This version also fixes the error handling when the reset gpio is
missing.

David Mosberger-Tang (2):
wilc1000: Add reset/enable GPIO support to SPI driver
wilc1000: Document enable-gpios and reset-gpios properties

.../net/wireless/microchip,wilc1000.yaml | 17 ++++++
drivers/net/wireless/microchip/wilc1000/spi.c | 58 ++++++++++++++++++-
.../net/wireless/microchip/wilc1000/wlan.c | 2 +-
3 files changed, 73 insertions(+), 4 deletions(-)

--
2.25.1



2021-12-14 16:33:38

by David Mosberger-Tang

[permalink] [raw]
Subject: [PATCH v4 2/2] wilc1000: Document enable-gpios and reset-gpios properties

Add documentation for the ENABLE and RESET GPIOs that may be needed by
wilc1000-spi.

Signed-off-by: David Mosberger-Tang <[email protected]>
---
.../net/wireless/microchip,wilc1000.yaml | 17 +++++++++++++++++
1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
index 6c35682377e6..790a774a19ee 100644
--- a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
@@ -32,6 +32,21 @@ properties:
clock-names:
const: rtc

+ enable-gpios:
+ maxItems: 1
+ description: Used by wilc1000-spi to determine the GPIO line
+ connected to the ENABLE line. If specified, reset-gpios
+ must be specified as well as otherwise the driver cannot
+ ensure the timing required between asserting ENABLE
+ and deasserting RESET. This should be declared as an
+ active-high signal.
+
+ reset-gpios:
+ maxItems: 1
+ description: Used by wilc1000-spi to determine the GPIO line
+ connected to the RESET line. This should be declared as an
+ active-low signal.
+
required:
- compatible
- interrupts
@@ -51,6 +66,8 @@ examples:
interrupts = <27 0>;
clocks = <&pck1>;
clock-names = "rtc";
+ enable-gpios = <&pioA 5 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&pioA 6 GPIO_ACTIVE_LOW>;
};
};

--
2.25.1


2021-12-14 20:04:56

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] wilc1000: Document enable-gpios and reset-gpios properties

On Tue, 14 Dec 2021 16:33:22 +0000, David Mosberger-Tang wrote:
> Add documentation for the ENABLE and RESET GPIOs that may be needed by
> wilc1000-spi.
>
> Signed-off-by: David Mosberger-Tang <[email protected]>
> ---
> .../net/wireless/microchip,wilc1000.yaml | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dts:30.37-38 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1413: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1567796

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


2021-12-14 23:30:26

by David Mosberger-Tang

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] wilc1000: Document enable-gpios and reset-gpios properties

On Tue, 2021-12-14 at 14:04 -0600, Rob Herring wrote:
> On Tue, 14 Dec 2021 16:33:22 +0000, David Mosberger-Tang wrote:
> > Add documentation for the ENABLE and RESET GPIOs that may be needed by
> > wilc1000-spi.
> >
> > Signed-off-by: David Mosberger-Tang <[email protected]>
> > ---
> > .../net/wireless/microchip,wilc1000.yaml | 17 +++++++++++++++++
> > 1 file changed, 17 insertions(+)
> >
>
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
>
> yamllint warnings/errors:
>
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dts:30.37-38 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dt.yaml] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1413: dt_binding_check] Error 2

So this error appears due to GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW in these
lines:

enable-gpios = <&pioA 5 GPIO_ACTIVE_HIGH>;
reset-gpios = <&pioA 6 GPIO_ACTIVE_LOW>;

I can replace those with 0 and 1 respectively, but I doubt a lot of people would
recognize what those integers standard for. Is there a better way to get this
to pass?

--david


2021-12-14 23:54:34

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] wilc1000: Document enable-gpios and reset-gpios properties

On Tue, Dec 14, 2021 at 5:30 PM David Mosberger-Tang <[email protected]> wrote:
>
> On Tue, 2021-12-14 at 14:04 -0600, Rob Herring wrote:
> > On Tue, 14 Dec 2021 16:33:22 +0000, David Mosberger-Tang wrote:
> > > Add documentation for the ENABLE and RESET GPIOs that may be needed by
> > > wilc1000-spi.
> > >
> > > Signed-off-by: David Mosberger-Tang <[email protected]>
> > > ---
> > > .../net/wireless/microchip,wilc1000.yaml | 17 +++++++++++++++++
> > > 1 file changed, 17 insertions(+)
> > >
> >
> > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> >
> > yamllint warnings/errors:
> >
> > dtschema/dtc warnings/errors:
> > Error: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dts:30.37-38 syntax error
> > FATAL ERROR: Unable to parse input tree
> > make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dt.yaml] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1413: dt_binding_check] Error 2
>
> So this error appears due to GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW in these
> lines:
>
> enable-gpios = <&pioA 5 GPIO_ACTIVE_HIGH>;
> reset-gpios = <&pioA 6 GPIO_ACTIVE_LOW>;
>
> I can replace those with 0 and 1 respectively, but I doubt a lot of people would
> recognize what those integers standard for. Is there a better way to get this
> to pass?

Include the header(s) you use in the example.

Rob

2021-12-15 02:53:56

by David Mosberger-Tang

[permalink] [raw]
Subject: Re: [PATCH v4 2/2] wilc1000: Document enable-gpios and reset-gpios properties

On Tue, 2021-12-14 at 17:54 -0600, Rob Herring wrote:
> On Tue, Dec 14, 2021 at 5:30 PM David Mosberger-Tang <[email protected]> wrote:
> > On Tue, 2021-12-14 at 14:04 -0600, Rob Herring wrote:
> > >
> > > dtschema/dtc warnings/errors:
> > > Error: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dts:30.37-38 syntax error
> > > FATAL ERROR: Unable to parse input tree
> > > make[1]: *** [scripts/Makefile.lib:373: Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.example.dt.yaml] Error 1
> > > make[1]: *** Waiting for unfinished jobs....
> > > make: *** [Makefile:1413: dt_binding_check] Error 2
> >
> > So this error appears due to GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW in these
> > lines:
> >
> > enable-gpios = <&pioA 5 GPIO_ACTIVE_HIGH>;
> > reset-gpios = <&pioA 6 GPIO_ACTIVE_LOW>;
> >
> > I can replace those with 0 and 1 respectively, but I doubt a lot of people would
> > recognize what those integers standard for. Is there a better way to get this
> > to pass?
>
> Include the header(s) you use in the example.

Huh, that works, thanks!

--david