All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
@ 2021-05-10  9:50 Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 01/28] leds: class: The -ENOTSUPP should never be seen by user space Andy Shevchenko
                   ` (28 more replies)
  0 siblings, 29 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

When analyzing the current state of affairs with fwnode reference counting 
I found that a lot of core doesn't take it right. Here is a bunch of
corresponding fixes against LED drivers.

The series includes some cleanups and a few other fixes grouped by a driver.

First two patches are taking care of -ENOTSUPP error code too  prevent its
appearance in the user space.

Andy Shevchenko (28):
  leds: class: The -ENOTSUPP should never be seen by user space
  leds: core: The -ENOTSUPP should never be seen by user space
  leds: el15203000: Give better margin for usleep_range()
  leds: el15203000: Make error handling more robust
  leds: el15203000: Correct headers (of*.h -> mod_devicetable.h)
  leds: el15203000: Introduce to_el15203000_led() helper
  leds: lgm-sso: Fix clock handling
  leds: lgm-sso: Put fwnode in any case during ->probe()
  leds: lgm-sso: Don't spam logs when probe is deferred
  leds: lgm-sso: Remove unneeded of_match_ptr()
  leds: lgm-sso: Remove explicit managed resource cleanups
  leds: lgm-sso: Drop duplicate NULL check for GPIO operations
  leds: lgm-sso: Convert to use list_for_each_entry*() API
  leds: lm3532: select regmap I2C API
  leds: lm3532: Make error handling more robust
  leds: lm36274: Put fwnode in error case during ->probe()
  leds: lm36274: Correct headers (of*.h -> mod_devicetable.h)
  leds: lm3692x: Put fwnode in any case during ->probe()
  leds: lm3692x: Correct headers (of*.h -> mod_devicetable.h)
  leds: lm3697: Update header block to reflect reality
  leds: lm3697: Make error handling more robust
  leds: lm3697: Don't spam logs when probe is deferred
  leds: lp50xx: Put fwnode in error case during ->probe()
  leds: lt3593: Put fwnode in any case during ->probe()
  leds: lt3593: Make use of device properties
  leds: pwm: Make error handling more robust
  leds: rt8515: Put fwnode in any case during ->probe()
  leds: sgm3140: Put fwnode in any case during ->probe()

 drivers/leds/Kconfig              |  7 ++-
 drivers/leds/blink/leds-lgm-sso.c | 86 +++++++++++++------------------
 drivers/leds/flash/leds-rt8515.c  |  4 +-
 drivers/leds/led-class.c          |  4 --
 drivers/leds/led-core.c           |  7 ++-
 drivers/leds/leds-el15203000.c    | 54 ++++++++-----------
 drivers/leds/leds-lm3532.c        |  7 +--
 drivers/leds/leds-lm36274.c       |  3 +-
 drivers/leds/leds-lm3692x.c       | 11 ++--
 drivers/leds/leds-lm3697.c        | 22 ++++----
 drivers/leds/leds-lp50xx.c        |  2 +-
 drivers/leds/leds-lt3593.c        | 13 ++---
 drivers/leds/leds-pwm.c           | 16 +++---
 drivers/leds/leds-sgm3140.c       |  8 +--
 14 files changed, 106 insertions(+), 138 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v1 01/28] leds: class: The -ENOTSUPP should never be seen by user space
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 02/28] leds: core: " Andy Shevchenko
                   ` (27 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko, Jean-Jacques Hiblot

Drop the bogus error code and let of_led_get() to take care about absent
of_node.

Fixes: e389240ad992 ("leds: Add managed API to get a LED from a device driver")
Cc: Jean-Jacques Hiblot <jjhiblot@ti.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/led-class.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index 2e495ff67856..fa3f5f504ff7 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -285,10 +285,6 @@ struct led_classdev *__must_check devm_of_led_get(struct device *dev,
 	if (!dev)
 		return ERR_PTR(-EINVAL);
 
-	/* Not using device tree? */
-	if (!IS_ENABLED(CONFIG_OF) || !dev->of_node)
-		return ERR_PTR(-ENOTSUPP);
-
 	led = of_led_get(dev->of_node, index);
 	if (IS_ERR(led))
 		return led;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 02/28] leds: core: The -ENOTSUPP should never be seen by user space
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 01/28] leds: class: The -ENOTSUPP should never be seen by user space Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:03   ` Pavel Machek
  2021-05-10  9:50 ` [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range() Andy Shevchenko
                   ` (26 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko, Jacek Anaszewski

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

Fixes: 13ae79bbe4c2 ("leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting")
Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 01/28] leds: class: The -ENOTSUPP should never be seen by user space Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 02/28] leds: core: " Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:04   ` Pavel Machek
  2021-05-10  9:50 ` [PATCH v1 04/28] leds: el15203000: Make error handling more robust Andy Shevchenko
                   ` (25 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

1 microsecond with 20 millisecond parameter is too low margin for
usleep_range(). Give 100 to make scheduler happier.

While at it, fix indentation in cases where EL_FW_DELAY_USEC is in use.
In the loop, move it to the end to avoid a conditional.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-el15203000.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
index 6ca47f2a2004..912451db05e6 100644
--- a/drivers/leds/leds-el15203000.c
+++ b/drivers/leds/leds-el15203000.c
@@ -95,27 +95,22 @@ static int el15203000_cmd(struct el15203000_led *led, u8 brightness)
 
 	/* to avoid SPI mistiming with firmware we should wait some time */
 	if (time_after(led->priv->delay, jiffies)) {
-		dev_dbg(led->priv->dev, "Wait %luus to sync",
-			EL_FW_DELAY_USEC);
+		dev_dbg(led->priv->dev, "Wait %luus to sync", EL_FW_DELAY_USEC);
 
-		usleep_range(EL_FW_DELAY_USEC,
-			     EL_FW_DELAY_USEC + 1);
+		usleep_range(EL_FW_DELAY_USEC, EL_FW_DELAY_USEC + 100);
 	}
 
 	cmd[0] = led->reg;
 	cmd[1] = brightness;
 
 	for (i = 0; i < ARRAY_SIZE(cmd); i++) {
-		if (i)
-			usleep_range(EL_FW_DELAY_USEC,
-				     EL_FW_DELAY_USEC + 1);
-
 		ret = spi_write(led->priv->spi, &cmd[i], sizeof(cmd[i]));
 		if (ret) {
 			dev_err(led->priv->dev,
 				"spi_write() error %d", ret);
 			break;
 		}
+		usleep_range(EL_FW_DELAY_USEC, EL_FW_DELAY_USEC + 100);
 	}
 
 	led->priv->delay = jiffies + usecs_to_jiffies(EL_FW_DELAY_USEC);
@@ -313,8 +308,7 @@ static int el15203000_probe(struct spi_device *spi)
 	priv->count	= count;
 	priv->dev	= &spi->dev;
 	priv->spi	= spi;
-	priv->delay	= jiffies -
-			  usecs_to_jiffies(EL_FW_DELAY_USEC);
+	priv->delay	= jiffies - usecs_to_jiffies(EL_FW_DELAY_USEC);
 
 	spi_set_drvdata(spi, priv);
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 04/28] leds: el15203000: Make error handling more robust
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (2 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 20:59   ` Oleh Kravchenko
  2021-05-10  9:50 ` [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
                   ` (24 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-el15203000.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
index 912451db05e6..bcdbbbc9c187 100644
--- a/drivers/leds/leds-el15203000.c
+++ b/drivers/leds/leds-el15203000.c
@@ -246,16 +246,13 @@ static int el15203000_probe_dt(struct el15203000 *priv)
 		ret = fwnode_property_read_u32(child, "reg", &led->reg);
 		if (ret) {
 			dev_err(priv->dev, "LED without ID number");
-			fwnode_handle_put(child);
-
-			break;
+			goto err_child_out;
 		}
 
 		if (led->reg > U8_MAX) {
 			dev_err(priv->dev, "LED value %d is invalid", led->reg);
-			fwnode_handle_put(child);
-
-			return -EINVAL;
+			ret = -EINVAL;
+			goto err_child_out;
 		}
 
 		led->priv			  = priv;
@@ -277,14 +274,16 @@ static int el15203000_probe_dt(struct el15203000 *priv)
 			dev_err(priv->dev,
 				"failed to register LED device %s, err %d",
 				led->ldev.name, ret);
-			fwnode_handle_put(child);
-
-			break;
+			goto err_child_out;
 		}
 
 		led++;
 	}
 
+	return 0;
+
+err_child_out:
+	fwnode_handle_put(child);
 	return ret;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h)
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (3 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 04/28] leds: el15203000: Make error handling more robust Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 21:00   ` Oleh Kravchenko
  2021-05-10  9:50 ` [PATCH v1 06/28] leds: el15203000: Introduce to_el15203000_led() helper Andy Shevchenko
                   ` (23 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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

While at it, drop unneeded dependency to OF.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/Kconfig           | 1 -
 drivers/leds/leds-el15203000.c | 3 ++-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index dcbfcd491fd0..531c79155717 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -167,7 +167,6 @@ config LEDS_EL15203000
 	tristate "LED Support for Crane EL15203000"
 	depends on LEDS_CLASS
 	depends on SPI
-	depends on OF
 	help
 	  This option enables support for EL15203000 LED Board
 	  (aka RED LED board) which is widely used in coffee vending
diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
index bcdbbbc9c187..fcb90d7cd42f 100644
--- a/drivers/leds/leds-el15203000.c
+++ b/drivers/leds/leds-el15203000.c
@@ -4,8 +4,9 @@
 
 #include <linux/delay.h>
 #include <linux/leds.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
+#include <linux/property.h>
 #include <linux/spi/spi.h>
 
 /*
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 06/28] leds: el15203000: Introduce to_el15203000_led() helper
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (4 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 21:01   ` Oleh Kravchenko
  2021-05-10  9:50 ` [PATCH v1 07/28] leds: lgm-sso: Fix clock handling Andy Shevchenko
                   ` (22 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

Introduce a helper to replace open coded container_of() calls.
At the same time move ldev member to be first in the struct el15203000_led,
that makes container_of() effectivelly a no-op.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-el15203000.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
index fcb90d7cd42f..e81a93d57210 100644
--- a/drivers/leds/leds-el15203000.c
+++ b/drivers/leds/leds-el15203000.c
@@ -69,8 +69,8 @@ enum el15203000_command {
 };
 
 struct el15203000_led {
-	struct el15203000	*priv;
 	struct led_classdev	ldev;
+	struct el15203000	*priv;
 	u32			reg;
 };
 
@@ -83,6 +83,8 @@ struct el15203000 {
 	struct el15203000_led	leds[];
 };
 
+#define to_el15203000_led(d)	container_of(d, struct el15203000_led, ldev)
+
 static int el15203000_cmd(struct el15203000_led *led, u8 brightness)
 {
 	int		ret;
@@ -124,9 +126,7 @@ static int el15203000_cmd(struct el15203000_led *led, u8 brightness)
 static int el15203000_set_blocking(struct led_classdev *ldev,
 				   enum led_brightness brightness)
 {
-	struct el15203000_led *led = container_of(ldev,
-						  struct el15203000_led,
-						  ldev);
+	struct el15203000_led *led = to_el15203000_led(ldev);
 
 	return el15203000_cmd(led, brightness == LED_OFF ? EL_OFF : EL_ON);
 }
@@ -135,9 +135,7 @@ static int el15203000_pattern_set_S(struct led_classdev *ldev,
 				    struct led_pattern *pattern,
 				    u32 len, int repeat)
 {
-	struct el15203000_led *led = container_of(ldev,
-						  struct el15203000_led,
-						  ldev);
+	struct el15203000_led *led = to_el15203000_led(ldev);
 
 	if (repeat > 0 || len != 2 ||
 	    pattern[0].delta_t != 4000 || pattern[0].brightness != 0 ||
@@ -188,10 +186,8 @@ static int el15203000_pattern_set_P(struct led_classdev *ldev,
 				    struct led_pattern *pattern,
 				    u32 len, int repeat)
 {
+	struct el15203000_led	*led = to_el15203000_led(ldev);
 	u8			cmd;
-	struct el15203000_led	*led = container_of(ldev,
-						    struct el15203000_led,
-						    ldev);
 
 	if (repeat > 0)
 		return -EINVAL;
@@ -228,9 +224,7 @@ static int el15203000_pattern_set_P(struct led_classdev *ldev,
 
 static int el15203000_pattern_clear(struct led_classdev *ldev)
 {
-	struct el15203000_led	*led = container_of(ldev,
-						    struct el15203000_led,
-						    ldev);
+	struct el15203000_led *led = to_el15203000_led(ldev);
 
 	return el15203000_cmd(led, EL_OFF);
 }
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 07/28] leds: lgm-sso: Fix clock handling
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (5 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 06/28] leds: el15203000: Introduce to_el15203000_led() helper Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe() Andy Shevchenko
                   ` (21 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

The clock handling has a few issues:
 - when getting second clock fails, the first one left prepared and enabled
 - on ->remove() clocks are unprepared and disabled twice

Fix all these by converting to use bulk clock operations since both clocks
are mandatory.

Fixes: c3987cd2bca3 ("leds: lgm: Add LED controller driver for LGM SoC")
Cc: Amireddy Mallikarjuna reddy <mallikarjunax.reddy@linux.intel.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/blink/leds-lgm-sso.c | 44 ++++++++++++-------------------
 1 file changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index 484f6831e6e7..6a6d75f07af0 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -133,8 +133,7 @@ struct sso_led_priv {
 	struct regmap *mmap;
 	struct device *dev;
 	struct platform_device *pdev;
-	struct clk *gclk;
-	struct clk *fpid_clk;
+	struct clk_bulk_data clocks[2];
 	u32 fpid_clkrate;
 	u32 gptc_clkrate;
 	u32 freq[MAX_FREQ_RANK];
@@ -766,12 +765,11 @@ static int sso_probe_gpios(struct sso_led_priv *priv)
 	return sso_gpio_gc_init(dev, priv);
 }
 
-static void sso_clk_disable(void *data)
+static void sso_clock_disable_unprepare(void *data)
 {
 	struct sso_led_priv *priv = data;
 
-	clk_disable_unprepare(priv->fpid_clk);
-	clk_disable_unprepare(priv->gclk);
+	clk_bulk_disable_unprepare(ARRAY_SIZE(priv->clocks), priv->clocks);
 }
 
 static int intel_sso_led_probe(struct platform_device *pdev)
@@ -788,36 +786,30 @@ static int intel_sso_led_probe(struct platform_device *pdev)
 	priv->dev = dev;
 
 	/* gate clock */
-	priv->gclk = devm_clk_get(dev, "sso");
-	if (IS_ERR(priv->gclk)) {
-		dev_err(dev, "get sso gate clock failed!\n");
-		return PTR_ERR(priv->gclk);
-	}
+	priv->clocks[0].id = "sso";
+
+	/* fpid clock */
+	priv->clocks[1].id = "fpid";
 
-	ret = clk_prepare_enable(priv->gclk);
+	ret = devm_clk_bulk_get(dev, ARRAY_SIZE(priv->clocks), priv->clocks);
 	if (ret) {
-		dev_err(dev, "Failed to prepare/enable sso gate clock!\n");
+		dev_err(dev, "Getting clocks failed!\n");
 		return ret;
 	}
 
-	priv->fpid_clk = devm_clk_get(dev, "fpid");
-	if (IS_ERR(priv->fpid_clk)) {
-		dev_err(dev, "Failed to get fpid clock!\n");
-		return PTR_ERR(priv->fpid_clk);
-	}
-
-	ret = clk_prepare_enable(priv->fpid_clk);
+	ret = clk_bulk_prepare_enable(ARRAY_SIZE(priv->clocks), priv->clocks);
 	if (ret) {
-		dev_err(dev, "Failed to prepare/enable fpid clock!\n");
+		dev_err(dev, "Failed to prepare and enable clocks!\n");
 		return ret;
 	}
-	priv->fpid_clkrate = clk_get_rate(priv->fpid_clk);
 
-	ret = devm_add_action_or_reset(dev, sso_clk_disable, priv);
-	if (ret) {
-		dev_err(dev, "Failed to devm_add_action_or_reset, %d\n", ret);
+	ret = devm_add_action_or_reset(dev, sso_clock_disable_unprepare, priv);
+	if (ret)
 		return ret;
-	}
+
+	priv->fpid_clkrate = clk_get_rate(priv->clocks[1].clk);
+
+	priv->mmap = syscon_node_to_regmap(dev->of_node);
 
 	priv->mmap = syscon_node_to_regmap(dev->of_node);
 	if (IS_ERR(priv->mmap)) {
@@ -862,8 +854,6 @@ static int intel_sso_led_remove(struct platform_device *pdev)
 		sso_led_shutdown(led);
 	}
 
-	clk_disable_unprepare(priv->fpid_clk);
-	clk_disable_unprepare(priv->gclk);
 	regmap_exit(priv->mmap);
 
 	return 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (6 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 07/28] leds: lgm-sso: Fix clock handling Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:08   ` Pavel Machek
  2021-05-10  9:50 ` [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred Andy Shevchenko
                   ` (20 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <mallikarjunax.reddy@linux.intel.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/blink/leds-lgm-sso.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index 6a6d75f07af0..f6c7a5d0c2f7 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);
@@ -734,10 +736,15 @@ static int sso_led_dt_parse(struct sso_led_priv *priv)
 	if (fw_ssoled) {
 		ret = __sso_led_dt_parse(priv, fw_ssoled);
 		if (ret)
-			return ret;
+			goto err_child_out;
 	}
 
+	fwnode_handle_put(fw_ssoled);
 	return 0;
+
+err_child_out:
+	fwnode_handle_put(fw_ssoled);
+	return ret;
 }
 
 static int sso_probe_gpios(struct sso_led_priv *priv)
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (7 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:11   ` Pavel Machek
  2021-05-10  9:50 ` [PATCH v1 10/28] leds: lgm-sso: Remove unneeded of_match_ptr() Andy Shevchenko
                   ` (19 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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 <mallikarjunax.reddy@linux.intel.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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 f6c7a5d0c2f7..91844dcb8bc7 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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 10/28] leds: lgm-sso: Remove unneeded of_match_ptr()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (8 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups Andy Shevchenko
                   ` (18 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

LGM SSO is an OF dependent driver, so of_match_ptr() can be safely
removed.

Remove the unneeded of_match_ptr().

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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 91844dcb8bc7..e76be25480b4 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -878,7 +878,7 @@ static struct platform_driver intel_sso_led_driver = {
 	.remove		= intel_sso_led_remove,
 	.driver		= {
 			.name = "lgm-ssoled",
-			.of_match_table = of_match_ptr(of_sso_led_match),
+			.of_match_table = of_sso_led_match,
 	},
 };
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (9 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 10/28] leds: lgm-sso: Remove unneeded of_match_ptr() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:09   ` Pavel Machek
  2021-05-10  9:50 ` [PATCH v1 12/28] leds: lgm-sso: Drop duplicate NULL check for GPIO operations Andy Shevchenko
                   ` (17 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/blink/leds-lgm-sso.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
index e76be25480b4..a7f2e5436ba2 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -606,16 +606,10 @@ static void sso_led_shutdown(struct sso_led *led)
 {
 	struct sso_led_priv *priv = led->priv;
 
-	/* unregister led */
-	devm_led_classdev_unregister(priv->dev, &led->cdev);
-
 	/* clear HW control bit */
 	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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 12/28] leds: lgm-sso: Drop duplicate NULL check for GPIO operations
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (10 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 13/28] leds: lgm-sso: Convert to use list_for_each_entry*() API Andy Shevchenko
                   ` (16 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

Since GPIO operations are NULL-aware, we don't need to duplicate
this check. Remove it and fold the rest of the code.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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 a7f2e5436ba2..f44d6bf5a5b3 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -259,7 +259,7 @@ static void sso_led_brightness_set(struct led_classdev *led_cdev,
 				   1 << desc->pin);
 	}
 
-	if (!desc->hw_trig && led->gpiod)
+	if (!desc->hw_trig)
 		gpiod_set_value(led->gpiod, val);
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 13/28] leds: lgm-sso: Convert to use list_for_each_entry*() API
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (11 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 12/28] leds: lgm-sso: Drop duplicate NULL check for GPIO operations Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 14/28] leds: lm3532: select regmap I2C API Andy Shevchenko
                   ` (15 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <andy.shevchenko@gmail.com>
---
 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 f44d6bf5a5b3..5505eda3c800 100644
--- a/drivers/leds/blink/leds-lgm-sso.c
+++ b/drivers/leds/blink/leds-lgm-sso.c
@@ -620,7 +620,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;
@@ -706,10 +705,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;
 }
@@ -844,14 +841,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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 14/28] leds: lm3532: select regmap I2C API
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (12 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 13/28] leds: lgm-sso: Convert to use list_for_each_entry*() API Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 15/28] leds: lm3532: Make error handling more robust Andy Shevchenko
                   ` (14 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

Regmap APIs should be selected, otherwise link can fail

ERROR: modpost: "__devm_regmap_init_i2c" [drivers/leds/leds-lm3532.ko] undefined!

Fixes: bc1b8492c764 ("leds: lm3532: Introduce the lm3532 LED driver")
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index 531c79155717..b06eb12f14bf 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -198,6 +198,7 @@ config LEDS_LM3530
 
 config LEDS_LM3532
 	tristate "LCD Backlight driver for LM3532"
+	select REGMAP_I2C
 	depends on LEDS_CLASS
 	depends on I2C
 	help
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 15/28] leds: lm3532: Make error handling more robust
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (13 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 14/28] leds: lm3532: select regmap I2C API Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 16/28] leds: lm36274: Put fwnode in error case during ->probe() Andy Shevchenko
                   ` (13 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lm3532.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 0bf25bdde02f..beb53040e09e 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -586,7 +586,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 		ret = fwnode_property_read_u32(child, "reg", &control_bank);
 		if (ret) {
 			dev_err(&priv->client->dev, "reg property missing\n");
-			fwnode_handle_put(child);
 			goto child_out;
 		}
 
@@ -601,7 +600,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 					       &led->mode);
 		if (ret) {
 			dev_err(&priv->client->dev, "ti,led-mode property missing\n");
-			fwnode_handle_put(child);
 			goto child_out;
 		}
 
@@ -636,7 +634,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 						    led->num_leds);
 		if (ret) {
 			dev_err(&priv->client->dev, "led-sources property missing\n");
-			fwnode_handle_put(child);
 			goto child_out;
 		}
 
@@ -647,7 +644,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 		if (ret) {
 			dev_err(&priv->client->dev, "led register err: %d\n",
 				ret);
-			fwnode_handle_put(child);
 			goto child_out;
 		}
 
@@ -655,14 +651,15 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 		if (ret) {
 			dev_err(&priv->client->dev, "register init err: %d\n",
 				ret);
-			fwnode_handle_put(child);
 			goto child_out;
 		}
 
 		i++;
 	}
+	return 0;
 
 child_out:
+	fwnode_handle_put(child);
 	return ret;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 16/28] leds: lm36274: Put fwnode in error case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (14 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 15/28] leds: lm3532: Make error handling more robust Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 17/28] leds: lm36274: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
                   ` (12 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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

In the older code the same is implied with device_for_each_child_node().

Fixes: 11e1bbc116a7 ("leds: lm36274: Introduce the TI LM36274 LED driver")
Fixes: a448fcf19c9c ("leds: lm36274: don't iterate through children since there is only one")
Cc: Dan Murphy <dmurphy@ti.com>
Cc: Marek Behún <marek.behun@nic.cz>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lm36274.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index aadb03468a40..a23a9424c2f3 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -127,6 +127,7 @@ static int lm36274_probe(struct platform_device *pdev)
 
 	ret = lm36274_init(chip);
 	if (ret) {
+		fwnode_handle_put(init_data.fwnode);
 		dev_err(chip->dev, "Failed to init the device\n");
 		return ret;
 	}
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 17/28] leds: lm36274: Correct headers (of*.h -> mod_devicetable.h)
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (15 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 16/28] leds: lm36274: Put fwnode in error case during ->probe() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 18/28] leds: lm3692x: Put fwnode in any case during ->probe() Andy Shevchenko
                   ` (11 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lm36274.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index a23a9424c2f3..90dc5cbebed4 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -7,8 +7,8 @@
 #include <linux/err.h>
 #include <linux/leds.h>
 #include <linux/leds-ti-lmu-common.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of_device.h>
 #include <linux/platform_device.h>
 
 #include <linux/mfd/ti-lmu.h>
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 18/28] leds: lm3692x: Put fwnode in any case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (16 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 17/28] leds: lm36274: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 19/28] leds: lm3692x: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
                   ` (10 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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: 9a5c1c64ac0a ("leds: lm3692x: Change DT calls to fwnode calls")
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lm3692x.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index e945de45388c..55e6443997ec 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -435,6 +435,7 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 
 	ret = fwnode_property_read_u32(child, "reg", &led->led_enable);
 	if (ret) {
+		fwnode_handle_put(child);
 		dev_err(&led->client->dev, "reg DT property missing\n");
 		return ret;
 	}
@@ -449,12 +450,11 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 
 	ret = devm_led_classdev_register_ext(&led->client->dev, &led->led_dev,
 					     &init_data);
-	if (ret) {
+	if (ret)
 		dev_err(&led->client->dev, "led register err: %d\n", ret);
-		return ret;
-	}
 
-	return 0;
+	fwnode_handle_put(init_data.fwnode);
+	return ret;
 }
 
 static int lm3692x_probe(struct i2c_client *client,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 19/28] leds: lm3692x: Correct headers (of*.h -> mod_devicetable.h)
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (17 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 18/28] leds: lm3692x: Put fwnode in any case during ->probe() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 20/28] leds: lm3697: Update header block to reflect reality Andy Shevchenko
                   ` (9 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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

While at it, drop unneeded dependency to OF.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/Kconfig        | 2 +-
 drivers/leds/leds-lm3692x.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index b06eb12f14bf..ff80142facbb 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -232,7 +232,7 @@ config LEDS_LM3642
 
 config LEDS_LM3692X
 	tristate "LED support for LM3692x Chips"
-	depends on LEDS_CLASS && I2C && OF
+	depends on LEDS_CLASS && I2C
 	select REGMAP_I2C
 	help
 	  This option enables support for the TI LM3692x family
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 55e6443997ec..50b4b8ee49fd 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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 20/28] leds: lm3697: Update header block to reflect reality
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (18 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 19/28] leds: lm3692x: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 21/28] leds: lm3697: Make error handling more robust Andy Shevchenko
                   ` (8 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <andy.shevchenko@gmail.com>
---
 drivers/leds/Kconfig       | 2 +-
 drivers/leds/leds-lm3697.c | 9 +++++++--
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index ff80142facbb..e3ee3c34d535 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -878,7 +878,7 @@ config LEDS_TI_LMU_COMMON
 config LEDS_LM3697
 	tristate "LED driver for LM3697"
 	depends on LEDS_TI_LMU_COMMON
-	depends on I2C && OF
+	depends on I2C
 	help
 	  Say Y to enable the LM3697 LED driver for TI LMU devices.
 	  This supports the LED device LM3697.
diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index 7d216cdb91a8..9d35dd2a9bf0 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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 21/28] leds: lm3697: Make error handling more robust
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (19 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 20/28] leds: lm3697: Update header block to reflect reality Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:10   ` Pavel Machek
  2021-05-10  9:50 ` [PATCH v1 22/28] leds: lm3697: Don't spam logs when probe is deferred Andy Shevchenko
                   ` (7 subsequent siblings)
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lm3697.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index 9d35dd2a9bf0..6262ae69591e 100644
--- a/drivers/leds/leds-lm3697.c
+++ b/drivers/leds/leds-lm3697.c
@@ -224,14 +224,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;
 		}
 
