* [PATCH] watchdog: wm831x: Use GPIO descriptor
@ 2019-01-05 1:09 Linus Walleij
2019-01-05 1:45 ` Guenter Roeck
0 siblings, 1 reply; 2+ messages in thread
From: Linus Walleij @ 2019-01-05 1:09 UTC (permalink / raw)
To: Wim Van Sebroeck, Guenter Roeck
Cc: linux-watchdog, Linus Walleij, Richard Fitzgerald,
Charles Keepax, Mark Brown
The WM831x watchdog driver passes a global GPIO number from
platform data into this driver, this is discouraged so pass
a GPIO descriptor instead.
More thorough approaches are possible passing descriptors
associated with the device through machine descriptor tables,
but no boardfiles in the kernel currently use this driver
so it is hard to test.
Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
Cc: Mark Brown <broonie@kernel.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/watchdog/wm831x_wdt.c | 23 ++++++-----------------
include/linux/mfd/wm831x/pdata.h | 3 ++-
2 files changed, 8 insertions(+), 18 deletions(-)
diff --git a/drivers/watchdog/wm831x_wdt.c b/drivers/watchdog/wm831x_wdt.c
index 116c2f47b463..998a39f83c48 100644
--- a/drivers/watchdog/wm831x_wdt.c
+++ b/drivers/watchdog/wm831x_wdt.c
@@ -13,7 +13,7 @@
#include <linux/platform_device.h>
#include <linux/watchdog.h>
#include <linux/uaccess.h>
-#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
#include <linux/mfd/wm831x/core.h>
#include <linux/mfd/wm831x/pdata.h>
@@ -29,7 +29,7 @@ struct wm831x_wdt_drvdata {
struct watchdog_device wdt;
struct wm831x *wm831x;
struct mutex lock;
- int update_gpio;
+ struct gpio_desc *update_gpiod;
int update_state;
};
@@ -103,8 +103,8 @@ static int wm831x_wdt_ping(struct watchdog_device *wdt_dev)
mutex_lock(&driver_data->lock);
- if (driver_data->update_gpio) {
- gpio_set_value_cansleep(driver_data->update_gpio,
+ if (driver_data->update_gpiod) {
+ gpiod_set_value_cansleep(driver_data->update_gpiod,
driver_data->update_state);
driver_data->update_state = !driver_data->update_state;
ret = 0;
@@ -239,19 +239,8 @@ static int wm831x_wdt_probe(struct platform_device *pdev)
reg |= pdata->secondary << WM831X_WDOG_SECACT_SHIFT;
reg |= pdata->software << WM831X_WDOG_RST_SRC_SHIFT;
- if (pdata->update_gpio) {
- ret = devm_gpio_request_one(&pdev->dev,
- pdata->update_gpio,
- GPIOF_OUT_INIT_LOW,
- "Watchdog update");
- if (ret < 0) {
- dev_err(wm831x->dev,
- "Failed to request update GPIO: %d\n",
- ret);
- return ret;
- }
-
- driver_data->update_gpio = pdata->update_gpio;
+ if (pdata->update_gpiod) {
+ driver_data->update_gpiod = pdata->update_gpiod;
/* Make sure the watchdog takes hardware updates */
reg |= WM831X_WDOG_RST_SRC;
diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
index dcc9631b3052..ea7e9ad10a14 100644
--- a/include/linux/mfd/wm831x/pdata.h
+++ b/include/linux/mfd/wm831x/pdata.h
@@ -15,6 +15,7 @@
#ifndef __MFD_WM831X_PDATA_H__
#define __MFD_WM831X_PDATA_H__
+struct gpio_desc;
struct wm831x;
struct regulator_init_data;
@@ -95,7 +96,7 @@ enum wm831x_watchdog_action {
struct wm831x_watchdog_pdata {
enum wm831x_watchdog_action primary, secondary;
- int update_gpio;
+ struct gpio_desc *update_gpiod;
unsigned int software:1;
};
--
2.19.2
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] watchdog: wm831x: Use GPIO descriptor
2019-01-05 1:09 [PATCH] watchdog: wm831x: Use GPIO descriptor Linus Walleij
@ 2019-01-05 1:45 ` Guenter Roeck
0 siblings, 0 replies; 2+ messages in thread
From: Guenter Roeck @ 2019-01-05 1:45 UTC (permalink / raw)
To: Linus Walleij, Wim Van Sebroeck
Cc: linux-watchdog, Richard Fitzgerald, Charles Keepax, Mark Brown
On 1/4/19 5:09 PM, Linus Walleij wrote:
> The WM831x watchdog driver passes a global GPIO number from
> platform data into this driver, this is discouraged so pass
> a GPIO descriptor instead.
>
> More thorough approaches are possible passing descriptors
> associated with the device through machine descriptor tables,
> but no boardfiles in the kernel currently use this driver
> so it is hard to test.
>
There is a semantic change: Up to now, the gpio descriptor/pin
is released when the driver is removed. This is no longer the
case after this patch is applied. What happens with the provided
gpiod on driver removal ?
Anyway, it seems to me that it might make more sense to remove
the platform data and all its uses entirely. It isn't provided
and thus not used, and who knows if it even works. Providing
it would require changes in the mfd driver, which are unlikely
to happen at this point since the driver is close to 10 years old.
Mark, since you wrote that code, do you have any idea what
the use case might be (or have been) ?
Thanks,
Guenter
> Cc: Richard Fitzgerald <rf@opensource.cirrus.com>
> Cc: Charles Keepax <ckeepax@opensource.cirrus.com>
> Cc: Mark Brown <broonie@kernel.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> drivers/watchdog/wm831x_wdt.c | 23 ++++++-----------------
> include/linux/mfd/wm831x/pdata.h | 3 ++-
> 2 files changed, 8 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/watchdog/wm831x_wdt.c b/drivers/watchdog/wm831x_wdt.c
> index 116c2f47b463..998a39f83c48 100644
> --- a/drivers/watchdog/wm831x_wdt.c
> +++ b/drivers/watchdog/wm831x_wdt.c
> @@ -13,7 +13,7 @@
> #include <linux/platform_device.h>
> #include <linux/watchdog.h>
> #include <linux/uaccess.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/consumer.h>
>
> #include <linux/mfd/wm831x/core.h>
> #include <linux/mfd/wm831x/pdata.h>
> @@ -29,7 +29,7 @@ struct wm831x_wdt_drvdata {
> struct watchdog_device wdt;
> struct wm831x *wm831x;
> struct mutex lock;
> - int update_gpio;
> + struct gpio_desc *update_gpiod;
> int update_state;
> };
>
> @@ -103,8 +103,8 @@ static int wm831x_wdt_ping(struct watchdog_device *wdt_dev)
>
> mutex_lock(&driver_data->lock);
>
> - if (driver_data->update_gpio) {
> - gpio_set_value_cansleep(driver_data->update_gpio,
> + if (driver_data->update_gpiod) {
> + gpiod_set_value_cansleep(driver_data->update_gpiod,
> driver_data->update_state);
> driver_data->update_state = !driver_data->update_state;
> ret = 0;
> @@ -239,19 +239,8 @@ static int wm831x_wdt_probe(struct platform_device *pdev)
> reg |= pdata->secondary << WM831X_WDOG_SECACT_SHIFT;
> reg |= pdata->software << WM831X_WDOG_RST_SRC_SHIFT;
>
> - if (pdata->update_gpio) {
> - ret = devm_gpio_request_one(&pdev->dev,
> - pdata->update_gpio,
> - GPIOF_OUT_INIT_LOW,
> - "Watchdog update");
> - if (ret < 0) {
> - dev_err(wm831x->dev,
> - "Failed to request update GPIO: %d\n",
> - ret);
> - return ret;
> - }
> -
> - driver_data->update_gpio = pdata->update_gpio;
> + if (pdata->update_gpiod) {
> + driver_data->update_gpiod = pdata->update_gpiod;
>
> /* Make sure the watchdog takes hardware updates */
> reg |= WM831X_WDOG_RST_SRC;
> diff --git a/include/linux/mfd/wm831x/pdata.h b/include/linux/mfd/wm831x/pdata.h
> index dcc9631b3052..ea7e9ad10a14 100644
> --- a/include/linux/mfd/wm831x/pdata.h
> +++ b/include/linux/mfd/wm831x/pdata.h
> @@ -15,6 +15,7 @@
> #ifndef __MFD_WM831X_PDATA_H__
> #define __MFD_WM831X_PDATA_H__
>
> +struct gpio_desc;
> struct wm831x;
> struct regulator_init_data;
>
> @@ -95,7 +96,7 @@ enum wm831x_watchdog_action {
>
> struct wm831x_watchdog_pdata {
> enum wm831x_watchdog_action primary, secondary;
> - int update_gpio;
> + struct gpio_desc *update_gpiod;
> unsigned int software:1;
> };
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-01-05 1:45 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-05 1:09 [PATCH] watchdog: wm831x: Use GPIO descriptor Linus Walleij
2019-01-05 1:45 ` Guenter Roeck
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).