2021-05-29 11:20:44

by Andy Shevchenko

Subject: [PATCH v2 01/13] leds: core: The -ENOTSUPP should never be seen by user space

Luckily there is no user which checks for returned code and actually
returns it, but since the function is exported any user may try to return
an error code from it to user space, usually during probe phase,

Replace -ENOTSUPP by -EOPNOTSUPP when returning from exported function.

Fixes: 13ae79bbe4c2 ("leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting")
Cc: Jacek Anaszewski <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: amended the commit message
drivers/leds/led-core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index d56ff4939492..f962620a504f 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -289,6 +289,8 @@ EXPORT_SYMBOL_GPL(led_set_brightness_nosleep);

int led_set_brightness_sync(struct led_classdev *led_cdev, unsigned int value)
{
+ int ret;
+
if (led_cdev->blink_delay_on || led_cdev->blink_delay_off)
return -EBUSY;

@@ -297,7 +299,10 @@ int led_set_brightness_sync(struct led_classdev *led_cdev, unsigned int value)
if (led_cdev->flags & LED_SUSPENDED)
return 0;

- return __led_set_brightness_blocking(led_cdev, led_cdev->brightness);
+ ret = __led_set_brightness_blocking(led_cdev, led_cdev->brightness);
+ if (ret == -ENOTSUPP)
+ return -EOPNOTSUPP;
+ return ret;
}
EXPORT_SYMBOL_GPL(led_set_brightness_sync);

--
2.31.1

2021-05-29 11:20:50

by Andy Shevchenko

Subject: [PATCH v2 06/13] leds: lgm-sso: Remove explicit managed GPIO resource cleanup

The idea of managed resources is that they will be cleaned up automatically
and in the proper order. Remove explicit GPIO cleanup.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: dropped one part of the change, i.e. LED unregistration (Pavel)
drivers/leds/blink/leds-lgm-sso.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index 877b44594b26..24f4057d5a05 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -613,9 +613,6 @@ static void sso_led_shutdown(struct sso_led *led)
if (led->desc.hw_trig)
regmap_update_bits(priv->mmap, SSO_CON3, BIT(led->desc.pin), 0);

- if (led->gpiod)
- devm_gpiod_put(priv->dev, led->gpiod);
-
led->priv = NULL;
}

--
2.31.1

2021-05-29 11:20:55

by Andy Shevchenko

Subject: [PATCH v2 05/13] leds: lgm-sso: Don't spam logs when probe is deferred

When requesting GPIO line the probe can be deferred.
In such case don't spam logs with an error message.
This can be achieved by switching to dev_err_probe().

Fixes: c3987cd2bca3 ("leds: lgm: Add LED controller driver for LGM SoC")
Cc: Amireddy Mallikarjuna reddy <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: fixed authorship / committer (Pavel)
drivers/leds/blink/leds-lgm-sso.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index a27687e0fc04..877b44594b26 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -646,7 +646,7 @@ __sso_led_dt_parse(struct sso_led_priv *priv, struct fwnode_handle *fw_ssoled)
fwnode_child,
GPIOD_ASIS, NULL);
if (IS_ERR(led->gpiod)) {
- dev_err(dev, "led: get gpio fail!\n");
+ dev_err_probe(dev, PTR_ERR(led->gpiod), "led: get gpio fail!\n");
goto __dt_err;
}

--
2.31.1

2021-05-29 11:21:01

by Andy Shevchenko

Subject: [PATCH v2 07/13] leds: lgm-sso: Convert to use list_for_each_entry*() API