@@ -262,7 +260,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;
 		}
 
@@ -287,7 +284,6 @@ 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;
 		}
 
@@ -295,6 +291,7 @@ static int lm3697_probe_dt(struct lm3697 *priv)
 	}
 
 child_out:
+	fwnode_handle_put(child);
 	return ret;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 22/28] leds: lm3697: Don't spam logs when probe is deferred
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (20 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 21/28] leds: lm3697: Make error handling more robust Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 23/28] leds: lp50xx: Put fwnode in error case during ->probe() Andy Shevchenko
                   ` (6 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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: 5c1d824cda9f ("leds: lm3697: Introduce the lm3697 driver")
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lm3697.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index 6262ae69591e..7e52f9bd9469 100644
--- a/drivers/leds/leds-lm3697.c
+++ b/drivers/leds/leds-lm3697.c
@@ -208,11 +208,9 @@ static int lm3697_probe_dt(struct lm3697 *priv)
 
 	priv->enable_gpio = devm_gpiod_get_optional(dev, "enable",
 						    GPIOD_OUT_LOW);
-	if (IS_ERR(priv->enable_gpio)) {
-		ret = PTR_ERR(priv->enable_gpio);
-		dev_err(dev, "Failed to get enable gpio: %d\n", ret);
-		return ret;
-	}
+	if (IS_ERR(priv->enable_gpio))
+		return dev_err_probe(dev, PTR_ERR(priv->enable_gpio),
+					  "Failed to get enable GPIO\n");
 
 	priv->regulator = devm_regulator_get(dev, "vled");
 	if (IS_ERR(priv->regulator))
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 23/28] leds: lp50xx: Put fwnode in error case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (21 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 22/28] leds: lm3697: Don't spam logs when probe is deferred Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 24/28] leds: lt3593: Put fwnode in any " Andy Shevchenko
                   ` (5 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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

OTOH, the successful iteration will drop reference count under the hood, no need
to do it twice.

Fixes: 242b81170fb8 ("leds: lp50xx: Add the LP50XX family of the RGB LED driver")
Cc: Dan Murphy <dmurphy@ti.com>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-lp50xx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index 06230614fdc5..401df1e2e05d 100644
--- a/drivers/leds/leds-lp50xx.c
+++ b/drivers/leds/leds-lp50xx.c
@@ -490,6 +490,7 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
 			ret = fwnode_property_read_u32(led_node, "color",
 						       &color_id);
 			if (ret) {
+				fwnode_handle_put(led_node);
 				dev_err(priv->dev, "Cannot read color\n");
 				goto child_out;
 			}
@@ -512,7 +513,6 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
 			goto child_out;
 		}
 		i++;
