[PATCH v6 0/2] wilc1000: Add reset/enable GPIO support to SPI driver

[PATCH v6 0/2] wilc1000: Add reset/enable GPIO support to SPI driver

[David Mosberger-Tang]

v6:
	- Convert line comments to block comments.
v5:
	- Fix a dt_binding_check error by including
	  <dt-bindings/gpio/gpio.h> in microchip,wilc1000.yaml.
v4:
	- Simplify wilc_wlan_power() by letting gpiod_set_value()
	  handle NULL gpios.
v3:
	- Fix to include correct header file.
	- Rename wilc_set_enable() to wilc_wlan_power().
	- Use devm_gpiod_get{,_optional}() instead of of_get_named_gpio()
	- Parse GPIO pins once at probe time.
v2:
	- Split documentation update and driver changes into seprate
          patches.

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      | 19 ++++++
 drivers/net/wireless/microchip/wilc1000/spi.c | 62 ++++++++++++++++++-
 .../net/wireless/microchip/wilc1000/wlan.c    |  2 +-
 3 files changed, 79 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v6 2/2] wilc1000: Document enable-gpios and reset-gpios properties

[David Mosberger-Tang]

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

Signed-off-by: David Mosberger-Tang <davidm@egauge.net>
---
 .../net/wireless/microchip,wilc1000.yaml      | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml b/Documentation/devicetree/bindings/net/wireless/microchip,wilc1000.yaml
index 6c35682377e6d..60de78f1bc7b9 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
@@ -40,6 +55,8 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/gpio/gpio.h>
+
     spi {
       #address-cells = <1>;
       #size-cells = <0>;
@@ -51,6 +68,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

Re: [PATCH v6 2/2] wilc1000: Document enable-gpios and reset-gpios properties

[Rob Herring]

On Mon, 20 Dec 2021 18:03:38 +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 <davidm@egauge.net>
> ---
>  .../net/wireless/microchip,wilc1000.yaml      | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 


Please add Acked-by/Reviewed-by tags when posting new versions. However,
there's no need to repost patches *only* to add the tags. The upstream
maintainer will do that for acks received on the version they apply.

If a tag was not added on purpose, please state why and what changed.

Re: [PATCH v6 2/2] wilc1000: Document enable-gpios and reset-gpios properties

[David Mosberger-Tang]

On Tue, 2021-12-21 at 11:12 -0400, Rob Herring wrote:
> On Mon, 20 Dec 2021 18:03:38 +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 <davidm@egauge.net>
> > ---
> >  .../net/wireless/microchip,wilc1000.yaml      | 19
> > +++++++++++++++++++
> >  1 file changed, 19 insertions(+)
> > 
> 
> Please add Acked-by/Reviewed-by tags when posting new versions. 

Ah, sorry about that.

> However,
> there's no need to repost patches *only* to add the tags. The
> upstream
> maintainer will do that for acks received on the version they apply.
> 
> If a tag was not added on purpose, please state why and what changed.

Not on purpose.  I just didn't know how this is handled.

Thanks and best regards,

  --david

Re: [PATCH v6 2/2] wilc1000: Document enable-gpios and reset-gpios properties

[Kalle Valo]

David Mosberger-Tang <davidm@egauge.net> writes:

> On Tue, 2021-12-21 at 11:12 -0400, Rob Herring wrote:
>> On Mon, 20 Dec 2021 18:03:38 +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 <davidm@egauge.net>
>> > ---
>> >  .../net/wireless/microchip,wilc1000.yaml      | 19
>> > +++++++++++++++++++
>> >  1 file changed, 19 insertions(+)
>> > 
>> 
>> Please add Acked-by/Reviewed-by tags when posting new versions. 
>
> Ah, sorry about that.
>
>> However,
>> there's no need to repost patches *only* to add the tags. The
>> upstream
>> maintainer will do that for acks received on the version they apply.
>> 
>> If a tag was not added on purpose, please state why and what changed.
>
> Not on purpose.  I just didn't know how this is handled.

No worries, we have a lot of special rules :) I'll add Rob's tag from v5
when I commit the patch.

Actually, I'm lazy and patchwork can pick it up automatically:

Reviewed-by: Rob Herring <robh@kernel.org>

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH v6 1/2] wilc1000: Add reset/enable GPIO support to SPI driver

[David Mosberger-Tang]

For the SDIO driver, the RESET/ENABLE pins of WILC1000 are controlled
through the SDIO power sequence driver.  This commit adds analogous
support for the SPI driver.  Specifically, during initialization, the
chip will be ENABLEd and taken out of RESET and during
deinitialization, the chip will be placed back into RESET and disabled
(both to reduce power consumption and to ensure the WiFi radio is
off).

Both RESET and ENABLE GPIOs are optional.  However, if the ENABLE GPIO
is specified, then the RESET GPIO should normally also be specified as
otherwise there is no way to ensure proper timing of the ENABLE/RESET
sequence.

Signed-off-by: David Mosberger-Tang <davidm@egauge.net>
---
 drivers/net/wireless/microchip/wilc1000/spi.c | 62 ++++++++++++++++++-
 .../net/wireless/microchip/wilc1000/wlan.c    |  2 +-
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index e0871b89917dd..86233982120a8 100644
--- a/drivers/net/wireless/microchip/wilc1000/spi.c
+++ b/drivers/net/wireless/microchip/wilc1000/spi.c
@@ -8,6 +8,7 @@
 #include <linux/spi/spi.h>
 #include <linux/crc7.h>
 #include <linux/crc-itu-t.h>
+#include <linux/gpio/consumer.h>
 
 #include "netdev.h"
 #include "cfg80211.h"
