On Sunday 28 June 2020 10:52:36 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 <jerome.pouiller@xxxxxxxxxx>
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
> 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 | 11 +++++----
> drivers/staging/wfx/main.c | 42 ++++-------------------------------
> drivers/staging/wfx/main.h | 2 --
> 3 files changed, 9 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/staging/wfx/bus_spi.c b/drivers/staging/wfx/bus_spi.c
> index e8da61fb096b..88ca5d453e83 100644
> --- a/drivers/staging/wfx/bus_spi.c
> +++ b/drivers/staging/wfx/bus_spi.c
> @@ -8,7 +8,6 @@
> */
> #include <linux/module.h>
> #include <linux/delay.h>
> -#include <linux/gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/spi/spi.h>
> #include <linux/interrupt.h>
> @@ -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,14 @@ 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_HIGH);
The original code asks for 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");
In the original code, this case also generated the warning "gpio reset is
not defined" (raised from wfx_get_gpio()). I suggest to change the warning
below into "gpio reset is not defined, try 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..d90169fe1851 100644
> --- a/drivers/staging/wfx/main.c
> +++ b/drivers/staging/wfx/main.c
> @@ -13,7 +13,6 @@
> #include <linux/module.h>
> #include <linux/of.h>
> #include <linux/of_net.h>
> -#include <linux/gpio.h>
> #include <linux/gpio/consumer.h>
> #include <linux/mmc/sdio_func.h>
> #include <linux/spi/spi.h>
> @@ -41,10 +40,6 @@ MODULE_DESCRIPTION("Silicon Labs 802.11 Wireless LAN driver for WFx");
> MODULE_AUTHOR("Jérôme Pouiller <jerome.pouiller@xxxxxxxxxx>");
> 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_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_IN);
GPIOD_OUT_LOW?
> + if (IS_ERR(wdev->pdata.gpio_wakeup))
> + return ERR_CAST(wdev->pdata.gpio_wakeup);
This code will fail if the "wakeup" attribute is not defined. I suggest to
just print a notification and continue (the previous code printed a warning,
but the level INFO is sufficient).
> + gpiod_set_consumer_name(wdev->pdata.gpio_wakeup, "wfx wakeup");
> wfx_sl_fill_pdata(dev, &wdev->pdata);
>
> mutex_init(&wdev->conf_mutex);
> diff --git a/drivers/staging/wfx/main.h b/drivers/staging/wfx/main.h
> index f832ce409fda..c59d375dd3ad 100644
> --- a/drivers/staging/wfx/main.h
> +++ b/drivers/staging/wfx/main.h
> @@ -38,8 +38,6 @@ struct wfx_dev *wfx_init_common(struct device *dev,
> int wfx_probe(struct wfx_dev *wdev);
> void wfx_release(struct wfx_dev *wdev);
>
> -struct gpio_desc *wfx_get_gpio(struct device *dev, int override,
> - const char *label);
> bool wfx_api_older_than(struct wfx_dev *wdev, int major, int minor);
> int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len);
>
> --
> 2.25.4
>
>
--
Jérôme Pouiller
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