-		fwnode_handle_put(child);
 	}
 
 	return 0;
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 24/28] leds: lt3593: Put fwnode in any case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (22 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 23/28] leds: lp50xx: Put fwnode in error case during ->probe() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 25/28] leds: lt3593: Make use of device properties Andy Shevchenko
                   ` (4 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko, Daniel Mack

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 <daniel@zonque.org>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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 68e06434ac08..7dab08773a34 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -99,10 +99,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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 25/28] leds: lt3593: Make use of device properties
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (23 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 24/28] leds: lt3593: Put fwnode in any " Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 26/28] leds: pwm: Make error handling more robust Andy Shevchenko
                   ` (3 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Device property API allows to gather device resources from different sources,
such as ACPI. Convert the driver to unleash the power of device property API.

Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/leds/Kconfig       | 1 -
 drivers/leds/leds-lt3593.c | 8 +++-----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
index e3ee3c34d535..9ab706d5ded7 100644
--- a/drivers/leds/Kconfig
+++ b/drivers/leds/Kconfig
@@ -616,7 +616,6 @@ config LEDS_LT3593
 	tristate "LED driver for LT3593 controllers"
 	depends on LEDS_CLASS
 	depends on GPIOLIB || COMPILE_TEST
-	depends on OF
 	help
 	  This option enables support for LEDs driven by a Linear Technology
 	  LT3593 controller. This controller uses a special one-wire pulse
diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index 7dab08773a34..d0160fde0f94 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -7,8 +7,9 @@
 #include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/slab.h>
+#include <linux/mod_devicetable.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/property.h>
 
 #define LED_LT3593_NAME "lt3593"
 
@@ -68,9 +69,6 @@ static int lt3593_led_probe(struct platform_device *pdev)
 	struct led_init_data init_data = {};
 	const char *tmp;
 
-	if (!dev_of_node(dev))
-		return -ENODEV;
-
 	led_data = devm_kzalloc(dev, sizeof(*led_data), GFP_KERNEL);
 	if (!led_data)
 		return -ENOMEM;
@@ -118,7 +116,7 @@ static struct platform_driver lt3593_led_driver = {
 	.probe		= lt3593_led_probe,
 	.driver		= {
 		.name	= "leds-lt3593",
-		.of_match_table = of_match_ptr(of_lt3593_leds_match),
+		.of_match_table = of_lt3593_leds_match,
 	},
 };
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 26/28] leds: pwm: Make error handling more robust
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (24 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 25/28] leds: lt3593: Make use of device properties Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 27/28] leds: rt8515: Put fwnode in any case during ->probe() Andy Shevchenko
                   ` (2 subsequent siblings)
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <andy.shevchenko@gmail.com>
---
 drivers/leds/leds-pwm.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index f53f9309ca6c..d71e9fa5c8de 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -101,7 +101,7 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 {
 	struct fwnode_handle *fwnode;
 	struct led_pwm led;
-	int ret = 0;
+	int ret;
 
 	memset(&led, 0, sizeof(led));
 
@@ -111,8 +111,8 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 			led.name = to_of_node(fwnode)->name;
 
 		if (!led.name) {
-			fwnode_handle_put(fwnode);
-			return -EINVAL;
+			ret = EINVAL;
+			goto err_child_out;
 		}
 
 		led.active_low = fwnode_property_read_bool(fwnode,
@@ -121,12 +121,14 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 					 &led.max_brightness);
 
 		ret = led_pwm_add(dev, priv, &led, fwnode);
-		if (ret) {
-			fwnode_handle_put(fwnode);
-			break;
-		}
+		if (ret)
+			goto err_child_out;
 	}
 
+	return 0;
+
+err_child_out:
+	fwnode_handle_put(fwnode);
 	return ret;
 }
 
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 27/28] leds: rt8515: Put fwnode in any case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (25 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 26/28] leds: pwm: Make error handling more robust Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-10  9:50 ` [PATCH v1 28/28] leds: sgm3140: " Andy Shevchenko
  2021-05-17  7:30 ` [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
  28 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko

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 <linus.walleij@linaro.org>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* [PATCH v1 28/28] leds: sgm3140: Put fwnode in any case during ->probe()
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (26 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 27/28] leds: rt8515: Put fwnode in any case during ->probe() Andy Shevchenko
@ 2021-05-10  9:50 ` Andy Shevchenko
  2021-05-28 10:14   ` Pavel Machek
  2021-05-17  7:30 ` [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
  28 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-10  9:50 UTC (permalink / raw)
  To: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel
  Cc: Andy Shevchenko, Luca Weiss

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 <luca@z3ntu.xyz>
Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
                   ` (27 preceding siblings ...)
  2021-05-10  9:50 ` [PATCH v1 28/28] leds: sgm3140: " Andy Shevchenko
@ 2021-05-17  7:30 ` Andy Shevchenko
  2021-05-24 14:56   ` Andy Shevchenko
  2021-05-28 10:02   ` Pavel Machek
  28 siblings, 2 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-17  7:30 UTC (permalink / raw)
  To: Pavel Machek, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

On Mon, May 10, 2021 at 12:50:17PM +0300, Andy Shevchenko wrote:
> When analyzing the current state of affairs with fwnode reference counting
> I found that a lot of core doesn't take it right. Here is a bunch of
> corresponding fixes against LED drivers.
> 
> The series includes some cleanups and a few other fixes grouped by a driver.
> 
> First two patches are taking care of -ENOTSUPP error code too  prevent its
> appearance in the user space.

Pavel, any comments on this bug fix series?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-17  7:30 ` [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
@ 2021-05-24 14:56   ` Andy Shevchenko
  2021-05-24 17:49     ` Pavel Machek
  2021-05-28 10:02   ` Pavel Machek
  1 sibling, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-24 14:56 UTC (permalink / raw)
  To: Pavel Machek, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

On Mon, May 17, 2021 at 10:30:08AM +0300, Andy Shevchenko wrote:
> On Mon, May 10, 2021 at 12:50:17PM +0300, Andy Shevchenko wrote:
> > When analyzing the current state of affairs with fwnode reference counting
> > I found that a lot of core doesn't take it right. Here is a bunch of
> > corresponding fixes against LED drivers.
> > 
> > The series includes some cleanups and a few other fixes grouped by a driver.
> > 
> > First two patches are taking care of -ENOTSUPP error code too  prevent its
> > appearance in the user space.
> 
> Pavel, any comments on this bug fix series?

Pavel, we are at rc-3 already and this is kinda a big series that needs more
time to be sit in Linux-next, unfortunately while I see your activities here
and there, it is kept uncommented and unapplied. Can you shed a light what's
going on here? Do I need something to be amended?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-24 14:56   ` Andy Shevchenko
@ 2021-05-24 17:49     ` Pavel Machek
  2021-05-24 18:39       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-24 17:49 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

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

On Mon 2021-05-24 17:56:13, Andy Shevchenko wrote:
> On Mon, May 17, 2021 at 10:30:08AM +0300, Andy Shevchenko wrote:
> > On Mon, May 10, 2021 at 12:50:17PM +0300, Andy Shevchenko wrote:
> > > When analyzing the current state of affairs with fwnode reference counting
> > > I found that a lot of core doesn't take it right. Here is a bunch of
> > > corresponding fixes against LED drivers.
> > > 
> > > The series includes some cleanups and a few other fixes grouped by a driver.
> > > 
> > > First two patches are taking care of -ENOTSUPP error code too  prevent its
> > > appearance in the user space.
> > 
> > Pavel, any comments on this bug fix series?
> 
> Pavel, we are at rc-3 already and this is kinda a big series that needs more
> time to be sit in Linux-next, unfortunately while I see your activities here
> and there, it is kept uncommented and unapplied. Can you shed a light what's
> going on here? Do I need something to be amended?

I'm busy, sorry.

Series looks kind of okay on quick look.

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

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

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-24 17:49     ` Pavel Machek
@ 2021-05-24 18:39       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-24 18:39 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Mon, May 24, 2021 at 07:49:03PM +0200, Pavel Machek wrote:
> On Mon 2021-05-24 17:56:13, Andy Shevchenko wrote:
> > On Mon, May 17, 2021 at 10:30:08AM +0300, Andy Shevchenko wrote:
> > > On Mon, May 10, 2021 at 12:50:17PM +0300, Andy Shevchenko wrote:
> > > > When analyzing the current state of affairs with fwnode reference counting
> > > > I found that a lot of core doesn't take it right. Here is a bunch of
> > > > corresponding fixes against LED drivers.
> > > > 
> > > > The series includes some cleanups and a few other fixes grouped by a driver.
> > > > 
> > > > First two patches are taking care of -ENOTSUPP error code too  prevent its
> > > > appearance in the user space.
> > > 
> > > Pavel, any comments on this bug fix series?
> > 
> > Pavel, we are at rc-3 already and this is kinda a big series that needs more
> > time to be sit in Linux-next, unfortunately while I see your activities here
> > and there, it is kept uncommented and unapplied. Can you shed a light what's
> > going on here? Do I need something to be amended?
> 
> I'm busy, sorry.

Oh, I see. Good you eventually answered!

> Series looks kind of okay on quick look.

I'll look forward to see it applied at some point in the future, thanks!


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-17  7:30 ` [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
  2021-05-24 14:56   ` Andy Shevchenko
@ 2021-05-28 10:02   ` Pavel Machek
  2021-05-28 11:05     ` Andy Shevchenko
  1 sibling, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:02 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

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

On Mon 2021-05-17 10:30:08, Andy Shevchenko wrote:
> On Mon, May 10, 2021 at 12:50:17PM +0300, Andy Shevchenko wrote:
> > When analyzing the current state of affairs with fwnode reference counting
> > I found that a lot of core doesn't take it right. Here is a bunch of
> > corresponding fixes against LED drivers.
> > 
> > The series includes some cleanups and a few other fixes grouped by a driver.
> > 
> > First two patches are taking care of -ENOTSUPP error code too  prevent its
> > appearance in the user space.
> 
> Pavel, any comments on this bug fix series?

I took these:

95138e01275e1af3f1fc2780fe1d9c6138b29c7a leds: pwm: Make error
handling more robust
d33e98a1f3ee76f38668304f9ffce49af07da77a leds: lt3593: Make use of
device properties
f1e1d532da7e6ef355528a22fb97d9a8fbf76c4e leds: lp50xx: Put fwnode in
error case during ->probe()
807553f8bf4afa673750e52905e0f9488179112f leds: lm3697: Don't spam logs
when probe is deferred
f55db1c7fadc2a29c9fa4ff3aec98dbb111f2206 leds: lm3692x: Put fwnode in
any case during ->probe()
e2e8e4e8187509a77cb6328d876d9c09c07c2e82 leds: lm36274: Correct
headers (of*.h -> mod_devicetable.h)
3c5f655c44bb65cb7e3c219d08c130ce5fa45d7f leds: lm36274: Put fwnode in
error case during ->probe()
2f39f68cec0a19c0371c1e7cb149159810e87f64 leds: lm3532: Make error
handling more robust
99be74f61cb0292b518f5e6d7e5c6611555c2ec7 leds: lm3532: select regmap
I2C API
f3e2b3825ffb034b001fbe283d7a32a56e41aca7 leds: lgm-sso: Drop duplicate
NULL check for GPIO operations
2cbbe9c50d13b6417e0baf8e8475ed73d4d12c2d leds: lgm-sso: Remove
unneeded of_match_ptr()
fba8a6f2263bc54683cf3fd75df4dbd5d041c195 leds: lgm-sso: Fix clock
handling
a43a4e588e72bafc81924d61bf1167cd6810ecbb leds: el15203000: Introduce
to_el15203000_led() helper
0ac40af86077982a5346dbc9655172d2775d6b08 leds: class: The -ENOTSUPP
should never be seen by user space

For the "remove depends on OF"... I'd preffer not to take those. We
don't need to ask the user for configurations that never happen.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 02/28] leds: core: The -ENOTSUPP should never be seen by user space
  2021-05-10  9:50 ` [PATCH v1 02/28] leds: core: " Andy Shevchenko
@ 2021-05-28 10:03   ` Pavel Machek
  2021-05-28 10:43     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:03 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel, Jacek Anaszewski

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

On Mon 2021-05-10 12:50:19, Andy Shevchenko wrote:
> Replace -ENOTSUPP by -EOPNOTSUPP when returning from exported function.
> 
> Fixes: 13ae79bbe4c2 ("leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting")
> Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Are you sure this is real problem? This does not sound like an error
path that should happen.

BR,
								Pavel

>  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);
>  

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range()
  2021-05-10  9:50 ` [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range() Andy Shevchenko
@ 2021-05-28 10:04   ` Pavel Machek
  2021-05-28 10:45     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:04 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

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

On Mon 2021-05-10 12:50:20, Andy Shevchenko wrote:
> 1 microsecond with 20 millisecond parameter is too low margin for
> usleep_range(). Give 100 to make scheduler happier.
> 
> While at it, fix indentation in cases where EL_FW_DELAY_USEC is in use.
> In the loop, move it to the end to avoid a conditional.

Its not like unhappy schedulers are problem...
								Pavel
								
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe()
  2021-05-10  9:50 ` [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe() Andy Shevchenko
@ 2021-05-28 10:08   ` Pavel Machek
  2021-05-28 10:46     ` Andy Shevchenko
  2021-05-29  9:28     ` Andy Shevchenko
  0 siblings, 2 replies; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

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

Hi!

> @@ -734,10 +736,15 @@ static int sso_led_dt_parse(struct sso_led_priv *priv)
>  	if (fw_ssoled) {
>  		ret = __sso_led_dt_parse(priv, fw_ssoled);
>  		if (ret)
> -			return ret;
> +			goto err_child_out;
>  	}
>  
> +	fwnode_handle_put(fw_ssoled);
>  	return 0;
> +
> +err_child_out:
> +	fwnode_handle_put(fw_ssoled);
> +	return ret;
>  }