Convert to use list_for_each_entry*() API insted of open coded variants.
It saves few lines of code and makes iteasier to read and maintain.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no change
drivers/leds/blink/leds-lgm-sso.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index 24f4057d5a05..77c933051d67 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -623,7 +623,6 @@ __sso_led_dt_parse(struct sso_led_priv *priv, struct fwnode_handle *fw_ssoled)
struct device *dev = priv->dev;
struct sso_led_desc *desc;
struct sso_led *led;
- struct list_head *p;
const char *tmp;
u32 prop;
int ret;
@@ -709,10 +708,8 @@ __sso_led_dt_parse(struct sso_led_priv *priv, struct fwnode_handle *fw_ssoled)
__dt_err:
fwnode_handle_put(fwnode_child);
/* unregister leds */
- list_for_each(p, &priv->led_list) {
- led = list_entry(p, struct sso_led, list);
+ list_for_each_entry(led, &priv->led_list, list)
sso_led_shutdown(led);
- }

return -EINVAL;
}
@@ -843,14 +840,12 @@ static int intel_sso_led_probe(struct platform_device *pdev)
static int intel_sso_led_remove(struct platform_device *pdev)
{
struct sso_led_priv *priv;
- struct list_head *pos, *n;
- struct sso_led *led;
+ struct sso_led *led, *n;

priv = platform_get_drvdata(pdev);

- list_for_each_safe(pos, n, &priv->led_list) {
- list_del(pos);
- led = list_entry(pos, struct sso_led, list);
+ list_for_each_entry_safe(led, n, &priv->led_list, list) {
+ list_del(&led->list);
sso_led_shutdown(led);
}

--
2.31.1

2021-05-29 11:21:12

by Andy Shevchenko

Subject: [PATCH v2 08/13] leds: lm3692x: Correct headers (of*.h -> mod_devicetable.h)

There is no user of of*.h headers, but mod_devicetable.h.
Update header block accordingly.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: dopped OF change (Pavel)
drivers/leds/leds-lm3692x.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index a02756d7ed8f..afe6fb297855 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -7,10 +7,9 @@
#include <linux/init.h>
#include <linux/leds.h>
#include <linux/log2.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
#include <linux/mutex.h>
-#include <linux/of.h>
-#include <linux/of_gpio.h>
#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
--
2.31.1

2021-05-29 11:21:15

by Andy Shevchenko

Subject: [PATCH v2 04/13] leds: lgm-sso: Put fwnode in any case during ->probe()

fwnode_get_next_child_node() bumps a reference counting of a returned variable.
We have to balance it whenever we return to the caller.

All the same in fwnode_for_each_child_node() case.

Fixes: c3987cd2bca3 ("leds: lgm: Add LED controller driver for LGM SoC")
Cc: Amireddy Mallikarjuna reddy <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: simplified sso_led_dt_parse() fix (Pavel)
drivers/leds/blink/leds-lgm-sso.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index e63112d445eb..a27687e0fc04 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -633,8 +633,10 @@ __sso_led_dt_parse(struct sso_led_priv *priv, struct fwnode_handle *fw_ssoled)

fwnode_for_each_child_node(fw_ssoled, fwnode_child) {
led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
- if (!led)
- return -ENOMEM;
+ if (!led) {
+ ret = -ENOMEM;
+ goto __dt_err;
+ }

INIT_LIST_HEAD(&led->list);
led->priv = priv;
@@ -704,11 +706,11 @@ __sso_led_dt_parse(struct sso_led_priv *priv, struct fwnode_handle *fw_ssoled)
if (sso_create_led(priv, led, fwnode_child))
goto __dt_err;
}
- fwnode_handle_put(fw_ssoled);

return 0;
+
__dt_err:
- fwnode_handle_put(fw_ssoled);
+ fwnode_handle_put(fwnode_child);
/* unregister leds */
list_for_each(p, &priv->led_list) {
led = list_entry(p, struct sso_led, list);
@@ -733,6 +735,7 @@ static int sso_led_dt_parse(struct sso_led_priv *priv)
fw_ssoled = fwnode_get_named_child_node(fwnode, "ssoled");
if (fw_ssoled) {
ret = __sso_led_dt_parse(priv, fw_ssoled);
+ fwnode_handle_put(fw_ssoled);
if (ret)
return ret;
}
--
2.31.1

2021-05-29 11:21:46

by Andy Shevchenko

Subject: [PATCH v2 11/13] leds: lt3593: Put fwnode in any case during ->probe()

device_get_next_child_node() bumps a reference counting of a returned variable.
We have to balance it whenever we return to the caller.

Fixes: 8cd7d6daba93 ("leds: lt3593: Add device tree probing glue")
Cc: Daniel Mack <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no changes
drivers/leds/leds-lt3593.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index 3bb52d3165d9..d0160fde0f94 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -97,10 +97,9 @@ static int lt3593_led_probe(struct platform_device *pdev)
init_data.default_label = ":";

ret = devm_led_classdev_register_ext(dev, &led_data->cdev, &init_data);
- if (ret < 0) {
- fwnode_handle_put(child);
+ fwnode_handle_put(child);
+ if (ret < 0)
return ret;
- }

platform_set_drvdata(pdev, led_data);

--
2.31.1

2021-05-29 11:21:53

by Andy Shevchenko

Subject: [PATCH v2 12/13] leds: rt8515: Put fwnode in any case during ->probe()

fwnode_get_next_available_child_node() bumps a reference counting of
a returned variable. We have to balance it whenever we return to
the caller.

Fixes: e1c6edcbea13 ("leds: rt8515: Add Richtek RT8515 LED driver")
Cc: Linus Walleij <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no changes
drivers/leds/flash/leds-rt8515.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/flash/leds-rt8515.c b/drivers/leds/flash/leds-rt8515.c
index 590bfa180d10..44904fdee3cc 100644
--- a/drivers/leds/flash/leds-rt8515.c
+++ b/drivers/leds/flash/leds-rt8515.c
@@ -343,8 +343,9 @@ static int rt8515_probe(struct platform_device *pdev)

ret = devm_led_classdev_flash_register_ext(dev, fled, &init_data);
if (ret) {
- dev_err(dev, "can't register LED %s\n", led->name);
+ fwnode_handle_put(child);
mutex_destroy(&rt->lock);
+ dev_err(dev, "can't register LED %s\n", led->name);
return ret;
}

@@ -362,6 +363,7 @@ static int rt8515_probe(struct platform_device *pdev)
*/
}

+ fwnode_handle_put(child);
return 0;
}

--
2.31.1

2021-05-29 11:21:58

by Andy Shevchenko

Subject: [PATCH v2 09/13] leds: lm3697: Update header block to reflect reality

Currently the headers to be included look rather like a random set.
Update them a bit to reflect the reality.

While at it, drop unneeded dependcy to OF.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: dropped OF change (Pavel)
drivers/leds/leds-lm3697.c | 9 +++++++--
2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index 970a4f34791b..292d64b2eeab 100644
--- a/drivers/leds/leds-lm3697.c
+++ b/drivers/leds/leds-lm3697.c
@@ -2,11 +2,16 @@
// TI LM3697 LED chip family driver
// Copyright (C) 2018 Texas Instruments Incorporated - https://www.ti.com/

+#include <linux/bits.h>
#include <linux/gpio/consumer.h>
#include <linux/i2c.h>
-#include <linux/of.h>
-#include <linux/of_gpio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+
#include <linux/leds-ti-lmu-common.h>

#define LM3697_REV 0x0
--
2.31.1

2021-05-29 11:22:54

by Andy Shevchenko

Subject: [PATCH v2 10/13] leds: lm3697: Make error handling more robust

It's easy to miss necessary clean up, e.g. firmware node reference counting,
during error path in ->probe(). Make it more robust by moving to a single
point of return.

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: don't call for fwnode put on success (Pavel)
drivers/leds/leds-lm3697.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index 292d64b2eeab..a8c9322558cc 100644
--- a/drivers/leds/leds-lm3697.c
+++ b/drivers/leds/leds-lm3697.c
@@ -226,14 +226,12 @@ static int lm3697_probe_dt(struct lm3697 *priv)
ret = fwnode_property_read_u32(child, "reg", &control_bank);
if (ret) {
dev_err(dev, "reg property missing\n");
- fwnode_handle_put(child);
goto child_out;
}

if (control_bank > LM3697_CONTROL_B) {
dev_err(dev, "reg property is invalid\n");
ret = -EINVAL;
- fwnode_handle_put(child);
goto child_out;
}

@@ -264,7 +262,6 @@ static int lm3697_probe_dt(struct lm3697 *priv)
led->num_leds);
if (ret) {
dev_err(dev, "led-sources property missing\n");
- fwnode_handle_put(child);
goto child_out;
}

