From: <Claudiu.Beznea@microchip.com>
To: <davidm@egauge.net>, <Ajay.Kathat@microchip.com>
Cc: <kvalo@codeaurora.org>, <davem@davemloft.net>, <kuba@kernel.org>,
<linux-wireless@vger.kernel.org>, <netdev@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <adham.abozaeid@microchip.com>,
<devicetree@vger.kernel.org>
Subject: Re: [PATCH v2 1/2] wilc1000: Add reset/enable GPIO support to SPI driver
Date: Mon, 13 Dec 2021 06:32:26 +0000 [thread overview]
Message-ID: <0eba4b4c-7c00-291a-e9a3-c8c45fe4be86@microchip.com> (raw)
In-Reply-To: <20211208061559.3404738-2-davidm@egauge.net>
On 08.12.2021 08:16, 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>
> ---
Please specify what have been changed since previous version either here
under "---" or in cover letter.
> drivers/net/wireless/microchip/wilc1000/spi.c | 36 +++++++++++++++++--
> .../net/wireless/microchip/wilc1000/wlan.c | 2 +-
> 2 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/microchip/wilc1000/spi.c b/drivers/net/wireless/microchip/wilc1000/spi.c
> index 640850f989dd..37215fcc27e0 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/of_gpio.h>
GPIO descriptors are covered by <linux/gpio/consumer.h>
>
> #include "netdev.h"
> #include "cfg80211.h"
> @@ -152,6 +153,31 @@ struct wilc_spi_special_cmd_rsp {
> u8 status;
> } __packed;
>
> +static void wilc_set_enable(struct spi_device *spi, bool on)
I liked better wilc_wlan_power().
> +{
> + int enable_gpio, reset_gpio;
> +
> + enable_gpio = of_get_named_gpio(spi->dev.of_node, "chip_en-gpios", 0);
> + reset_gpio = of_get_named_gpio(spi->dev.of_node, "reset-gpios", 0);
The equivalent of of_get_named_gpio(), device managed, is
devm_gpiod_get_from_of_node() and could be used on behalf of spi device.
But I presume devm_gpiod_get()/devm_gpiod_get_optional() could also be used
for your use case.
And I would keep the parsing just one time, at probe.
> +
> + if (on) {
> + if (gpio_is_valid(enable_gpio))
> + /* assert ENABLE */
> + gpio_direction_output(enable_gpio, 1);
> + mdelay(5); /* 5ms delay required by WILC1000 */
> + if (gpio_is_valid(reset_gpio))
> + /* deassert RESET */
> + gpio_direction_output(reset_gpio, 1);
> + } else {
> + if (gpio_is_valid(reset_gpio))
> + /* assert RESET */
> + gpio_direction_output(reset_gpio, 0);
> + if (gpio_is_valid(enable_gpio))
> + /* deassert ENABLE */
> + gpio_direction_output(enable_gpio, 0);
> + }
With gpio descriptors as far as I can tell you don't have to explicitly
check the validity of gpio as it is embedded in gpiod_direction_output()
specific function thus the above code may become:
if (on) {
gpiod_direction_output(enable_gpio, on);
mdelay(5);
gpiod_direction_output(reset_gpio, on);
} else {
gpiod_direction_output(reset_gpio, on);
gpiod_direction_output(enable_gpio, on);
}
> +}
> +
> static int wilc_bus_probe(struct spi_device *spi)
> {
> int ret;
> @@ -977,9 +1003,11 @@ static int wilc_spi_reset(struct wilc *wilc)
>
> static int wilc_spi_deinit(struct wilc *wilc)
> {
> - /*
> - * TODO:
> - */
> + struct spi_device *spi = to_spi_device(wilc->dev);
> + struct wilc_spi *spi_priv = wilc->bus_data;
> +
> + spi_priv->isinit = false;
> + wilc_set_enable(spi, false);
> return 0;
> }
>
> @@ -1000,6 +1028,8 @@ static int wilc_spi_init(struct wilc *wilc, bool resume)
> dev_err(&spi->dev, "Fail cmd read chip id...\n");
> }
>
> + wilc_set_enable(spi, true);
> +
> /*
> * configure protocol
> */
> diff --git a/drivers/net/wireless/microchip/wilc1000/wlan.c b/drivers/net/wireless/microchip/wilc1000/wlan.c
> index 82566544419a..f1e4ac3a2ad5 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan.c
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan.c
> @@ -1253,7 +1253,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
>
next prev parent reply other threads:[~2021-12-13 6:32 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-08 6:16 [PATCH v2 0/2] wilc1000-spi: Add reset/enable GPIO support David Mosberger-Tang
2021-12-08 6:16 ` [PATCH v2 1/2] wilc1000: Add reset/enable GPIO support to SPI driver David Mosberger-Tang
2021-12-13 6:32 ` Claudiu.Beznea [this message]
2021-12-08 6:16 ` [PATCH v2 2/2] wilc1000: Document enable-gpios and reset-gpios properties David Mosberger-Tang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0eba4b4c-7c00-291a-e9a3-c8c45fe4be86@microchip.com \
--to=claudiu.beznea@microchip.com \
--cc=Ajay.Kathat@microchip.com \
--cc=adham.abozaeid@microchip.com \
--cc=davem@davemloft.net \
--cc=davidm@egauge.net \
--cc=devicetree@vger.kernel.org \
--cc=kuba@kernel.org \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).