Just delete the return and you get the same effect, no? No need to
have two exits here.

BR,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups
  2021-05-10  9:50 ` [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups Andy Shevchenko
@ 2021-05-28 10:09   ` Pavel Machek
  2021-05-28 10:49     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:09 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

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

On Mon 2021-05-10 12:50:28, Andy Shevchenko wrote:
> The idea of managed resources that they will be cleaned up automatically
> and in the proper order. Remove explicit cleanups.

Are you really sure this is good idea with the regmap_update_bits in
between?

BR,
								Pavel

> ---
>  drivers/leds/blink/leds-lgm-sso.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/leds/blink/leds-lgm-sso.c b/drivers/leds/blink/leds-lgm-sso.c
> index e76be25480b4..a7f2e5436ba2 100644
> --- a/drivers/leds/blink/leds-lgm-sso.c
> +++ b/drivers/leds/blink/leds-lgm-sso.c
> @@ -606,16 +606,10 @@ static void sso_led_shutdown(struct sso_led *led)
>  {
>  	struct sso_led_priv *priv = led->priv;
>  
> -	/* unregister led */
> -	devm_led_classdev_unregister(priv->dev, &led->cdev);
> -
>  	/* clear HW control bit */
>  	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;
>  }
>  

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 21/28] leds: lm3697: Make error handling more robust
  2021-05-10  9:50 ` [PATCH v1 21/28] leds: lm3697: Make error handling more robust Andy Shevchenko
@ 2021-05-28 10:10   ` Pavel Machek
  2021-05-28 10:50     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:10 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

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

