Clean up the driver, fix one leak, and make it possible to use outside
of OF systems.
Andy Shevchenko (6):
leds: spi-byte: call of_node_put() on error path
leds: spi-byte: Get rid of custom led_init_default_state_get()
leds: spi-byte: Make use of device properties
leds: spi-byte: Utilise temporary variable for struct device
leds: spi-byte: Use devm_mutex_init() for mutex initialization
leds: spi-byte: Move OF ID table closer to their user
drivers/leds/Kconfig | 1 -
drivers/leds/leds-spi-byte.c | 63 +++++++++++++-----------------------
2 files changed, 22 insertions(+), 42 deletions(-)
--
2.43.0.rc1.1336.g36b5255a03ac
Add a missing call to of_node_put(np) on error.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/leds/leds-spi-byte.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/leds-spi-byte.c b/drivers/leds/leds-spi-byte.c
index 96296db5f410..b04cf502e603 100644
--- a/drivers/leds/leds-spi-byte.c
+++ b/drivers/leds/leds-spi-byte.c
@@ -91,7 +91,6 @@ static int spi_byte_probe(struct spi_device *spi)
dev_err(dev, "Device must have exactly one LED sub-node.");
return -EINVAL;
}
- child = of_get_next_available_child(dev_of_node(dev), NULL);
led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
if (!led)
@@ -104,11 +103,13 @@ static int spi_byte_probe(struct spi_device *spi)
led->ldev.max_brightness = led->cdef->max_value - led->cdef->off_value;
led->ldev.brightness_set_blocking = spi_byte_brightness_set_blocking;
+ child = of_get_next_available_child(dev_of_node(dev), NULL);
state = of_get_property(child, "default-state", NULL);
if (state) {
if (!strcmp(state, "on")) {
led->ldev.brightness = led->ldev.max_brightness;
} else if (strcmp(state, "off")) {
+ of_node_put(child);
/* all other cases except "off" */
dev_err(dev, "default-state can only be 'on' or 'off'");
return -EINVAL;
@@ -123,9 +124,12 @@ static int spi_byte_probe(struct spi_device *spi)
ret = devm_led_classdev_register_ext(&spi->dev, &led->ldev, &init_data);
if (ret) {
+ of_node_put(child);
mutex_destroy(&led->mutex);
return ret;
}
+
+ of_node_put(child);
spi_set_drvdata(spi, led);
return 0;
--
2.43.0.rc1.1336.g36b5255a03ac
There is no code that uses ID table directly, except the
struct device_driver at the end of the file. Hence, move
table closer to its user. It's always possible to access
them via a pointer.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/leds/leds-spi-byte.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/leds/leds-spi-byte.c b/drivers/leds/leds-spi-byte.c
index 985bbbed251b..d24d0ddf347c 100644
--- a/drivers/leds/leds-spi-byte.c
+++ b/drivers/leds/leds-spi-byte.c
@@ -56,13 +56,6 @@ static const struct spi_byte_chipdef ubnt_acb_spi_led_cdef = {
.max_value = 0x3F,
};
-static const struct of_device_id spi_byte_dt_ids[] = {
- { .compatible = "ubnt,acb-spi-led", .data = &ubnt_acb_spi_led_cdef },
- {},
-};
-
-MODULE_DEVICE_TABLE(of, spi_byte_dt_ids);
-
static int spi_byte_brightness_set_blocking(struct led_classdev *dev,
enum led_brightness brightness)
{
@@ -122,6 +115,12 @@ static int spi_byte_probe(struct spi_device *spi)
return devm_led_classdev_register_ext(dev, &led->ldev, &init_data);
}
+static const struct of_device_id spi_byte_dt_ids[] = {
+ { .compatible = "ubnt,acb-spi-led", .data = &ubnt_acb_spi_led_cdef },
+ {}
+};
+MODULE_DEVICE_TABLE(of, spi_byte_dt_ids);
+
static struct spi_driver spi_byte_driver = {
.probe = spi_byte_probe,
.driver = {
--
2.43.0.rc1.1336.g36b5255a03ac
LED core provides a helper to parse default state from firmware node.
Use it instead of custom implementation.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/leds/leds-spi-byte.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/leds/leds-spi-byte.c b/drivers/leds/leds-spi-byte.c
index b04cf502e603..76733946c99a 100644
--- a/drivers/leds/leds-spi-byte.c
+++ b/drivers/leds/leds-spi-byte.c
@@ -84,7 +84,7 @@ static int spi_byte_probe(struct spi_device *spi)
struct device *dev = &spi->dev;
struct spi_byte_led *led;
struct led_init_data init_data = {};
- const char *state;
+ enum led_default_state state;
int ret;
if (of_get_available_child_count(dev_of_node(dev)) != 1) {
@@ -104,17 +104,10 @@ static int spi_byte_probe(struct spi_device *spi)
led->ldev.brightness_set_blocking = spi_byte_brightness_set_blocking;
child = of_get_next_available_child(dev_of_node(dev), NULL);
- state = of_get_property(child, "default-state", NULL);
- if (state) {
- if (!strcmp(state, "on")) {
- led->ldev.brightness = led->ldev.max_brightness;
- } else if (strcmp(state, "off")) {
- of_node_put(child);
- /* all other cases except "off" */
- dev_err(dev, "default-state can only be 'on' or 'off'");
- return -EINVAL;
- }
- }
+
+ state = led_init_default_state_get(of_fwnode_handle(child));
+ if (state == LEDS_DEFSTATE_ON)
+ led->ldev.brightness = led->ldev.max_brightness;
spi_byte_brightness_set_blocking(&led->ldev,
led->ldev.brightness);
--
2.43.0.rc1.1336.g36b5255a03ac
On Thu, Jun 06, 2024 at 08:29:18PM +0300, Andy Shevchenko wrote:
> Add a missing call to of_node_put(np) on error.
Oh, yeah, this might need the Fixes tag. depending if you want to backport
this fix or no hurry.
--
With Best Regards,
Andy Shevchenko
We have a temporary variable to keep a pointer to struct device.
Utilise it where it makes sense.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/leds/leds-spi-byte.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/leds/leds-spi-byte.c b/drivers/leds/leds-spi-byte.c
index 1fc0a8cc123e..e63958e584c2 100644
--- a/drivers/leds/leds-spi-byte.c
+++ b/drivers/leds/leds-spi-byte.c
@@ -116,7 +116,7 @@ static int spi_byte_probe(struct spi_device *spi)
init_data.devicename = "leds-spi-byte";
init_data.default_label = ":";
- ret = devm_led_classdev_register_ext(&spi->dev, &led->ldev, &init_data);
+ ret = devm_led_classdev_register_ext(dev, &led->ldev, &init_data);
if (ret) {
mutex_destroy(&led->mutex);
return ret;
--
2.43.0.rc1.1336.g36b5255a03ac
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 mutex which was destroyed already
in module's remove() so use devm API instead.
Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/leds/leds-spi-byte.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/leds/leds-spi-byte.c b/drivers/leds/leds-spi-byte.c
index e63958e584c2..985bbbed251b 100644
--- a/drivers/leds/leds-spi-byte.c
+++ b/drivers/leds/leds-spi-byte.c
@@ -31,9 +31,9 @@
#include <linux/leds.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/mutex.h>
#include <linux/property.h>
#include <linux/spi/spi.h>
-#include <linux/mutex.h>
#include <uapi/linux/uleds.h>
struct spi_byte_chipdef {
@@ -97,8 +97,11 @@ static int spi_byte_probe(struct spi_device *spi)
if (!led)
return -ENOMEM;
+ ret = devm_mutex_init(dev, &led->mutex);
+ if (ret)
+ return ret;
+
led->spi = spi;
- mutex_init(&led->mutex);
led->cdef = device_get_match_data(dev);
led->ldev.brightness = LED_OFF;
led->ldev.max_brightness = led->cdef->max_value - led->cdef->off_value;
@@ -116,33 +119,16 @@ static int spi_byte_probe(struct spi_device *spi)
init_data.devicename = "leds-spi-byte";
init_data.default_label = ":";
- ret = devm_led_classdev_register_ext(dev, &led->ldev, &init_data);
- if (ret) {
- mutex_destroy(&led->mutex);
- return ret;
- }
-
- spi_set_drvdata(spi, led);
-
- return 0;
-}
-
-static void spi_byte_remove(struct spi_device *spi)
-{
- struct spi_byte_led *led = spi_get_drvdata(spi);
-
- mutex_destroy(&led->mutex);
+ return devm_led_classdev_register_ext(dev, &led->ldev, &init_data);
}
static struct spi_driver spi_byte_driver = {
.probe = spi_byte_probe,
- .remove = spi_byte_remove,
.driver = {
.name = KBUILD_MODNAME,
.of_match_table = spi_byte_dt_ids,
},
};
-
module_spi_driver(spi_byte_driver);
MODULE_AUTHOR("Christian Mauderer <[email protected]>");
--
2.43.0.rc1.1336.g36b5255a03ac
On Thu, 06 Jun 2024 20:29:17 +0300, Andy Shevchenko wrote:
> Clean up the driver, fix one leak, and make it possible to use outside
> of OF systems.
>
> Andy Shevchenko (6):
> leds: spi-byte: call of_node_put() on error path
> leds: spi-byte: Get rid of custom led_init_default_state_get()
> leds: spi-byte: Make use of device properties
> leds: spi-byte: Utilise temporary variable for struct device
> leds: spi-byte: Use devm_mutex_init() for mutex initialization
> leds: spi-byte: Move OF ID table closer to their user
>
> [...]
Applied, thanks!
[1/6] leds: spi-byte: call of_node_put() on error path
commit: 057739faa00602ca64090e084a664715ee74eb3c
[2/6] leds: spi-byte: Get rid of custom led_init_default_state_get()
commit: 9cb832b2c7bcd8b5decd9e9c07302c802ee25faa
[3/6] leds: spi-byte: Make use of device properties
commit: 3e3735a40ec6694fa70ae20d33545783b9d555af
[4/6] leds: spi-byte: Utilise temporary variable for struct device
commit: dc5f64e4c1531d935c239eb4db3957c5cf569bb6
[5/6] leds: spi-byte: Use devm_mutex_init() for mutex initialization
commit: cc5f66ad5640340ab8fb5753acc7218e2b60d36c
[6/6] leds: spi-byte: Move OF ID table closer to their user
commit: 2ac2628cc23cae5c3a317eaa7f0d3a1d9359a3c9
--
Lee Jones [李琼斯]