* [PATCH 1/2] hwmon: (sch5627) Use devres function
2021-04-17 21:09 [PATCH 0/2] hwmon: (sch5627) Use devres function W_Armin
@ 2021-04-17 21:09 ` W_Armin
2021-04-17 21:09 ` [PATCH 2/2] hwmon: (sch5627) Remove unnecessary error path W_Armin
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: W_Armin @ 2021-04-17 21:09 UTC (permalink / raw)
To: hdegoede; +Cc: jdelvare, linux, linux-hwmon
From: Armin Wolf <W_Armin@gmx.de>
Use devm_hwmon_device_register_with_info() and remove hwmon_dev
from sch5627_data struct as it is not needed anymore.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/sch5627.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c
index 8be339ae5f7d..ea042a6dae58 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 device *hwmon_dev;
struct sch56xx_watchdog_data *watchdog;
u8 control;
u8 temp_max[SCH5627_NO_TEMPS];
@@ -365,15 +364,13 @@ static int sch5627_remove(struct platform_device *pdev)
if (data->watchdog)
sch56xx_watchdog_unregister(data->watchdog);
- if (data->hwmon_dev)
- hwmon_device_unregister(data->hwmon_dev);
-
return 0;
}
static int sch5627_probe(struct platform_device *pdev)
{
struct sch5627_data *data;
+ struct device *hwmon_dev;
int err, build_code, build_id, hwmon_rev, val;
data = devm_kzalloc(&pdev->dev, sizeof(struct sch5627_data),
@@ -471,12 +468,10 @@ static int sch5627_probe(struct platform_device *pdev)
pr_info("firmware build: code 0x%02X, id 0x%04X, hwmon: rev 0x%02X\n",
build_code, build_id, hwmon_rev);
- data->hwmon_dev = hwmon_device_register_with_info(&pdev->dev, DEVNAME, data,
- &sch5627_chip_info,
- NULL);
- if (IS_ERR(data->hwmon_dev)) {
- err = PTR_ERR(data->hwmon_dev);
- data->hwmon_dev = NULL;
+ hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, DEVNAME, data,
+ &sch5627_chip_info, NULL);
+ if (IS_ERR(hwmon_dev)) {
+ err = PTR_ERR(hwmon_dev);
goto error;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH 2/2] hwmon: (sch5627) Remove unnecessary error path
2021-04-17 21:09 [PATCH 0/2] hwmon: (sch5627) Use devres function W_Armin
2021-04-17 21:09 ` [PATCH 1/2] " W_Armin
@ 2021-04-17 21:09 ` W_Armin
2021-04-18 12:48 ` [PATCH 0/2] hwmon: (sch5627) Use devres function Hans de Goede
2021-04-19 3:59 ` Guenter Roeck
3 siblings, 0 replies; 5+ messages in thread
From: W_Armin @ 2021-04-17 21:09 UTC (permalink / raw)
To: hdegoede; +Cc: jdelvare, linux, linux-hwmon
From: Armin Wolf <W_Armin@gmx.de>
Calling remove() on error whould have only unregistered
the watchdog, and since a failure in registering him
is considered non-fatal and happens last, remove the
error path and return the error codes directly.
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
drivers/hwmon/sch5627.c | 70 +++++++++++++++--------------------------
1 file changed, 25 insertions(+), 45 deletions(-)
diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c
index ea042a6dae58..4324a5dbc968 100644
--- a/drivers/hwmon/sch5627.c
+++ b/drivers/hwmon/sch5627.c
@@ -383,72 +383,58 @@ static int sch5627_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, data);
val = sch56xx_read_virtual_reg(data->addr, SCH5627_REG_HWMON_ID);
- if (val < 0) {
- err = val;
- goto error;
- }
+ if (val < 0)
+ return val;
+
if (val != SCH5627_HWMON_ID) {
pr_err("invalid %s id: 0x%02X (expected 0x%02X)\n", "hwmon",
val, SCH5627_HWMON_ID);
- err = -ENODEV;
- goto error;
+ return -ENODEV;
}
val = sch56xx_read_virtual_reg(data->addr, SCH5627_REG_COMPANY_ID);
- if (val < 0) {
- err = val;
- goto error;
- }
+ if (val < 0)
+ return val;
+
if (val != SCH5627_COMPANY_ID) {
pr_err("invalid %s id: 0x%02X (expected 0x%02X)\n", "company",
val, SCH5627_COMPANY_ID);
- err = -ENODEV;
- goto error;
+ return -ENODEV;
}
val = sch56xx_read_virtual_reg(data->addr, SCH5627_REG_PRIMARY_ID);
- if (val < 0) {
- err = val;
- goto error;
- }
+ if (val < 0)
+ return val;
+
if (val != SCH5627_PRIMARY_ID) {
pr_err("invalid %s id: 0x%02X (expected 0x%02X)\n", "primary",
val, SCH5627_PRIMARY_ID);
- err = -ENODEV;
- goto error;
+ return -ENODEV;
}
build_code = sch56xx_read_virtual_reg(data->addr,
SCH5627_REG_BUILD_CODE);
- if (build_code < 0) {
- err = build_code;
- goto error;
- }
+ if (build_code < 0)
+ return build_code;
build_id = sch56xx_read_virtual_reg16(data->addr,
SCH5627_REG_BUILD_ID);
- if (build_id < 0) {
- err = build_id;
- goto error;
- }
+ if (build_id < 0)
+ return build_id;
hwmon_rev = sch56xx_read_virtual_reg(data->addr,
SCH5627_REG_HWMON_REV);
- if (hwmon_rev < 0) {
- err = hwmon_rev;
- goto error;
- }
+ if (hwmon_rev < 0)
+ return hwmon_rev;
val = sch56xx_read_virtual_reg(data->addr, SCH5627_REG_CTRL);
- if (val < 0) {
- err = val;
- goto error;
- }
+ if (val < 0)
+ return val;
+
data->control = val;
if (!(data->control & 0x01)) {
pr_err("hardware monitoring not enabled\n");
- err = -ENODEV;
- goto error;
+ return -ENODEV;
}
/* Trigger a Vbat voltage measurement, so that we get a valid reading
the first time we read Vbat */
@@ -462,7 +448,7 @@ static int sch5627_probe(struct platform_device *pdev)
*/
err = sch5627_read_limits(data);
if (err)
- goto error;
+ return err;
pr_info("found %s chip at %#hx\n", DEVNAME, data->addr);
pr_info("firmware build: code 0x%02X, id 0x%04X, hwmon: rev 0x%02X\n",
@@ -470,10 +456,8 @@ static int sch5627_probe(struct platform_device *pdev)
hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, DEVNAME, data,
&sch5627_chip_info, NULL);
- if (IS_ERR(hwmon_dev)) {
- err = PTR_ERR(hwmon_dev);
- goto error;
- }
+ if (IS_ERR(hwmon_dev))
+ 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,
@@ -481,10 +465,6 @@ static int sch5627_probe(struct platform_device *pdev)
&data->update_lock, 1);
return 0;
-
-error:
- sch5627_remove(pdev);
- return err;
}
static struct platform_driver sch5627_driver = {
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] hwmon: (sch5627) Use devres function
2021-04-17 21:09 [PATCH 0/2] hwmon: (sch5627) Use devres function W_Armin
2021-04-17 21:09 ` [PATCH 1/2] " W_Armin
2021-04-17 21:09 ` [PATCH 2/2] hwmon: (sch5627) Remove unnecessary error path W_Armin
@ 2021-04-18 12:48 ` Hans de Goede
2021-04-19 3:59 ` Guenter Roeck
3 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-04-18 12:48 UTC (permalink / raw)
To: W_Armin; +Cc: jdelvare, linux, linux-hwmon
Hi,
On 4/17/21 11:09 PM, W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
>
> Use devm_hwmon_device_register_with_info() to simplify
> the source code and remove an unnecessary error path
> in sch5627_probe as it performs nothing useful when
> using the devres function.
>
> Tested on a Fujitsu Esprimo P720.
>
> Armin Wolf (2):
> hwmon: (sch5627) Use devres function
> hwmon: (sch5627) Remove unnecessary error path
Thanks, both patches looks good to me:
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
For the series.
Regards,
Hans
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] hwmon: (sch5627) Use devres function
2021-04-17 21:09 [PATCH 0/2] hwmon: (sch5627) Use devres function W_Armin
` (2 preceding siblings ...)
2021-04-18 12:48 ` [PATCH 0/2] hwmon: (sch5627) Use devres function Hans de Goede
@ 2021-04-19 3:59 ` Guenter Roeck
3 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2021-04-19 3:59 UTC (permalink / raw)
To: W_Armin, hdegoede; +Cc: jdelvare, linux-hwmon
On 4/17/21 2:09 PM, W_Armin@gmx.de wrote:
> From: Armin Wolf <W_Armin@gmx.de>
>
> Use devm_hwmon_device_register_with_info() to simplify
> the source code and remove an unnecessary error path
> in sch5627_probe as it performs nothing useful when
> using the devres function.
>
> Tested on a Fujitsu Esprimo P720.
>
> Armin Wolf (2):
> hwmon: (sch5627) Use devres function
> hwmon: (sch5627) Remove unnecessary error path
>
> drivers/hwmon/sch5627.c | 81 ++++++++++++++---------------------------
> 1 file changed, 28 insertions(+), 53 deletions(-)
>
> --
> 2.20.1
>
Both applied, though it would be even better if you'd do the same
with the watchdog device.
Guenter
^ permalink raw reply [flat|nested] 5+ messages in thread