On Mon 2021-05-10 12:50:38, Andy Shevchenko wrote:
> 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 <andy.shevchenko@gmail.com>

You are now putting the handle even in the success case. Is that
right?
								Pavel

> ---
>  drivers/leds/leds-lm3697.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
> index 9d35dd2a9bf0..6262ae69591e 100644
> --- a/drivers/leds/leds-lm3697.c
> +++ b/drivers/leds/leds-lm3697.c
> @@ -224,14 +224,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;
>  		}
>  
> @@ -262,7 +260,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;
>  		}
>  
> @@ -287,7 +284,6 @@ 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;
>  		}
>  
> @@ -295,6 +291,7 @@ static int lm3697_probe_dt(struct lm3697 *priv)
>  	}
>  
>  child_out:
> +	fwnode_handle_put(child);
>  	return ret;
>  }
>  

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred
  2021-05-10  9:50 ` [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred Andy Shevchenko
@ 2021-05-28 10:11   ` Pavel Machek
  2021-05-28 10:47     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:11 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

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

On Mon 2021-05-10 12:50:26, Andy Shevchenko wrote:
> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> 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 <mallikarjunax.reddy@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>

From: does not match signed-off here.
							Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 28/28] leds: sgm3140: Put fwnode in any case during ->probe()
  2021-05-10  9:50 ` [PATCH v1 28/28] leds: sgm3140: " Andy Shevchenko
@ 2021-05-28 10:14   ` Pavel Machek
  2021-05-28 10:59     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 10:14 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel, Luca Weiss

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

Hi!

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

This (and similar) -- in half of the drivers we hold the handle from
successful probe. Is it a problem and why is it problem only for some
drivers?

Thanks for series, btw, I pushed out current version of the tree.

Best regards,
								Pavel

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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 02/28] leds: core: The -ENOTSUPP should never be seen by user space
  2021-05-28 10:03   ` Pavel Machek
@ 2021-05-28 10:43     ` Andy Shevchenko
  2021-05-29  9:42       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:43 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel, Jacek Anaszewski

On Fri, May 28, 2021 at 12:03:39PM +0200, Pavel Machek wrote:
> On Mon 2021-05-10 12:50:19, Andy Shevchenko wrote:
> > Replace -ENOTSUPP by -EOPNOTSUPP when returning from exported function.
> > 
> > Fixes: 13ae79bbe4c2 ("leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting")
> > Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
> > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Are you sure this is real problem?

(Potential) real problem, yes.

» This does not sound like an error
> path that should happen.