@@ -289,14 +286,16 @@ static int lm3697_probe_dt(struct lm3697 *priv)
&init_data);
if (ret) {
dev_err(dev, "led register err: %d\n", ret);
- fwnode_handle_put(child);
goto child_out;
}

i++;
}

+ return ret;
+
child_out:
+ fwnode_handle_put(child);
return ret;
}

--
2.31.1

2021-05-29 11:22:59

by Andy Shevchenko

Subject: [PATCH v2 13/13] leds: sgm3140: Put fwnode in any case during ->probe()

fwnode_get_next_child_node() bumps a reference counting of a returned variable.
We have to balance it whenever we return to the caller.

Fixes: cef8ec8cbd21 ("leds: add sgm3140 driver")
Cc: Luca Weiss <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: no changes
drivers/leds/leds-sgm3140.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/leds-sgm3140.c b/drivers/leds/leds-sgm3140.c
index f4f831570f11..df9402071695 100644
--- a/drivers/leds/leds-sgm3140.c
+++ b/drivers/leds/leds-sgm3140.c
@@ -266,12 +266,8 @@ static int sgm3140_probe(struct platform_device *pdev)
child_node,
fled_cdev, NULL,
&v4l2_sd_cfg);
- if (IS_ERR(priv->v4l2_flash)) {
- ret = PTR_ERR(priv->v4l2_flash);
- goto err;
- }
-
- return ret;
+ fwnode_handle_put(child_node);
+ return PTR_ERR_OR_ZERO(priv->v4l2_flash);

