2023-12-14 08:31:19

by George Stark

[permalink] [raw]
Subject: [PATCH v3 05/11] leds: aw200xx: use devm API to cleanup module's resources

In this driver LEDs are registered using devm_led_classdev_register()
so they are automatically unregistered after module's remove() is done.
led_classdev_unregister() calls module's led_set_brightness() to turn off
the LEDs and that callback uses resources which were destroyed already
in module's remove() so use devm API instead of remove().

Signed-off-by: George Stark <[email protected]>
---
drivers/leds/leds-aw200xx.c | 33 ++++++++++++++++++++++-----------
1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-aw200xx.c b/drivers/leds/leds-aw200xx.c
index 1d3943f86f7f..eed1e65c1c0e 100644
--- a/drivers/leds/leds-aw200xx.c
+++ b/drivers/leds/leds-aw200xx.c
@@ -10,6 +10,7 @@
#include <linux/bitfield.h>
#include <linux/bits.h>
#include <linux/container_of.h>
+#include <linux/devm-helpers.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
#include <linux/leds.h>
@@ -530,6 +531,16 @@ static const struct regmap_config aw200xx_regmap_config = {
.disable_locking = true,
};

+static void aw200xx_chip_reset_action(void *data)
+{
+ aw200xx_chip_reset(data);
+}
+
+static void aw200xx_disable_action(void *data)
+{
+ aw200xx_disable(data);
+}
+
static int aw200xx_probe(struct i2c_client *client)
{
const struct aw200xx_chipdef *cdef;
@@ -568,11 +579,17 @@ static int aw200xx_probe(struct i2c_client *client)

aw200xx_enable(chip);

+ ret = devm_add_action(&client->dev, aw200xx_disable_action, chip);
+ if (ret)
+ return ret;
+
ret = aw200xx_chip_check(chip);
if (ret)
return ret;

- mutex_init(&chip->mutex);
+ ret = devm_mutex_init(&client->dev, &chip->mutex);
+ if (ret)
+ return ret;

/* Need a lock now since after call aw200xx_probe_fw, sysfs nodes created */
mutex_lock(&chip->mutex);
@@ -581,6 +598,10 @@ static int aw200xx_probe(struct i2c_client *client)
if (ret)
goto out_unlock;

+ ret = devm_add_action(&client->dev, aw200xx_chip_reset_action, chip);
+ if (ret)
+ goto out_unlock;
+
ret = aw200xx_probe_fw(&client->dev, chip);
if (ret)
goto out_unlock;
@@ -595,15 +616,6 @@ static int aw200xx_probe(struct i2c_client *client)
return ret;
}

-static void aw200xx_remove(struct i2c_client *client)
-{
- struct aw200xx *chip = i2c_get_clientdata(client);
-
- aw200xx_chip_reset(chip);
- aw200xx_disable(chip);
- mutex_destroy(&chip->mutex);
-}
-
static const struct aw200xx_chipdef aw20036_cdef = {
.channels = 36,
.display_size_rows_max = 3,
@@ -652,7 +664,6 @@ static struct i2c_driver aw200xx_driver = {
.of_match_table = aw200xx_match_table,
},
.probe_new = aw200xx_probe,
- .remove = aw200xx_remove,
.id_table = aw200xx_id,
};
module_i2c_driver(aw200xx_driver);
--
2.25.1


2023-12-14 08:31:22

by George Stark

[permalink] [raw]
Subject: [PATCH v3 08/11] leds: nic78bx: use devm API to cleanup module's resources

In this driver LEDs are registered using devm_led_classdev_register()
so they are automatically unregistered after module's remove() is done.
led_classdev_unregister() calls module's led_set_brightness() to turn off
the LEDs and that callback uses resources which were destroyed already
in module's remove() so use devm API instead of remove().

Signed-off-by: George Stark <[email protected]>
---
drivers/leds/leds-nic78bx.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/leds/leds-nic78bx.c b/drivers/leds/leds-nic78bx.c
index f196f52eec1e..f3049fa14f04 100644
--- a/drivers/leds/leds-nic78bx.c
+++ b/drivers/leds/leds-nic78bx.c
@@ -118,6 +118,15 @@ static struct nic78bx_led nic78bx_leds[] = {
}
};

