[PATCH AUTOSEL 5.4 032/134] leds: lm3692x: Handle failure to probe the regulator

[PATCH AUTOSEL 5.4 032/134] leds: lm3692x: Handle failure to probe the regulator

[Sasha Levin]

From: Guido Günther <agx@sigxcpu.org>

[ Upstream commit 396128d2ffcba6e1954cfdc9a89293ff79cbfd7c ]

Instead use devm_regulator_get_optional since the regulator
is optional and check for errors.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
Acked-by: Pavel Machek <pavel@ucw.cz>
Reviewed-by: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/leds/leds-lm3692x.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 3d381f2f73d04..1ac9a44570eed 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -334,9 +334,18 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		return ret;
 	}
 
-	led->regulator = devm_regulator_get(&led->client->dev, "vled");
-	if (IS_ERR(led->regulator))
+	led->regulator = devm_regulator_get_optional(&led->client->dev, "vled");
+	if (IS_ERR(led->regulator)) {
+		ret = PTR_ERR(led->regulator);
+		if (ret != -ENODEV) {
+			if (ret != -EPROBE_DEFER)
+				dev_err(&led->client->dev,
+					"Failed to get vled regulator: %d\n",
+					ret);
+			return ret;
+		}
 		led->regulator = NULL;
+	}
 
 	child = device_get_next_child_node(&led->client->dev, child);
 	if (!child) {
-- 
2.20.1

Re: [PATCH AUTOSEL 5.4 032/134] leds: lm3692x: Handle failure to probe the regulator

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 857 bytes --]

On Wed 2019-12-11 10:10:08, Sasha Levin wrote:
> From: Guido Günther <agx@sigxcpu.org>
> 
> [ Upstream commit 396128d2ffcba6e1954cfdc9a89293ff79cbfd7c ]
> 
> Instead use devm_regulator_get_optional since the regulator
> is optional and check for errors.

Support for new hardware, more than serious bug.

Please drop. Also please drop leds from your monitoring scripts, it
just adds unneccessary workload for everyone. If we see something
_really_ critical, we'll cc: stable.

Actually, I believe your monitoring scripts are doing more harm than
good, and it would be better to get positive confirmation from
contributors before pushing random changes into stable.

Thanks,
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH AUTOSEL 5.4 033/134] leds: an30259a: add a check for devm_regmap_init_i2c

[Sasha Levin]

From: Chuhong Yuan <hslester96@gmail.com>

[ Upstream commit fc7b5028f2627133c7c18734715a08829eab4d1f ]

an30259a_probe misses a check for devm_regmap_init_i2c and may cause
problems.
Add a check and print errors like other leds drivers.

Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/leds/leds-an30259a.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c
index 250dc9d6f6350..82350a28a5644 100644
--- a/drivers/leds/leds-an30259a.c
+++ b/drivers/leds/leds-an30259a.c
@@ -305,6 +305,13 @@ static int an30259a_probe(struct i2c_client *client)
 
 	chip->regmap = devm_regmap_init_i2c(client, &an30259a_regmap_config);
 
+	if (IS_ERR(chip->regmap)) {
+		err = PTR_ERR(chip->regmap);
+		dev_err(&client->dev, "Failed to allocate register map: %d\n",
+			err);
+		goto exit;
+	}
+
 	for (i = 0; i < chip->num_leds; i++) {
 		struct led_init_data init_data = {};
 
-- 
2.20.1

Re: [PATCH AUTOSEL 5.4 033/134] leds: an30259a: add a check for devm_regmap_init_i2c

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1370 bytes --]

On Wed 2019-12-11 10:10:09, Sasha Levin wrote:
> From: Chuhong Yuan <hslester96@gmail.com>
> 
> [ Upstream commit fc7b5028f2627133c7c18734715a08829eab4d1f ]
> 
> an30259a_probe misses a check for devm_regmap_init_i2c and may cause
> problems.
> Add a check and print errors like other leds drivers.

Please drop.
								Pavel

> Signed-off-by: Chuhong Yuan <hslester96@gmail.com>
> Signed-off-by: Pavel Machek <pavel@ucw.cz>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/leds/leds-an30259a.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c
> index 250dc9d6f6350..82350a28a5644 100644
> --- a/drivers/leds/leds-an30259a.c
> +++ b/drivers/leds/leds-an30259a.c
> @@ -305,6 +305,13 @@ static int an30259a_probe(struct i2c_client *client)
>  
>  	chip->regmap = devm_regmap_init_i2c(client, &an30259a_regmap_config);
>  
> +	if (IS_ERR(chip->regmap)) {
> +		err = PTR_ERR(chip->regmap);
> +		dev_err(&client->dev, "Failed to allocate register map: %d\n",
> +			err);
> +		goto exit;
> +	}
> +
>  	for (i = 0; i < chip->num_leds; i++) {
>  		struct led_init_data init_data = {};
>  
> -- 
> 2.20.1

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[PATCH AUTOSEL 5.4 034/134] leds: trigger: netdev: fix handling on interface rename

[Sasha Levin]

From: Martin Schiller <ms@dev.tdt.de>

[ Upstream commit 5f820ed52371b4f5d8c43c93f03408d0dbc01e5b ]

The NETDEV_CHANGENAME code is not "unneeded" like it is stated in commit
4cb6560514fa ("leds: trigger: netdev: fix refcnt leak on interface
rename").

The event was accidentally misinterpreted equivalent to
NETDEV_UNREGISTER, but should be equivalent to NETDEV_REGISTER.

This was the case in the original code from the openwrt project.

Otherwise, you are unable to set netdev led triggers for (non-existent)
netdevices, which has to be renamed. This is the case, for example, for
ppp interfaces in openwrt.

Fixes: 06f502f57d0d ("leds: trigger: Introduce a NETDEV trigger")
Fixes: 4cb6560514fa ("leds: trigger: netdev: fix refcnt leak on interface rename")
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
Signed-off-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/leds/trigger/ledtrig-netdev.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c
index 136f86a1627d1..d5e774d830215 100644
--- a/drivers/leds/trigger/ledtrig-netdev.c
+++ b/drivers/leds/trigger/ledtrig-netdev.c
@@ -302,10 +302,12 @@ static int netdev_trig_notify(struct notifier_block *nb,
 		container_of(nb, struct led_netdev_data, notifier);
 
 	if (evt != NETDEV_UP && evt != NETDEV_DOWN && evt != NETDEV_CHANGE
-	    && evt != NETDEV_REGISTER && evt != NETDEV_UNREGISTER)
+	    && evt != NETDEV_REGISTER && evt != NETDEV_UNREGISTER
+	    && evt != NETDEV_CHANGENAME)
 		return NOTIFY_DONE;
 
 	if (!(dev == trigger_data->net_dev ||
+	      (evt == NETDEV_CHANGENAME && !strcmp(dev->name, trigger_data->device_name)) ||
 	      (evt == NETDEV_REGISTER && !strcmp(dev->name, trigger_data->device_name))))
 		return NOTIFY_DONE;
 
@@ -315,6 +317,7 @@ static int netdev_trig_notify(struct notifier_block *nb,
 
 	clear_bit(NETDEV_LED_MODE_LINKUP, &trigger_data->mode);
 	switch (evt) {
+	case NETDEV_CHANGENAME:
 	case NETDEV_REGISTER:
 		if (trigger_data->net_dev)
 			dev_put(trigger_data->net_dev);
-- 
2.20.1

Re: [PATCH AUTOSEL 5.4 034/134] leds: trigger: netdev: fix handling on interface rename

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 886 bytes --]

On Wed 2019-12-11 10:10:10, Sasha Levin wrote:
> From: Martin Schiller <ms@dev.tdt.de>
> 
> [ Upstream commit 5f820ed52371b4f5d8c43c93f03408d0dbc01e5b ]
> 
> The NETDEV_CHANGENAME code is not "unneeded" like it is stated in commit
> 4cb6560514fa ("leds: trigger: netdev: fix refcnt leak on interface
> rename").
> 
> The event was accidentally misinterpreted equivalent to
> NETDEV_UNREGISTER, but should be equivalent to NETDEV_REGISTER.
> 
> This was the case in the original code from the openwrt project.
> 
> Otherwise, you are unable to set netdev led triggers for (non-existent)
> netdevices, which has to be renamed. This is the case, for example, for
> ppp interfaces in openwrt.

Please drop.
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH AUTOSEL 5.4 034/134] leds: trigger: netdev: fix handling on interface rename

[Sasha Levin]

On Sat, Dec 14, 2019 at 09:43:31AM +0100, Pavel Machek wrote:
>On Wed 2019-12-11 10:10:10, Sasha Levin wrote:
>> From: Martin Schiller <ms@dev.tdt.de>
>>
>> [ Upstream commit 5f820ed52371b4f5d8c43c93f03408d0dbc01e5b ]
>>
>> The NETDEV_CHANGENAME code is not "unneeded" like it is stated in commit
>> 4cb6560514fa ("leds: trigger: netdev: fix refcnt leak on interface
>> rename").
>>
>> The event was accidentally misinterpreted equivalent to
>> NETDEV_UNREGISTER, but should be equivalent to NETDEV_REGISTER.
>>
>> This was the case in the original code from the openwrt project.
>>
>> Otherwise, you are unable to set netdev led triggers for (non-existent)
>> netdevices, which has to be renamed. This is the case, for example, for
>> ppp interfaces in openwrt.
>
>Please drop.

Here's a bug report from a user (fixed by this patch):

        https://forum.openwrt.org/t/18-06-4-serious-led-problems-td-w8970-v1/40417

He has titled the report "18.06.4:Serious ‘LED’ Problems!", and in the
bug report itself he has mentioned:

        The LED's are really important to me. please let me know how to
        fix this by my self! I cannot report it and wait for next few
        months for patch, again!

There are two other similar bug reports:

        https://bugs.openwrt.org/index.php?do=details&task_id=2193
        https://bugs.openwrt.org/index.php?do=details&task_id=2239

So this is obviously an issue that affects users and needs to be fixed.

Beyond the above, the patch is upstream, it fixes a single issue, and is
shorter than 100 lines.

I'm going to go ahead and ignore your input for this and the rest of the
led patches in the series for similar reasons.

-- 
Thanks,
Sasha