err:
fwnode_handle_put(child_node);
--
2.31.1

2021-06-01 10:09:40

by Linus Walleij

Subject: Re: [PATCH v2 12/13] leds: rt8515: Put fwnode in any case during ->probe()

On Sat, May 29, 2021 at 1:19 PM Andy Shevchenko
<[email protected]> wrote:

> fwnode_get_next_available_child_node() bumps a reference counting of
> a returned variable. We have to balance it whenever we return to
> the caller.
>
> Fixes: e1c6edcbea13 ("leds: rt8515: Add Richtek RT8515 LED driver")
> Cc: Linus Walleij <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Linus Walleij <[email protected]>

Yours,
Linus Walleij

2021-06-05 10:04:50

by andy

Subject: Re: [PATCH v2 12/13] leds: rt8515: Put fwnode in any case during ->probe()

Tue, Jun 01, 2021 at 12:06:05PM +0200, Linus Walleij kirjoitti:
> On Sat, May 29, 2021 at 1:19 PM Andy Shevchenko
> <[email protected]> wrote:
>
> > fwnode_get_next_available_child_node() bumps a reference counting of
> > a returned variable. We have to balance it whenever we return to
> > the caller.
> >
> > Fixes: e1c6edcbea13 ("leds: rt8515: Add Richtek RT8515 LED driver")
> > Cc: Linus Walleij <[email protected]>
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> Reviewed-by: Linus Walleij <[email protected]>

Thanks!

Pavel, can you, please, review this batch? I think I addressed most of your
comments if not all.

--
With Best Regards,
Andy Shevchenko

2021-06-23 13:19:27

by Andy Shevchenko

Subject: Re: [PATCH v2 01/13] leds: core: The -ENOTSUPP should never be seen by user space

On Sat, May 29, 2021 at 02:19:23PM +0300, Andy Shevchenko wrote:
> Luckily there is no user which checks for returned code and actually
> returns it, but since the function is exported any user may try to return
> an error code from it to user space, usually during probe phase,
>
> Replace -ENOTSUPP by -EOPNOTSUPP when returning from exported function.