+static void lock_led_reg_action(void *data)
+{
+ struct nic78bx_led_data *led_data = (struct nic78bx_led_data *)data;
+
+ /* Lock LED register */
+ outb(NIC78BX_LOCK_VALUE,
+ led_data->io_base + NIC78BX_LOCK_REG_OFFSET);
+}
+
static int nic78bx_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -152,6 +161,10 @@ static int nic78bx_probe(struct platform_device *pdev)
led_data->io_base = io_rc->start;
spin_lock_init(&led_data->lock);

+ ret = devm_add_action(dev, lock_led_reg_action, led_data);
+ if (ret)
+ return ret;
+
for (i = 0; i < ARRAY_SIZE(nic78bx_leds); i++) {
nic78bx_leds[i].data = led_data;

@@ -167,17 +180,6 @@ static int nic78bx_probe(struct platform_device *pdev)
return ret;
}

-static int nic78bx_remove(struct platform_device *pdev)
-{
- struct nic78bx_led_data *led_data = platform_get_drvdata(pdev);
-
- /* Lock LED register */
- outb(NIC78BX_LOCK_VALUE,
- led_data->io_base + NIC78BX_LOCK_REG_OFFSET);
-
- return 0;
-}
-
static const struct acpi_device_id led_device_ids[] = {
{"NIC78B3", 0},
{"", 0},
@@ -186,7 +188,6 @@ MODULE_DEVICE_TABLE(acpi, led_device_ids);

static struct platform_driver led_driver = {
.probe = nic78bx_probe,
- .remove = nic78bx_remove,
.driver = {
.name = KBUILD_MODNAME,
.acpi_match_table = ACPI_PTR(led_device_ids),
--
2.25.1

2023-12-14 08:31:23

by George Stark

[permalink] [raw]
Subject: [PATCH v3 11/11] leds: powernv: use LED_RETAIN_AT_SHUTDOWN flag for leds

This driver wants to keep its LEDs state after module is removed
and implemented it in its own way. LED subsystem supports dedicated
flag LED_RETAIN_AT_SHUTDOWN for the same purpose so use the flag
instead of custom implementation.

Signed-off-by: George Stark <[email protected]>
---
drivers/leds/leds-powernv.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/drivers/leds/leds-powernv.c b/drivers/leds/leds-powernv.c
index 743e2cdd0891..018ec933ac10 100644
--- a/drivers/leds/leds-powernv.c
+++ b/drivers/leds/leds-powernv.c
@@ -30,15 +30,6 @@ static const struct led_type_map led_type_map[] = {
};

struct powernv_led_common {
- /*
- * By default unload path resets all the LEDs. But on PowerNV
- * platform we want to retain LED state across reboot as these
- * are controlled by firmware. Also service processor can modify
- * the LEDs independent of OS. Hence avoid resetting LEDs in
- * unload path.
- */
- bool led_disabled;
-
/* Max supported LED type */
__be64 max_led_type;

@@ -178,10 +169,6 @@ static int powernv_brightness_set(struct led_classdev *led_cdev,
struct powernv_led_common *powernv_led_common = powernv_led->common;
int rc;

- /* Do not modify LED in unload path */
- if (powernv_led_common->led_disabled)
- return 0;
-
mutex_lock(&powernv_led_common->lock);
rc = powernv_led_set(powernv_led, value);
mutex_unlock(&powernv_led_common->lock);
@@ -225,6 +212,14 @@ static int powernv_led_create(struct device *dev,

powernv_led->cdev.brightness_set_blocking = powernv_brightness_set;
powernv_led->cdev.brightness_get = powernv_brightness_get;
+ /*
+ * By default unload path resets all the LEDs. But on PowerNV
+ * platform we want to retain LED state across reboot as these
+ * are controlled by firmware. Also service processor can modify
+ * the LEDs independent of OS. Hence avoid resetting LEDs in
+ * unload path.
+ */
+ powernv_led->cdev.flags = LED_RETAIN_AT_SHUTDOWN;
powernv_led->cdev.brightness = LED_OFF;
powernv_led->cdev.max_brightness = LED_FULL;

@@ -313,9 +308,7 @@ static int powernv_led_remove(struct platform_device *pdev)
{
struct powernv_led_common *powernv_led_common;

- /* Disable LED operation */
powernv_led_common = platform_get_drvdata(pdev);
- powernv_led_common->led_disabled = true;

/* Destroy lock */
mutex_destroy(&powernv_led_common->lock);
--
2.25.1