Before crafting this patch I have checked callers and _luckily_ they haven't
tested the returned code. But if any of the user decides to check -> real
problem.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range()
  2021-05-28 10:04   ` Pavel Machek
@ 2021-05-28 10:45     ` Andy Shevchenko
  2021-05-29  9:41       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:45 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 12:04:40PM +0200, Pavel Machek wrote:
> On Mon 2021-05-10 12:50:20, Andy Shevchenko wrote:
> > 1 microsecond with 20 millisecond parameter is too low margin for
> > usleep_range(). Give 100 to make scheduler happier.
> > 
> > While at it, fix indentation in cases where EL_FW_DELAY_USEC is in use.
> > In the loop, move it to the end to avoid a conditional.
> 
> Its not like unhappy schedulers are problem...

Any hints then? To me it sounds like torturing scheduler is the real problem
and that's why scheduler is unhappy.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe()
  2021-05-28 10:08   ` Pavel Machek
@ 2021-05-28 10:46     ` Andy Shevchenko
  2021-05-29  9:28     ` Andy Shevchenko
  1 sibling, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:46 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 12:08:00PM +0200, Pavel Machek wrote:
> > @@ -734,10 +736,15 @@ static int sso_led_dt_parse(struct sso_led_priv *priv)
> >  	if (fw_ssoled) {
> >  		ret = __sso_led_dt_parse(priv, fw_ssoled);
> >  		if (ret)
> > -			return ret;
> > +			goto err_child_out;
> >  	}
> >  
> > +	fwnode_handle_put(fw_ssoled);
> >  	return 0;
> > +
> > +err_child_out:
> > +	fwnode_handle_put(fw_ssoled);
> > +	return ret;
> >  }
> 
> Just delete the return and you get the same effect, no? No need to
> have two exits here.

I prefer to see it clear and follow the same pattern, but if you insist, I can
change in proposed way.

Thanks for review!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred
  2021-05-28 10:11   ` Pavel Machek
@ 2021-05-28 10:47     ` Andy Shevchenko
  2021-05-29  9:54       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:47 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 12:11:22PM +0200, Pavel Machek wrote:
> On Mon 2021-05-10 12:50:26, Andy Shevchenko wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > 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 <mallikarjunax.reddy@linux.intel.com>
> > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> From: does not match signed-off here.

True, I have noticed this later, because I have used existing commit as
template. I'll fix all such occurrences.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups
  2021-05-28 10:09   ` Pavel Machek
@ 2021-05-28 10:49     ` Andy Shevchenko
  2021-05-29  9:46       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:49 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 12:09:06PM +0200, Pavel Machek wrote:
> On Mon 2021-05-10 12:50:28, Andy Shevchenko wrote:
> > The idea of managed resources that they will be cleaned up automatically
> > and in the proper order. Remove explicit cleanups.
> 
> Are you really sure this is good idea with the regmap_update_bits in
> between?

Good question. I will check it.

> > -	/* unregister led */
> > -	devm_led_classdev_unregister(priv->dev, &led->cdev);
> > -
> >  	/* clear HW control bit */
> >  	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;


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 21/28] leds: lm3697: Make error handling more robust
  2021-05-28 10:10   ` Pavel Machek
@ 2021-05-28 10:50     ` Andy Shevchenko
  2021-05-29  9:50       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:50 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 12:10:57PM +0200, Pavel Machek wrote:
> On Mon 2021-05-10 12:50:38, Andy Shevchenko wrote:
> > 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 <andy.shevchenko@gmail.com>
> 
> You are now putting the handle even in the success case. Is that
> right?

Let's put it this way: it's no-op in successful case.

But yeah, I would prefer to have a separate case for error, I'll revisit this.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 28/28] leds: sgm3140: Put fwnode in any case during ->probe()
  2021-05-28 10:14   ` Pavel Machek
@ 2021-05-28 10:59     ` Andy Shevchenko
  2021-05-29  9:58       ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 10:59 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel, Luca Weiss

On Fri, May 28, 2021 at 12:14:54PM +0200, Pavel Machek wrote:
> Hi!
> 
> > fwnode_get_next_child_node() bumps a reference counting of a returned variable.
> > We have to balance it whenever we return to the caller.
> 
> This (and similar) -- in half of the drivers we hold the handle from
> successful probe. Is it a problem and why is it problem only for some
> drivers?

Hmm... I'm not sure I have understood the question correctly. Any examples of
the driver that you think needs some attention?

In general the idea is that these kind of for-loops or getting next fwnode
should be balanced.

In case of for-loops the error or any other breakage means that reference count
is bumped, for the get_next API it's always the case.

I have checked between drivers and only considered above cases. Wherever there
is a for-loop which isn't broken, we are fine. Wherever we have explicit
reference counter drop for get_next cases, we are fine. If (any) framework
requires the resource to be present that framework should bump and drop
reference count on the resource by itself (so I split LED framework out from
the consideration and consider that it does the right things)

> Thanks for series, btw, I pushed out current version of the tree.

Should I rebase the new version on something I can find in your Git tree?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-28 10:02   ` Pavel Machek
@ 2021-05-28 11:05     ` Andy Shevchenko
  2021-05-28 20:34       ` Pavel Machek
  0 siblings, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-28 11:05 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 12:02:54PM +0200, Pavel Machek wrote:
> On Mon 2021-05-17 10:30:08, Andy Shevchenko wrote:
> > On Mon, May 10, 2021 at 12:50:17PM +0300, Andy Shevchenko wrote:
> > > When analyzing the current state of affairs with fwnode reference counting
> > > I found that a lot of core doesn't take it right. Here is a bunch of
> > > corresponding fixes against LED drivers.
> > > 
> > > The series includes some cleanups and a few other fixes grouped by a driver.
> > > 
> > > First two patches are taking care of -ENOTSUPP error code too  prevent its
> > > appearance in the user space.
> > 
> > Pavel, any comments on this bug fix series?
> 
> I took these:

Thanks!

What branch/tree should I rebase the rest on?

> For the "remove depends on OF"... I'd preffer not to take those. We
> don't need to ask the user for configurations that never happen.

What do you mean by this? ACPI is quite a good configuration to make use
of it on the corresponding platforms. By default any discrete LED driver
(in hardware term here) IC should be considered independent from the type
of the platform description. Do you agree? If so, it means that dropping
OF dependency is a right thing to do to allow users of those ICs to be happy
even on ACPI based platforms.

Note, entire IIO subsystem is a good example of this activity. All the sensors
can be used now in ACPI environment without explicit requirement to have an
ACPI ID, although it's highly recommended to acquire for the real products
(not DIY ones).

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes
  2021-05-28 11:05     ` Andy Shevchenko
@ 2021-05-28 20:34       ` Pavel Machek
  0 siblings, 0 replies; 62+ messages in thread
From: Pavel Machek @ 2021-05-28 20:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

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

Hi!

> > > > First two patches are taking care of -ENOTSUPP error code too  prevent its
> > > > appearance in the user space.
> > > 
> > > Pavel, any comments on this bug fix series?
> > 
> > I took these:
> 
> Thanks!
> 
> What branch/tree should I rebase the rest on?

git@gitolite.kernel.org:pub/scm/linux/kernel/git/pavel/linux-leds.git
for-next would do the trick.

As would linux-next, I guess. This area should not be changing.

> > For the "remove depends on OF"... I'd preffer not to take those. We
> > don't need to ask the user for configurations that never happen.
> 
> What do you mean by this? ACPI is quite a good configuration to make use
> of it on the corresponding platforms. By default any discrete LED driver
> (in hardware term here) IC should be considered independent from the type
> of the platform description. Do you agree? If so, it means that

The drivers are independend, I guess. But I'm also very sure you will
not find some of the chips in a ACPI based machine. el15203000 is such
example.

I don't want people configuring for normal PCs to be asked if they
want el15203000 support.

If you know particular chip is present in ACPI-based machine, I'm okay
with removing the dependency.

(Maybe some of these chould depend on ARM || COMPILE_TEST instead?)

> > dropping
> OF dependency is a right thing to do to allow users of those ICs to be happy
> even on ACPI based platforms.
> 
> Note, entire IIO subsystem is a good example of this activity. All the sensors
> can be used now in ACPI environment without explicit requirement to have an
> ACPI ID, although it's highly recommended to acquire for the real products
> (not DIY ones).

Well. I'm not sure that is good step forward. It will result in
useless questions being asked.

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

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

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 04/28] leds: el15203000: Make error handling more robust
  2021-05-10  9:50 ` [PATCH v1 04/28] leds: el15203000: Make error handling more robust Andy Shevchenko
@ 2021-05-28 20:59   ` Oleh Kravchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Oleh Kravchenko @ 2021-05-28 20:59 UTC (permalink / raw)
  To: Andy Shevchenko, Pavel Machek, Andy Shevchenko,
	Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel



10.05.21 12:50, Andy Shevchenko пише:
> 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 <andy.shevchenko@gmail.com>
> ---
>  drivers/leds/leds-el15203000.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
> index 912451db05e6..bcdbbbc9c187 100644
> --- a/drivers/leds/leds-el15203000.c
> +++ b/drivers/leds/leds-el15203000.c
> @@ -246,16 +246,13 @@ static int el15203000_probe_dt(struct el15203000 *priv)
>  		ret = fwnode_property_read_u32(child, "reg", &led->reg);
>  		if (ret) {
>  			dev_err(priv->dev, "LED without ID number");
> -			fwnode_handle_put(child);
> -
> -			break;
> +			goto err_child_out;
>  		}
>  
>  		if (led->reg > U8_MAX) {
>  			dev_err(priv->dev, "LED value %d is invalid", led->reg);
> -			fwnode_handle_put(child);
> -
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto err_child_out;
>  		}
>  
>  		led->priv			  = priv;
> @@ -277,14 +274,16 @@ static int el15203000_probe_dt(struct el15203000 *priv)
>  			dev_err(priv->dev,
>  				"failed to register LED device %s, err %d",
>  				led->ldev.name, ret);
> -			fwnode_handle_put(child);
> -
> -			break;
> +			goto err_child_out;
>  		}
>  
>  		led++;
>  	}
>  
> +	return 0;
> +
> +err_child_out:
> +	fwnode_handle_put(child);
>  	return ret;
>  }
>  
> 

Reviewed-by: Oleh Kravchenko <oleg@kaa.org.ua>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h)
  2021-05-10  9:50 ` [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
@ 2021-05-28 21:00   ` Oleh Kravchenko
  2021-05-29  9:45     ` Andy Shevchenko
  0 siblings, 1 reply; 62+ messages in thread
From: Oleh Kravchenko @ 2021-05-28 21:00 UTC (permalink / raw)
  To: Andy Shevchenko, Pavel Machek, Andy Shevchenko,
	Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

10.05.21 12:50, Andy Shevchenko пише:
> There is no user of of*.h headers, but mod_devicetable.h.
> Update header block accordingly.
> 
> While at it, drop unneeded dependency to OF.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/leds/Kconfig           | 1 -
>  drivers/leds/leds-el15203000.c | 3 ++-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/leds/Kconfig b/drivers/leds/Kconfig
> index dcbfcd491fd0..531c79155717 100644
> --- a/drivers/leds/Kconfig
> +++ b/drivers/leds/Kconfig
> @@ -167,7 +167,6 @@ config LEDS_EL15203000
>  	tristate "LED Support for Crane EL15203000"
>  	depends on LEDS_CLASS
>  	depends on SPI
> -	depends on OF
>  	help
>  	  This option enables support for EL15203000 LED Board
>  	  (aka RED LED board) which is widely used in coffee vending
> diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
> index bcdbbbc9c187..fcb90d7cd42f 100644
> --- a/drivers/leds/leds-el15203000.c
> +++ b/drivers/leds/leds-el15203000.c
> @@ -4,8 +4,9 @@
>  
>  #include <linux/delay.h>
>  #include <linux/leds.h>
> +#include <linux/mod_devicetable.h>
>  #include <linux/module.h>
> -#include <linux/of_device.h>
> +#include <linux/property.h>
>  #include <linux/spi/spi.h>
>  
>  /*
> 

Reviewed-by: Oleh Kravchenko <oleg@kaa.org.ua>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 06/28] leds: el15203000: Introduce to_el15203000_led() helper
  2021-05-10  9:50 ` [PATCH v1 06/28] leds: el15203000: Introduce to_el15203000_led() helper Andy Shevchenko
@ 2021-05-28 21:01   ` Oleh Kravchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Oleh Kravchenko @ 2021-05-28 21:01 UTC (permalink / raw)
  To: Andy Shevchenko, Pavel Machek, Andy Shevchenko,
	Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

10.05.21 12:50, Andy Shevchenko пише:
> Introduce a helper to replace open coded container_of() calls.
> At the same time move ldev member to be first in the struct el15203000_led,
> that makes container_of() effectivelly a no-op.
> 
> Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> ---
>  drivers/leds/leds-el15203000.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
> index fcb90d7cd42f..e81a93d57210 100644
> --- a/drivers/leds/leds-el15203000.c
> +++ b/drivers/leds/leds-el15203000.c
> @@ -69,8 +69,8 @@ enum el15203000_command {
>  };
>  
>  struct el15203000_led {
> -	struct el15203000	*priv;
>  	struct led_classdev	ldev;
> +	struct el15203000	*priv;
>  	u32			reg;
>  };
>  
> @@ -83,6 +83,8 @@ struct el15203000 {
>  	struct el15203000_led	leds[];
>  };
>  
> +#define to_el15203000_led(d)	container_of(d, struct el15203000_led, ldev)
> +
>  static int el15203000_cmd(struct el15203000_led *led, u8 brightness)
>  {
>  	int		ret;
> @@ -124,9 +126,7 @@ static int el15203000_cmd(struct el15203000_led *led, u8 brightness)
>  static int el15203000_set_blocking(struct led_classdev *ldev,
>  				   enum led_brightness brightness)
>  {
> -	struct el15203000_led *led = container_of(ldev,
> -						  struct el15203000_led,
> -						  ldev);
> +	struct el15203000_led *led = to_el15203000_led(ldev);
>  
>  	return el15203000_cmd(led, brightness == LED_OFF ? EL_OFF : EL_ON);
>  }
> @@ -135,9 +135,7 @@ static int el15203000_pattern_set_S(struct led_classdev *ldev,
>  				    struct led_pattern *pattern,
>  				    u32 len, int repeat)
>  {
> -	struct el15203000_led *led = container_of(ldev,
> -						  struct el15203000_led,
> -						  ldev);
> +	struct el15203000_led *led = to_el15203000_led(ldev);
>  
>  	if (repeat > 0 || len != 2 ||
>  	    pattern[0].delta_t != 4000 || pattern[0].brightness != 0 ||
> @@ -188,10 +186,8 @@ static int el15203000_pattern_set_P(struct led_classdev *ldev,
>  				    struct led_pattern *pattern,
>  				    u32 len, int repeat)
>  {
> +	struct el15203000_led	*led = to_el15203000_led(ldev);
>  	u8			cmd;
> -	struct el15203000_led	*led = container_of(ldev,
> -						    struct el15203000_led,
> -						    ldev);
>  
>  	if (repeat > 0)
>  		return -EINVAL;
> @@ -228,9 +224,7 @@ static int el15203000_pattern_set_P(struct led_classdev *ldev,
>  
>  static int el15203000_pattern_clear(struct led_classdev *ldev)
>  {
> -	struct el15203000_led	*led = container_of(ldev,
> -						    struct el15203000_led,
> -						    ldev);
> +	struct el15203000_led *led = to_el15203000_led(ldev);
>  
>  	return el15203000_cmd(led, EL_OFF);
>  }
> 

Reviewed-by: Oleh Kravchenko <oleg@kaa.org.ua>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe()
  2021-05-28 10:08   ` Pavel Machek
  2021-05-28 10:46     ` Andy Shevchenko
@ 2021-05-29  9:28     ` Andy Shevchenko
  2021-05-29 10:46       ` Andy Shevchenko
  1 sibling, 1 reply; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:28 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

On Fri, May 28, 2021 at 1:08 PM Pavel Machek <pavel@ucw.cz> wrote:

> > @@ -734,10 +736,15 @@ static int sso_led_dt_parse(struct sso_led_priv *priv)
> >       if (fw_ssoled) {
> >               ret = __sso_led_dt_parse(priv, fw_ssoled);
> >               if (ret)
> > -                     return ret;
> > +                     goto err_child_out;
> >       }
> >
> > +     fwnode_handle_put(fw_ssoled);
> >       return 0;
> > +
> > +err_child_out:
> > +     fwnode_handle_put(fw_ssoled);
> > +     return ret;
> >  }
>
> Just delete the return and you get the same effect, no? No need to
> have two exits here.

Okay, I have tried and neither result is better:
option 1. Add ret = 0, but keep the label
option 2. Assign 0 to ret at the definition stage and replace return
with break and remove return 0 (I don't like that ret assigned to 0 in
the definition block. It usually may lead to subtle errors)
option 3+. Something I missed which you see can be done?

Which one do you prefer?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range()
  2021-05-28 10:45     ` Andy Shevchenko
@ 2021-05-29  9:41       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:41 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 1:46 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, May 28, 2021 at 12:04:40PM +0200, Pavel Machek wrote:
> > On Mon 2021-05-10 12:50:20, Andy Shevchenko wrote:
> > > 1 microsecond with 20 millisecond parameter is too low margin for
> > > usleep_range(). Give 100 to make scheduler happier.
> > >
> > > While at it, fix indentation in cases where EL_FW_DELAY_USEC is in use.
> > > In the loop, move it to the end to avoid a conditional.
> >
> > Its not like unhappy schedulers are problem...
>
> Any hints then? To me it sounds like torturing scheduler is the real problem
> and that's why scheduler is unhappy.

Okay, I have rephrased the commit message.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 02/28] leds: core: The -ENOTSUPP should never be seen by user space
  2021-05-28 10:43     ` Andy Shevchenko
@ 2021-05-29  9:42       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:42 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel, Jacek Anaszewski

On Fri, May 28, 2021 at 1:43 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, May 28, 2021 at 12:03:39PM +0200, Pavel Machek wrote:
> > On Mon 2021-05-10 12:50:19, Andy Shevchenko wrote:
> > > Replace -ENOTSUPP by -EOPNOTSUPP when returning from exported function.
> > >
> > > Fixes: 13ae79bbe4c2 ("leds: core: Drivers shouldn't enforce SYNC/ASYNC brightness setting")
> > > Cc: Jacek Anaszewski <j.anaszewski@samsung.com>
> > > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >
> > Are you sure this is real problem?
>
> (Potential) real problem, yes.
>
> » This does not sound like an error
> > path that should happen.
>
> Before crafting this patch I have checked callers and _luckily_ they haven't
> tested the returned code. But if any of the user decides to check -> real
> problem.

I have rephrased the commit message to point out the above.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h)
  2021-05-28 21:00   ` Oleh Kravchenko
@ 2021-05-29  9:45     ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:45 UTC (permalink / raw)
  To: Oleh Kravchenko
  Cc: Pavel Machek, Andy Shevchenko, Amireddy Mallikarjuna reddy,
	Linus Walleij, Marek Behún, Abanoub Sameh, Dan Murphy,
	Krzysztof Kozlowski, linux-leds, linux-kernel