There were no issue reported by bots, no comments from people (except one tag),
can we do something about this series or should I amend it?


--
With Best Regards,
Andy Shevchenko

2021-07-13 07:36:42

by Andy Shevchenko

Subject: Re: [PATCH v2 01/13] leds: core: The -ENOTSUPP should never be seen by user space

On Wed, Jun 23, 2021 at 4:16 PM Andy Shevchenko
<[email protected]> wrote:
>
> On Sat, May 29, 2021 at 02:19:23PM +0300, Andy Shevchenko wrote:
> > Luckily there is no user which checks for returned code and actually
> > returns it, but since the function is exported any user may try to return
> > an error code from it to user space, usually during probe phase,
> >
> > Replace -ENOTSUPP by -EOPNOTSUPP when returning from exported function.
>
> There were no issue reported by bots, no comments from people (except one tag),
> can we do something about this series or should I amend it?

Pavel, do I need to resend this series? It mostly should be the part
of v5.14 cycle (half of it is the fixes).

--
With Best Regards,
Andy Shevchenko

2021-08-03 21:50:03

by Pavel Machek

Subject: Re: [PATCH v2 09/13] leds: lm3697: Update header block to reflect reality

Hi!

> Currently the headers to be included look rather like a random set.
> Update them a bit to reflect the reality.
>
> While at it, drop unneeded dependcy to OF.

3-9 applied, thanks.
Pavel
--
http://www.livejournal.com/~pavelmachek

Attachments:

2021-08-03 21:52:58

by Pavel Machek

Subject: Re: [PATCH v2 12/13] leds: rt8515: Put fwnode in any case during ->probe()

On Sat 2021-06-05 12:46:11, [email protected] wrote:
> Tue, Jun 01, 2021 at 12:06:05PM +0200, Linus Walleij kirjoitti:
> > On Sat, May 29, 2021 at 1:19 PM Andy Shevchenko
> > <[email protected]> wrote:
> >
> > > fwnode_get_next_available_child_node() bumps a reference counting of
> > > a returned variable. We have to balance it whenever we return to
> > > the caller.
> > >
> > > Fixes: e1c6edcbea13 ("leds: rt8515: Add Richtek RT8515 LED driver")
> > > Cc: Linus Walleij <[email protected]>
> > > Signed-off-by: Andy Shevchenko <[email protected]>
> >
> > Reviewed-by: Linus Walleij <[email protected]>
>
> Thanks!
>
> Pavel, can you, please, review this batch? I think I addressed most of your
> comments if not all.

10-12 applied, thanks.
Pavel
--
http://www.livejournal.com/~pavelmachek

Attachments:

2021-08-03 21:54:17

by Pavel Machek

Subject: Re: [PATCH v2 12/13] leds: rt8515: Put fwnode in any case during ->probe()

On Sat 2021-06-05 12:46:11, [email protected] wrote:
> Tue, Jun 01, 2021 at 12:06:05PM +0200, Linus Walleij kirjoitti:
> > On Sat, May 29, 2021 at 1:19 PM Andy Shevchenko
> > <[email protected]> wrote:
> >
> > > fwnode_get_next_available_child_node() bumps a reference counting of
> > > a returned variable. We have to balance it whenever we return to
> > > the caller.
> > >
> > > Fixes: e1c6edcbea13 ("leds: rt8515: Add Richtek RT8515 LED driver")
> > > Cc: Linus Walleij <[email protected]>
> > > Signed-off-by: Andy Shevchenko <[email protected]>
> >
> > Reviewed-by: Linus Walleij <[email protected]>
>
> Thanks!
>
> Pavel, can you, please, review this batch? I think I addressed most of your
> comments if not all.

Your original email is: From: [email protected] . I don't
believe that's right.

Best regards,
Pavel

--
http://www.livejournal.com/~pavelmachek

Attachments: