* [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple()
@ 2021-05-07 21:24 W_Armin
2021-05-07 21:24 ` [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog W_Armin
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: W_Armin @ 2021-05-07 21:24 UTC (permalink / raw)
To: hdegoede; +Cc: linux-hwmon, linux, jdelvare
From: Armin Wolf <W_Armin@gmx.de>
Since it was requested to use devres for the watchdog device,
use devm_watchdog_register() for watchdog registration and do
some small cleanups.
Also use platform_device_register_simple() in sch56xx_device_add().
Tested on a Fujitsu Esprimo P720.
Armin Wolf (4):
hwmon: (sch56xx) Use devres functions for watchdog
hwmon: (sch56xx-common) Use strscpy
hwmon: (sch56xx-common) Use helper function
hwmon: (sch56xx-common) Simplify sch56xx_device_add
drivers/hwmon/sch5627.c | 12 ++------
drivers/hwmon/sch5636.c | 9 ++----
drivers/hwmon/sch56xx-common.c | 50 ++++++++--------------------------
3 files changed, 16 insertions(+), 55 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog
2021-05-07 21:24 [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() W_Armin
@ 2021-05-07 21:24 ` W_Armin
2021-05-07 21:43 ` Guenter Roeck
2021-05-07 21:24 ` [PATCH 2/4] hwmon: (sch56xx-common) Use strscpy W_Armin
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: W_Armin @ 2021-05-07 21:24 UTC (permalink / raw)
To: hdegoede; +Cc: linux-hwmon, linux, jdelvare
From: Armin Wolf <W_Armin@gmx.de>
Use devm_kzalloc()/devm_watchdog_register() for
watchdog registration since it allows us to remove
the sch56xx_watchdog_data struct from the drivers
own data structs.
Remove sch56xx_watchdog_unregister since devres
takes care of that now.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/sch5627.c | 12 +++---------
drivers/hwmon/sch5636.c | 9 ++-------
drivers/hwmon/sch56xx-common.c | 13 +++----------
3 files changed, 8 insertions(+), 26 deletions(-)
diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c
index 4324a5dbc968..8a71fba33ea0 100644
--- a/drivers/hwmon/sch5627.c
+++ b/drivers/hwmon/sch5627.c
@@ -64,7 +64,6 @@ static const char * const SCH5627_IN_LABELS[SCH5627_NO_IN] = {
struct sch5627_data {
unsigned short addr;
- struct sch56xx_watchdog_data *watchdog;
u8 control;
u8 temp_max[SCH5627_NO_TEMPS];
u8 temp_crit[SCH5627_NO_TEMPS];
@@ -359,11 +358,6 @@ static const struct hwmon_chip_info sch5627_chip_info = {
static int sch5627_remove(struct platform_device *pdev)
{
- struct sch5627_data *data = platform_get_drvdata(pdev);
-
- if (data->watchdog)
- sch56xx_watchdog_unregister(data->watchdog);
-
return 0;
}
@@ -460,9 +454,9 @@ static int sch5627_probe(struct platform_device *pdev)
return PTR_ERR(hwmon_dev);
/* Note failing to register the watchdog is not a fatal error */
- data->watchdog = sch56xx_watchdog_register(&pdev->dev, data->addr,
- (build_code << 24) | (build_id << 8) | hwmon_rev,
- &data->update_lock, 1);
+ sch56xx_watchdog_register(&pdev->dev, data->addr,
+ (build_code << 24) | (build_id << 8) | hwmon_rev,
+ &data->update_lock, 1);
return 0;
}
diff --git a/drivers/hwmon/sch5636.c b/drivers/hwmon/sch5636.c
index 5683a38740f6..a5cd4de36575 100644
--- a/drivers/hwmon/sch5636.c
+++ b/drivers/hwmon/sch5636.c
@@ -54,7 +54,6 @@ static const u16 SCH5636_REG_FAN_VAL[SCH5636_NO_FANS] = {
struct sch5636_data {
unsigned short addr;
struct device *hwmon_dev;
- struct sch56xx_watchdog_data *watchdog;
struct mutex update_lock;
char valid; /* !=0 if following fields are valid */
@@ -372,9 +371,6 @@ static int sch5636_remove(struct platform_device *pdev)
struct sch5636_data *data = platform_get_drvdata(pdev);
int i;
- if (data->watchdog)
- sch56xx_watchdog_unregister(data->watchdog);
-
if (data->hwmon_dev)
hwmon_device_unregister(data->hwmon_dev);
@@ -495,9 +491,8 @@ static int sch5636_probe(struct platform_device *pdev)
}
/* Note failing to register the watchdog is not a fatal error */
- data->watchdog = sch56xx_watchdog_register(&pdev->dev, data->addr,
- (revision[0] << 8) | revision[1],
- &data->update_lock, 0);
+ sch56xx_watchdog_register(&pdev->dev, data->addr, (revision[0] << 8) | revision[1],
+ &data->update_lock, 0);
return 0;
diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c
index 6c84780e358e..9c884fd0beb2 100644
--- a/drivers/hwmon/sch56xx-common.c
+++ b/drivers/hwmon/sch56xx-common.c
@@ -401,7 +401,7 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
return NULL;
}
- data = kzalloc(sizeof(struct sch56xx_watchdog_data), GFP_KERNEL);
+ data = devm_kzalloc(parent, sizeof(struct sch56xx_watchdog_data), GFP_KERNEL);
if (!data)
return NULL;
@@ -438,10 +438,10 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
data->watchdog_output_enable = output_enable;
watchdog_set_drvdata(&data->wddev, data);
- err = watchdog_register_device(&data->wddev);
+ err = devm_watchdog_register_device(parent, &data->wddev);
if (err) {
pr_err("Registering watchdog chardev: %d\n", err);
- kfree(data);
+ devm_kfree(parent, data);
return NULL;
}
@@ -449,13 +449,6 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
}
EXPORT_SYMBOL(sch56xx_watchdog_register);
-void sch56xx_watchdog_unregister(struct sch56xx_watchdog_data *data)
-{
- watchdog_unregister_device(&data->wddev);
- kfree(data);
-}
-EXPORT_SYMBOL(sch56xx_watchdog_unregister);
-
/*
* platform dev find, add and remove functions
*/
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/4] hwmon: (sch56xx-common) Use strscpy
2021-05-07 21:24 [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() W_Armin
2021-05-07 21:24 ` [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog W_Armin
@ 2021-05-07 21:24 ` W_Armin
2021-05-07 21:24 ` [PATCH 3/4] hwmon: (sch56xx-common) Use helper function W_Armin
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: W_Armin @ 2021-05-07 21:24 UTC (permalink / raw)
To: hdegoede; +Cc: linux-hwmon, linux, jdelvare
From: Armin Wolf <W_Armin@gmx.de>
strlcpy is considered deprecated.
Replace it with strscpy.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/sch56xx-common.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c
index 9c884fd0beb2..2d7bf017b2df 100644
--- a/drivers/hwmon/sch56xx-common.c
+++ b/drivers/hwmon/sch56xx-common.c
@@ -408,8 +408,7 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
data->addr = addr;
data->io_lock = io_lock;
- strlcpy(data->wdinfo.identity, "sch56xx watchdog",
- sizeof(data->wdinfo.identity));
+ strscpy(data->wdinfo.identity, "sch56xx watchdog", sizeof(data->wdinfo.identity));
data->wdinfo.firmware_version = revision;
data->wdinfo.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT;
if (!nowayout)
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/4] hwmon: (sch56xx-common) Use helper function
2021-05-07 21:24 [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() W_Armin
2021-05-07 21:24 ` [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog W_Armin
2021-05-07 21:24 ` [PATCH 2/4] hwmon: (sch56xx-common) Use strscpy W_Armin
@ 2021-05-07 21:24 ` W_Armin
2021-05-07 21:24 ` [PATCH 4/4] hwmon: (sch56xx-common) Simplify sch56xx_device_add W_Armin
2021-05-08 8:59 ` [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() Hans de Goede
4 siblings, 0 replies; 7+ messages in thread
From: W_Armin @ 2021-05-07 21:24 UTC (permalink / raw)
To: hdegoede; +Cc: linux-hwmon, linux, jdelvare
From: Armin Wolf <W_Armin@gmx.de>
Use watchdog_set_nowayout() to process param
setting and change param type to bool.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/sch56xx-common.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c
index 2d7bf017b2df..2273943960bf 100644
--- a/drivers/hwmon/sch56xx-common.c
+++ b/drivers/hwmon/sch56xx-common.c
@@ -20,8 +20,8 @@
#include "sch56xx-common.h"
/* Insmod parameters */
-static int nowayout = WATCHDOG_NOWAYOUT;
-module_param(nowayout, int, 0);
+static bool nowayout = WATCHDOG_NOWAYOUT;
+module_param(nowayout, bool, 0);
MODULE_PARM_DESC(nowayout, "Watchdog cannot be stopped once started (default="
__MODULE_STRING(WATCHDOG_NOWAYOUT) ")");
@@ -420,8 +420,7 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
data->wddev.timeout = 60;
data->wddev.min_timeout = 1;
data->wddev.max_timeout = 255 * 60;
- if (nowayout)
- set_bit(WDOG_NO_WAY_OUT, &data->wddev.status);
+ watchdog_set_nowayout(&data->wddev, nowayout);
if (output_enable & SCH56XX_WDOG_OUTPUT_ENABLE)
set_bit(WDOG_ACTIVE, &data->wddev.status);
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] hwmon: (sch56xx-common) Simplify sch56xx_device_add
2021-05-07 21:24 [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() W_Armin
` (2 preceding siblings ...)
2021-05-07 21:24 ` [PATCH 3/4] hwmon: (sch56xx-common) Use helper function W_Armin
@ 2021-05-07 21:24 ` W_Armin
2021-05-08 8:59 ` [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() Hans de Goede
4 siblings, 0 replies; 7+ messages in thread
From: W_Armin @ 2021-05-07 21:24 UTC (permalink / raw)
To: hdegoede; +Cc: linux-hwmon, linux, jdelvare
From: Armin Wolf <W_Armin@gmx.de>
Use platform_device_register_simple() instead of
manually calling platform_device_alloc()/platform_device_add().
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/sch56xx-common.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)
diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c
index 2273943960bf..b731c108ec56 100644
--- a/drivers/hwmon/sch56xx-common.c
+++ b/drivers/hwmon/sch56xx-common.c
@@ -507,37 +507,18 @@ static int __init sch56xx_device_add(int address, const char *name)
struct resource res = {
.start = address,
.end = address + REGION_LENGTH - 1,
+ .name = name,
.flags = IORESOURCE_IO,
};
int err;
- sch56xx_pdev = platform_device_alloc(name, address);
- if (!sch56xx_pdev)
- return -ENOMEM;
-
- res.name = sch56xx_pdev->name;
err = acpi_check_resource_conflict(&res);
if (err)
- goto exit_device_put;
-
- err = platform_device_add_resources(sch56xx_pdev, &res, 1);
- if (err) {
- pr_err("Device resource addition failed\n");
- goto exit_device_put;
- }
-
- err = platform_device_add(sch56xx_pdev);
- if (err) {
- pr_err("Device addition failed\n");
- goto exit_device_put;
- }
-
- return 0;
+ return err;
-exit_device_put:
- platform_device_put(sch56xx_pdev);
+ sch56xx_pdev = platform_device_register_simple(name, -1, &res, 1);
- return err;
+ return PTR_ERR_OR_ZERO(sch56xx_pdev);
}
static int __init sch56xx_init(void)
--
2.20.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog
2021-05-07 21:24 ` [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog W_Armin
@ 2021-05-07 21:43 ` Guenter Roeck
0 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2021-05-07 21:43 UTC (permalink / raw)
To: W_Armin, hdegoede; +Cc: linux-hwmon, jdelvare
On 5/7/21 2:24 PM, W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
>
> Use devm_kzalloc()/devm_watchdog_register() for
> watchdog registration since it allows us to remove
> the sch56xx_watchdog_data struct from the drivers
> own data structs.
> Remove sch56xx_watchdog_unregister since devres
> takes care of that now.
>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
> drivers/hwmon/sch5627.c | 12 +++---------
> drivers/hwmon/sch5636.c | 9 ++-------
> drivers/hwmon/sch56xx-common.c | 13 +++----------
> 3 files changed, 8 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c
> index 4324a5dbc968..8a71fba33ea0 100644
> --- a/drivers/hwmon/sch5627.c
> +++ b/drivers/hwmon/sch5627.c
> @@ -64,7 +64,6 @@ static const char * const SCH5627_IN_LABELS[SCH5627_NO_IN] = {
>
> struct sch5627_data {
> unsigned short addr;
> - struct sch56xx_watchdog_data *watchdog;
> u8 control;
> u8 temp_max[SCH5627_NO_TEMPS];
> u8 temp_crit[SCH5627_NO_TEMPS];
> @@ -359,11 +358,6 @@ static const struct hwmon_chip_info sch5627_chip_info = {
>
> static int sch5627_remove(struct platform_device *pdev)
> {
> - struct sch5627_data *data = platform_get_drvdata(pdev);
> -
> - if (data->watchdog)
> - sch56xx_watchdog_unregister(data->watchdog);
> -
> return 0;
> }
This function is no longer necessary and can be dropped as well.
>
> @@ -460,9 +454,9 @@ static int sch5627_probe(struct platform_device *pdev)
> return PTR_ERR(hwmon_dev);
>
> /* Note failing to register the watchdog is not a fatal error */
> - data->watchdog = sch56xx_watchdog_register(&pdev->dev, data->addr,
> - (build_code << 24) | (build_id << 8) | hwmon_rev,
> - &data->update_lock, 1);
> + sch56xx_watchdog_register(&pdev->dev, data->addr,
> + (build_code << 24) | (build_id << 8) | hwmon_rev,
> + &data->update_lock, 1);
>
> return 0;
> }
> diff --git a/drivers/hwmon/sch5636.c b/drivers/hwmon/sch5636.c
> index 5683a38740f6..a5cd4de36575 100644
> --- a/drivers/hwmon/sch5636.c
> +++ b/drivers/hwmon/sch5636.c
> @@ -54,7 +54,6 @@ static const u16 SCH5636_REG_FAN_VAL[SCH5636_NO_FANS] = {
> struct sch5636_data {
> unsigned short addr;
> struct device *hwmon_dev;
> - struct sch56xx_watchdog_data *watchdog;
>
> struct mutex update_lock;
> char valid; /* !=0 if following fields are valid */
> @@ -372,9 +371,6 @@ static int sch5636_remove(struct platform_device *pdev)
> struct sch5636_data *data = platform_get_drvdata(pdev);
> int i;
>
> - if (data->watchdog)
> - sch56xx_watchdog_unregister(data->watchdog);
> -
> if (data->hwmon_dev)
> hwmon_device_unregister(data->hwmon_dev);
>
> @@ -495,9 +491,8 @@ static int sch5636_probe(struct platform_device *pdev)
> }
>
> /* Note failing to register the watchdog is not a fatal error */
> - data->watchdog = sch56xx_watchdog_register(&pdev->dev, data->addr,
> - (revision[0] << 8) | revision[1],
> - &data->update_lock, 0);
> + sch56xx_watchdog_register(&pdev->dev, data->addr, (revision[0] << 8) | revision[1],
> + &data->update_lock, 0);
>
> return 0;
>
> diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c
> index 6c84780e358e..9c884fd0beb2 100644
> --- a/drivers/hwmon/sch56xx-common.c
> +++ b/drivers/hwmon/sch56xx-common.c
> @@ -401,7 +401,7 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
> return NULL;
> }
>
> - data = kzalloc(sizeof(struct sch56xx_watchdog_data), GFP_KERNEL);
> + data = devm_kzalloc(parent, sizeof(struct sch56xx_watchdog_data), GFP_KERNEL);
> if (!data)
> return NULL;
>
> @@ -438,10 +438,10 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
Since the return value is no longer used, the function does not need
to return it anymore.
> data->watchdog_output_enable = output_enable;
>
> watchdog_set_drvdata(&data->wddev, data);
> - err = watchdog_register_device(&data->wddev);
> + err = devm_watchdog_register_device(parent, &data->wddev);
> if (err) {
> pr_err("Registering watchdog chardev: %d\n", err);
> - kfree(data);
> + devm_kfree(parent, data);
> return NULL;
> }
>
> @@ -449,13 +449,6 @@ struct sch56xx_watchdog_data *sch56xx_watchdog_register(struct device *parent,
> }
> EXPORT_SYMBOL(sch56xx_watchdog_register);
>
> -void sch56xx_watchdog_unregister(struct sch56xx_watchdog_data *data)
> -{
> - watchdog_unregister_device(&data->wddev);
> - kfree(data);
> -}
> -EXPORT_SYMBOL(sch56xx_watchdog_unregister);
> -
> /*
> * platform dev find, add and remove functions
> */
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple()
2021-05-07 21:24 [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() W_Armin
` (3 preceding siblings ...)
2021-05-07 21:24 ` [PATCH 4/4] hwmon: (sch56xx-common) Simplify sch56xx_device_add W_Armin
@ 2021-05-08 8:59 ` Hans de Goede
4 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2021-05-08 8:59 UTC (permalink / raw)
To: W_Armin; +Cc: linux-hwmon, linux, jdelvare
Hi,
On 5/7/21 11:24 PM, W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
>
> Since it was requested to use devres for the watchdog device,
> use devm_watchdog_register() for watchdog registration and do
> some small cleanups.
>
> Also use platform_device_register_simple() in sch56xx_device_add().
>
> Tested on a Fujitsu Esprimo P720.
>
> Armin Wolf (4):
> hwmon: (sch56xx) Use devres functions for watchdog
> hwmon: (sch56xx-common) Use strscpy
> hwmon: (sch56xx-common) Use helper function
> hwmon: (sch56xx-common) Simplify sch56xx_device_add
Thanks, the entire series looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
For the series.
Regards,
Hans
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-08 9:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 21:24 [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() W_Armin
2021-05-07 21:24 ` [PATCH 1/4] hwmon: (sch56xx) Use devres functions for watchdog W_Armin
2021-05-07 21:43 ` Guenter Roeck
2021-05-07 21:24 ` [PATCH 2/4] hwmon: (sch56xx-common) Use strscpy W_Armin
2021-05-07 21:24 ` [PATCH 3/4] hwmon: (sch56xx-common) Use helper function W_Armin
2021-05-07 21:24 ` [PATCH 4/4] hwmon: (sch56xx-common) Simplify sch56xx_device_add W_Armin
2021-05-08 8:59 ` [PATCH 0/4] hwmon: (sch56xx) Use devres for watchdog and platform_device_register_simple() Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.