@@ -45,6 +46,10 @@ struct wilc_spi {
 	bool probing_crc;	/* true if we're probing chip's CRC config */
 	bool crc7_enabled;	/* true if crc7 is currently enabled */
 	bool crc16_enabled;	/* true if crc16 is currently enabled */
+	struct wilc_gpios {
+		struct gpio_desc *enable;	/* ENABLE GPIO or NULL */
+		struct gpio_desc *reset;	/* RESET GPIO or NULL */
+	} gpios;
 };
 
 static const struct wilc_hif_func wilc_hif_spi;
@@ -152,6 +157,50 @@ struct wilc_spi_special_cmd_rsp {
 	u8 status;
 } __packed;
 
+static int wilc_parse_gpios(struct wilc *wilc)
+{
+	struct spi_device *spi = to_spi_device(wilc->dev);
+	struct wilc_spi *spi_priv = wilc->bus_data;
+	struct wilc_gpios *gpios = &spi_priv->gpios;
+
+	/* get ENABLE pin and deassert it (if it is defined): */
+	gpios->enable = devm_gpiod_get_optional(&spi->dev,
+						"enable", GPIOD_OUT_LOW);
+	/* get RESET pin and assert it (if it is defined): */
+	if (gpios->enable) {
+		/* if enable pin exists, reset must exist as well */
+		gpios->reset = devm_gpiod_get(&spi->dev,
+					      "reset", GPIOD_OUT_HIGH);
+		if (IS_ERR(gpios->reset)) {
+			dev_err(&spi->dev, "missing reset gpio.\n");
+			return PTR_ERR(gpios->reset);
+		}
+	} else {
+		gpios->reset = devm_gpiod_get_optional(&spi->dev,
+						       "reset", GPIOD_OUT_HIGH);
+	}
+	return 0;
+}
+
+static void wilc_wlan_power(struct wilc *wilc, bool on)
+{
+	struct wilc_spi *spi_priv = wilc->bus_data;
+	struct wilc_gpios *gpios = &spi_priv->gpios;
+
+	if (on) {
+		/* assert ENABLE: */
+		gpiod_set_value(gpios->enable, 1);
+		mdelay(5);
+		/* deassert RESET: */
+		gpiod_set_value(gpios->reset, 0);
+	} else {
+		/* assert RESET: */
+		gpiod_set_value(gpios->reset, 1);
+		/* deassert ENABLE: */
+		gpiod_set_value(gpios->enable, 0);
+	}
+}
+
 static int wilc_bus_probe(struct spi_device *spi)
 {
 	int ret;
@@ -171,6 +220,10 @@ static int wilc_bus_probe(struct spi_device *spi)
 	wilc->bus_data = spi_priv;
 	wilc->dev_irq_num = spi->irq;
 
+	ret = wilc_parse_gpios(wilc);
+	if (ret < 0)
+		goto netdev_cleanup;
+
 	wilc->rtc_clk = devm_clk_get_optional(&spi->dev, "rtc");
 	if (IS_ERR(wilc->rtc_clk)) {
 		ret = PTR_ERR(wilc->rtc_clk);
@@ -981,9 +1034,10 @@ static int wilc_spi_reset(struct wilc *wilc)
 
 static int wilc_spi_deinit(struct wilc *wilc)
 {
-	/*
-	 * TODO:
-	 */
+	struct wilc_spi *spi_priv = wilc->bus_data;
+
+	spi_priv->isinit = false;
+	wilc_wlan_power(wilc, false);
 	return 0;
 }
 
@@ -1004,6 +1058,8 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
 		dev_err(&spi->dev, "Fail cmd read chip id...\n");
 	}
 
+	wilc_wlan_power(wilc, true);
+
 	/*
 	 * configure protocol
 	 */
diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
index 4ab391b1dd8c7..227ed939c0e6b 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -1250,7 +1250,7 @@ void wilc_wlan_cleanup(struct net_device *dev)
 	wilc->rx_buffer = NULL;
 	kfree(wilc->tx_buffer);
 	wilc->tx_buffer = NULL;
-	wilc->hif_func->hif_deinit(NULL);
+	wilc->hif_func->hif_deinit(wilc);
 }
 
 struct sk_buff *wilc_wlan_alloc_skb(struct wilc_vif *vif, size_t len)
-- 
2.25.1

Re: [PATCH v6 1/2] wilc1000: Add reset/enable GPIO support to SPI driver

[Kalle Valo]

David Mosberger-Tang <davidm@egauge.net> wrote:

> For the SDIO driver, the RESET/ENABLE pins of WILC1000 are controlled
> through the SDIO power sequence driver.  This commit adds analogous
> support for the SPI driver.  Specifically, during initialization, the
> chip will be ENABLEd and taken out of RESET and during
> deinitialization, the chip will be placed back into RESET and disabled
> (both to reduce power consumption and to ensure the WiFi radio is
> off).
> 
> Both RESET and ENABLE GPIOs are optional.  However, if the ENABLE GPIO
> is specified, then the RESET GPIO should normally also be specified as
> otherwise there is no way to ensure proper timing of the ENABLE/RESET
> sequence.
> 
> Signed-off-by: David Mosberger-Tang <davidm@egauge.net>

Failed to apply, please rebase on top of wireless-drivers-next.

error: sha1 information is lacking or useless (drivers/net/wireless/microchip/wilc1000/wlan.c).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch' to see the failed patch
Applying: wilc1000: Add reset/enable GPIO support to SPI driver
Patch failed at 0001 wilc1000: Add reset/enable GPIO support to SPI driver

2 patches set to Changes Requested.

12688345 [v6,1/2] wilc1000: Add reset/enable GPIO support to SPI driver
12688343 [v6,2/2] wilc1000: Document enable-gpios and reset-gpios properties

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20211220180334.3990693-2-davidm@egauge.net/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches