Re: [PATCH v4] staging: wfx: Get descriptors for GPIOs

2020-07-01 Thread Jérôme Pouiller
Hello Linus,

On Tuesday 30 June 2020 10:21:33 CEST Linus Walleij wrote:
> 
> The code has the functionality to insert the GPIO lines using
> the global GPIO numbers through module parameters.
> 
> As we are clearly deprecating the use of global GPIO numbers
> look up the GPIO descriptors from the device instead. This
> usually falls back to device hardware descriptions using e.g.
> device tree or ACPI. This device clearly supports device
> tree when used over SPI for example.
> 
> For example, this can be supplied in the device tree like so:
> 
>   wfx@0x01 {
>   compatible = "silabs,wf200";
>   reset-gpios = <&gpio0 1>;
>   wakeup-gpios = <&gpio0 2>;
>   };
> 
> Cc: Jérôme Pouiller 
> Signed-off-by: Linus Walleij 
> ---
> ChangeLog v3->v4:
> - Finally figured out how to compile this by selecting SPI
>   host and deselecting MMC host.

Weird, it should be selectable if MMC or SPI is selected.

> - Initialize the reset GPIO as OUT_LOW
> - Initialize the wakeup GPIO as OUT_LOW
> - Drop one more desc_to_gpio()
> - Update the warning if GPIO is not found.
> ChangeLog v2->v3:
> - ERR_CAST not PTR_CAST
> ChangeLog v1->v2:
> - Fixed a cast and a variable name.
> - I still don't know how to compile this but hey the zeroday
>   robot does.
> ---
>  drivers/staging/wfx/bus_spi.c | 14 +--
>  drivers/staging/wfx/main.c| 45 ---
>  drivers/staging/wfx/main.h|  2 --
>  3 files changed, 12 insertions(+), 49 deletions(-)
> 
[...]
> diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
> index 6bd96f476388..3828a2652313 100644
> --- a/drivers/staging/wfx/main.c
> +++ b/drivers/staging/wfx/main.c
[...]
> @@ -340,7 +303,10 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> memcpy(&wdev->pdata, pdata, sizeof(*pdata));
> of_property_read_string(dev->of_node, "config-file",
> &wdev->pdata.file_pds);
> -   wdev->pdata.gpio_wakeup = wfx_get_gpio(dev, gpio_wakeup, "wakeup");
> +   wdev->pdata.gpio_wakeup = devm_gpiod_get(dev, "wakeup", 
> GPIOD_OUT_LOW);
> +   if (IS_ERR(wdev->pdata.gpio_wakeup))
> +   return ERR_CAST(wdev->pdata.gpio_wakeup);
> +   gpiod_set_consumer_name(wdev->pdata.gpio_wakeup, "wfx wakeup");

In the original code, the gpio wakeup was not mandatory. But in this code,
wfx_init_common() will return an error if the gpio "wakeup" is not
defined.

> wfx_sl_fill_pdata(dev, &wdev->pdata);
> 
> mutex_init(&wdev->conf_mutex);
[...]

-- 
Jérôme Pouiller


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH v4] staging: wfx: Get descriptors for GPIOs

2020-06-30 Thread Linus Walleij
The code has the functionality to insert the GPIO lines using
the global GPIO numbers through module parameters.

As we are clearly deprecating the use of global GPIO numbers
look up the GPIO descriptors from the device instead. This
usually falls back to device hardware descriptions using e.g.
device tree or ACPI. This device clearly supports device
tree when used over SPI for example.

For example, this can be supplied in the device tree like so:

  wfx@0x01 {
  compatible = "silabs,wf200";
  reset-gpios = <&gpio0 1>;
  wakeup-gpios = <&gpio0 2>;
  };

Cc: Jérôme Pouiller 
Signed-off-by: Linus Walleij 
---
ChangeLog v3->v4:
- Finally figured out how to compile this by selecting SPI
  host and deselecting MMC host.
- Initialize the reset GPIO as OUT_LOW
- Initialize the wakeup GPIO as OUT_LOW
- Drop one more desc_to_gpio()
- Update the warning if GPIO is not found.
ChangeLog v2->v3:
- ERR_CAST not PTR_CAST
ChangeLog v1->v2:
- Fixed a cast and a variable name.
- I still don't know how to compile this but hey the zeroday
  robot does.
---
 drivers/staging/wfx/bus_spi.c | 14 +--
 drivers/staging/wfx/main.c| 45 ---
 drivers/staging/wfx/main.h|  2 --
 3 files changed, 12 insertions(+), 49 deletions(-)

diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
index e8da61fb096b..d19c0478e8be 100644
--- a/drivers/staging/wfx/bus_spi.c
+++ b/drivers/staging/wfx/bus_spi.c
@@ -8,7 +8,6 @@
  */
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -21,10 +20,6 @@
 #include "main.h"
 #include "bh.h"
 
-static int gpio_reset = -2;
-module_param(gpio_reset, int, 0644);
-MODULE_PARM_DESC(gpio_reset, "gpio number for reset. -1 for none.");
-
 #define SET_WRITE 0x7FFF/* usage: and operation */
 #define SET_READ 0x8000 /* usage: or operation */
 
@@ -211,10 +206,15 @@ static int wfx_spi_probe(struct spi_device *func)
bus->need_swab = true;
spi_set_drvdata(func, bus);
 
-   bus->gpio_reset = wfx_get_gpio(&func->dev, gpio_reset, "reset");
+   bus->gpio_reset = devm_gpiod_get_optional(&func->dev, "reset",
+ GPIOD_OUT_LOW);
+   if (IS_ERR(bus->gpio_reset))
+   return PTR_ERR(bus->gpio_reset);
if (!bus->gpio_reset) {
-   dev_warn(&func->dev, "try to load firmware anyway\n");
+   dev_warn(&func->dev,
+"gpio reset is not defined, trying to load firmware 
anyway\n");
} else {
+   gpiod_set_consumer_name(bus->gpio_reset, "wfx reset");
if (spi_get_device_id(func)->driver_data & WFX_RESET_INVERTED)
gpiod_toggle_active_low(bus->gpio_reset);
gpiod_set_value_cansleep(bus->gpio_reset, 1);
diff --git a/drivers/staging/wfx/main.c b/drivers/staging/wfx/main.c
index 6bd96f476388..3828a2652313 100644
--- a/drivers/staging/wfx/main.c
+++ b/drivers/staging/wfx/main.c
@@ -13,7 +13,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -41,10 +40,6 @@ MODULE_DESCRIPTION("Silicon Labs 802.11 Wireless LAN driver 
for WFx");
 MODULE_AUTHOR("Jérôme Pouiller ");
 MODULE_LICENSE("GPL");
 
-static int gpio_wakeup = -2;
-module_param(gpio_wakeup, int, 0644);
-MODULE_PARM_DESC(gpio_wakeup, "gpio number for wakeup. -1 for none.");
-
 #define RATETAB_ENT(_rate, _rateid, _flags) { \
.bitrate  = (_rate),   \
.hw_value = (_rateid), \
@@ -170,38 +165,6 @@ bool wfx_api_older_than(struct wfx_dev *wdev, int major, 
int minor)
return false;
 }
 
-struct gpio_desc *wfx_get_gpio(struct device *dev,
-  int override, const char *label)
-{
-   struct gpio_desc *ret;
-   char label_buf[256];
-
-   if (override >= 0) {
-   snprintf(label_buf, sizeof(label_buf), "wfx_%s", label);
-   ret = ERR_PTR(devm_gpio_request_one(dev, override,
-   GPIOF_OUT_INIT_LOW,
-   label_buf));
-   if (!ret)
-   ret = gpio_to_desc(override);
-   } else if (override == -1) {
-   ret = NULL;
-   } else {
-   ret = devm_gpiod_get(dev, label, GPIOD_OUT_LOW);
-   }
-   if (IS_ERR_OR_NULL(ret)) {
-   if (!ret || PTR_ERR(ret) == -ENOENT)
-   dev_warn(dev, "gpio %s is not defined\n", label);
-   else
-   dev_warn(dev, "error while requesting gpio %s\n",
-label);
-   ret = NULL;
-   } else {
-   dev_dbg(dev, "using gpio %d for %s\n",
-   desc_to_gpio(ret), label);
-   }
-   return ret;
-}
-
 /* NOTE: wfx_send_pds() destroy buf */
 int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
 {
@@ -340,7 +303,10 @@ struct wfx_dev *wfx_init_comm