diff mbox series

[v7,1/2] wilc1000: Add reset/enable GPIO support to SPI driver

Message ID 20211221212531.4011609-2-davidm@egauge.net (mailing list archive)
State Accepted
Commit ec031ac4792c72d9e925313366df8064e092f685
Delegated to: Kalle Valo
Headers show
Series wilc1000: Add reset/enable GPIO support to SPI driver | expand

Commit Message

David Mosberger-Tang Dec. 21, 2021, 9:25 p.m. UTC
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(-)

Comments

Claudiu Beznea Dec. 22, 2021, 6:21 a.m. UTC | #1
On 21.12.2021 23:25, David Mosberger-Tang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> 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>

Also, from v5:
Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

> ---
>  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 5ace9e3a56fc8..2c2ed4b09efd5 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);
> @@ -983,9 +1036,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;
>  }
> 
> @@ -1006,6 +1060,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 3f339c2f46f11..1a37a49fe6477 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan.c
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
> @@ -1254,7 +1254,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);
>  }
> 
>  static int wilc_wlan_cfg_commit(struct wilc_vif *vif, int type,
> --
> 2.25.1
>
Kalle Valo Dec. 22, 2021, 5:51 p.m. UTC | #2
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>
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

2 patches applied to wireless-drivers-next.git, thanks.

ec031ac4792c wilc1000: Add reset/enable GPIO support to SPI driver
f31ee3c0a555 wilc1000: Document enable-gpios and reset-gpios properties
diff mbox series

Patch

diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
index 5ace9e3a56fc8..2c2ed4b09efd5 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);
@@ -983,9 +1036,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;
 }
 
@@ -1006,6 +1060,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 3f339c2f46f11..1a37a49fe6477 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
@@ -1254,7 +1254,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);
 }
 
 static int wilc_wlan_cfg_commit(struct wilc_vif *vif, int type,