On Sat, May 29, 2021 at 12:00 AM Oleh Kravchenko <oleg@kaa.org.ua> wrote:
> 10.05.21 12:50, Andy Shevchenko пише:
> > There is no user of of*.h headers, but mod_devicetable.h.
> > Update header block accordingly.

> > While at it, drop unneeded dependency to OF.

> >       depends on LEDS_CLASS
> >       depends on SPI
> > -     depends on OF

> Reviewed-by: Oleh Kravchenko <oleg@kaa.org.ua>

Thanks! I have dropped the OF dependency removal, while keeping your Rb.
I think we may figure out later what is the best course of action regarding it.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups
  2021-05-28 10:49     ` Andy Shevchenko
@ 2021-05-29  9:46       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:46 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 1:50 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, May 28, 2021 at 12:09:06PM +0200, Pavel Machek wrote:
> > On Mon 2021-05-10 12:50:28, Andy Shevchenko wrote:
> > > The idea of managed resources that they will be cleaned up automatically
> > > and in the proper order. Remove explicit cleanups.
> >
> > Are you really sure this is good idea with the regmap_update_bits in
> > between?
>
> Good question. I will check it.
>
> > > -   /* unregister led */
> > > -   devm_led_classdev_unregister(priv->dev, &led->cdev);

I left this untouched, i.o.w. no removal in v2.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 21/28] leds: lm3697: Make error handling more robust
  2021-05-28 10:50     ` Andy Shevchenko
@ 2021-05-29  9:50       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:50 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 1:51 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, May 28, 2021 at 12:10:57PM +0200, Pavel Machek wrote:
> > On Mon 2021-05-10 12:50:38, Andy Shevchenko wrote:
> > > 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 <andy.shevchenko@gmail.com>
> >
> > You are now putting the handle even in the success case. Is that
> > right?
>
> Let's put it this way: it's no-op in successful case.
>
> But yeah, I would prefer to have a separate case for error, I'll revisit this.

I have added return 0; for a successful case.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred
  2021-05-28 10:47     ` Andy Shevchenko
@ 2021-05-29  9:54       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:54 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel

On Fri, May 28, 2021 at 1:50 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, May 28, 2021 at 12:11:22PM +0200, Pavel Machek wrote:
> > On Mon 2021-05-10 12:50:26, Andy Shevchenko wrote:
> > > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > >
> > > 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 <mallikarjunax.reddy@linux.intel.com>
> > > Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >
> > From: does not match signed-off here.
>
> True, I have noticed this later, because I have used existing commit as
> template. I'll fix all such occurrences.

Fixed in v2.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 28/28] leds: sgm3140: Put fwnode in any case during ->probe()
  2021-05-28 10:59     ` Andy Shevchenko
@ 2021-05-29  9:58       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29  9:58 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Amireddy Mallikarjuna reddy, Linus Walleij, Marek Behún,
	Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski, linux-leds,
	linux-kernel, Luca Weiss

On Fri, May 28, 2021 at 2:01 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Fri, May 28, 2021 at 12:14:54PM +0200, Pavel Machek wrote:
> > Hi!
> >
> > > fwnode_get_next_child_node() bumps a reference counting of a returned variable.
> > > We have to balance it whenever we return to the caller.
> >
> > This (and similar) -- in half of the drivers we hold the handle from
> > successful probe. Is it a problem and why is it problem only for some
> > drivers?
>
> Hmm... I'm not sure I have understood the question correctly. Any examples of
> the driver that you think needs some attention?
>
> In general the idea is that these kind of for-loops or getting next fwnode
> should be balanced.
>
> In case of for-loops the error or any other breakage means that reference count
> is bumped, for the get_next API it's always the case.
>
> I have checked between drivers and only considered above cases. Wherever there
> is a for-loop which isn't broken, we are fine. Wherever we have explicit
> reference counter drop for get_next cases, we are fine. If (any) framework
> requires the resource to be present that framework should bump and drop
> reference count on the resource by itself (so I split LED framework out from
> the consideration and consider that it does the right things)
>
> > Thanks for series, btw, I pushed out current version of the tree.
>
> Should I rebase the new version on something I can find in your Git tree?

I found the above is good justification, so I leave those patches
unchanged in v2.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe()
  2021-05-29  9:28     ` Andy Shevchenko
@ 2021-05-29 10:46       ` Andy Shevchenko
  0 siblings, 0 replies; 62+ messages in thread
From: Andy Shevchenko @ 2021-05-29 10:46 UTC (permalink / raw)
  To: Pavel Machek
  Cc: Andy Shevchenko, Amireddy Mallikarjuna reddy, Linus Walleij,
	Marek Behún, Abanoub Sameh, Dan Murphy, Krzysztof Kozlowski,
	linux-leds, linux-kernel

On Sat, May 29, 2021 at 12:28 PM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, May 28, 2021 at 1:08 PM Pavel Machek <pavel@ucw.cz> wrote:

> > > @@ -734,10 +736,15 @@ static int sso_led_dt_parse(struct sso_led_priv *priv)
> > >       if (fw_ssoled) {
> > >               ret = __sso_led_dt_parse(priv, fw_ssoled);
> > >               if (ret)
> > > -                     return ret;
> > > +                     goto err_child_out;
> > >       }
> > >
> > > +     fwnode_handle_put(fw_ssoled);
> > >       return 0;
> > > +
> > > +err_child_out:
> > > +     fwnode_handle_put(fw_ssoled);
> > > +     return ret;
> > >  }
> >
> > Just delete the return and you get the same effect, no? No need to
> > have two exits here.
>
> Okay, I have tried and neither result is better:
> option 1. Add ret = 0, but keep the label
> option 2. Assign 0 to ret at the definition stage and replace return
> with break and remove return 0 (I don't like that ret assigned to 0 in
> the definition block. It usually may lead to subtle errors)
> option 3+. Something I missed which you see can be done?
>
> Which one do you prefer?

I found option 3 which is better and follows your suggestion (I suppose).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2021-05-29 10:46 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-10  9:50 [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 01/28] leds: class: The -ENOTSUPP should never be seen by user space Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 02/28] leds: core: " Andy Shevchenko
2021-05-28 10:03   ` Pavel Machek
2021-05-28 10:43     ` Andy Shevchenko
2021-05-29  9:42       ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 03/28] leds: el15203000: Give better margin for usleep_range() Andy Shevchenko
2021-05-28 10:04   ` Pavel Machek
2021-05-28 10:45     ` Andy Shevchenko
2021-05-29  9:41       ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 04/28] leds: el15203000: Make error handling more robust Andy Shevchenko
2021-05-28 20:59   ` Oleh Kravchenko
2021-05-10  9:50 ` [PATCH v1 05/28] leds: el15203000: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
2021-05-28 21:00   ` Oleh Kravchenko
2021-05-29  9:45     ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 06/28] leds: el15203000: Introduce to_el15203000_led() helper Andy Shevchenko
2021-05-28 21:01   ` Oleh Kravchenko
2021-05-10  9:50 ` [PATCH v1 07/28] leds: lgm-sso: Fix clock handling Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 08/28] leds: lgm-sso: Put fwnode in any case during ->probe() Andy Shevchenko
2021-05-28 10:08   ` Pavel Machek
2021-05-28 10:46     ` Andy Shevchenko
2021-05-29  9:28     ` Andy Shevchenko
2021-05-29 10:46       ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 09/28] leds: lgm-sso: Don't spam logs when probe is deferred Andy Shevchenko
2021-05-28 10:11   ` Pavel Machek
2021-05-28 10:47     ` Andy Shevchenko
2021-05-29  9:54       ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 10/28] leds: lgm-sso: Remove unneeded of_match_ptr() Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 11/28] leds: lgm-sso: Remove explicit managed resource cleanups Andy Shevchenko
2021-05-28 10:09   ` Pavel Machek
2021-05-28 10:49     ` Andy Shevchenko
2021-05-29  9:46       ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 12/28] leds: lgm-sso: Drop duplicate NULL check for GPIO operations Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 13/28] leds: lgm-sso: Convert to use list_for_each_entry*() API Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 14/28] leds: lm3532: select regmap I2C API Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 15/28] leds: lm3532: Make error handling more robust Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 16/28] leds: lm36274: Put fwnode in error case during ->probe() Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 17/28] leds: lm36274: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 18/28] leds: lm3692x: Put fwnode in any case during ->probe() Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 19/28] leds: lm3692x: Correct headers (of*.h -> mod_devicetable.h) Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 20/28] leds: lm3697: Update header block to reflect reality Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 21/28] leds: lm3697: Make error handling more robust Andy Shevchenko
2021-05-28 10:10   ` Pavel Machek
2021-05-28 10:50     ` Andy Shevchenko
2021-05-29  9:50       ` Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 22/28] leds: lm3697: Don't spam logs when probe is deferred Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 23/28] leds: lp50xx: Put fwnode in error case during ->probe() Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 24/28] leds: lt3593: Put fwnode in any " Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 25/28] leds: lt3593: Make use of device properties Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 26/28] leds: pwm: Make error handling more robust Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 27/28] leds: rt8515: Put fwnode in any case during ->probe() Andy Shevchenko
2021-05-10  9:50 ` [PATCH v1 28/28] leds: sgm3140: " Andy Shevchenko
2021-05-28 10:14   ` Pavel Machek
2021-05-28 10:59     ` Andy Shevchenko
2021-05-29  9:58       ` Andy Shevchenko
2021-05-17  7:30 ` [PATCH v1 00/28] leds: cleanups and fwnode refcounting bug fixes Andy Shevchenko
2021-05-24 14:56   ` Andy Shevchenko
2021-05-24 17:49     ` Pavel Machek
2021-05-24 18:39       ` Andy Shevchenko
2021-05-28 10:02   ` Pavel Machek
2021-05-28 11:05     ` Andy Shevchenko
2021-05-28 20:34       ` Pavel Machek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.