[PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers)

[PATCH leds v3 0/9] Start moving parsing of `linux,default-trigger` to LED core (a cleanup of LED drivers)

[Marek Behún]

Hi Pavel,

this is v3, on top of your current for-next branch.

Changes:
- lm36274 again with the use-after-free issue you mentioned solved
- lm3532 does not need to parse `label` since it uses init_data
- leds-syscon can also use init_data by simple change
- the last patch, which moves parsing of `linux,default-trigger`, is now
  compatible with current version of the patches

Marek

Marek Behún (9):
  leds: lm36274: cosmetic: rename lm36274_data to chip
  leds: lm36274: don't iterate through children since there is only one
  leds: lm36274: use struct led_init_data when registering
  leds: lm36274: do not set chip settings in DT parsing function
  leds: lm36274: use platform device as parent of LED
  leds: lm36274: use devres LED registering function
  leds: lm3532: don't parse label DT property
  leds: syscon: use struct led_init_data when registering
  leds: parse linux,default-trigger DT property in LED core

 drivers/leds/led-class.c         |   5 ++
 drivers/leds/leds-an30259a.c     |   3 -
 drivers/leds/leds-aw2013.c       |   3 -
 drivers/leds/leds-bcm6328.c      |   4 -
 drivers/leds/leds-bcm6358.c      |   4 -
 drivers/leds/leds-cr0014114.c    |   3 -
 drivers/leds/leds-el15203000.c   |   3 -
 drivers/leds/leds-gpio.c         |   3 -
 drivers/leds/leds-is31fl32xx.c   |   3 -
 drivers/leds/leds-lm3532.c       |  15 ----
 drivers/leds/leds-lm36274.c      | 128 ++++++++++++++-----------------
 drivers/leds/leds-lm3692x.c      |   3 -
 drivers/leds/leds-lm3697.c       |   3 -
 drivers/leds/leds-lp50xx.c       |   3 -
 drivers/leds/leds-lp8860.c       |   4 -
 drivers/leds/leds-lt3593.c       |   3 -
 drivers/leds/leds-max77650.c     |   3 -
 drivers/leds/leds-mt6323.c       |   4 -
 drivers/leds/leds-ns2.c          |   3 -
 drivers/leds/leds-pm8058.c       |   2 -
 drivers/leds/leds-pwm.c          |   5 --
 drivers/leds/leds-syscon.c       |   9 +--
 drivers/leds/leds-tlc591xx.c     |   3 -
 drivers/leds/leds-turris-omnia.c |   2 -
 24 files changed, 68 insertions(+), 153 deletions(-)


base-commit: a0e550dc351ab5fabe8ea86e45b974494a0a6bf8
-- 
2.26.2

[PATCH leds v3 1/9] leds: lm36274: cosmetic: rename lm36274_data to chip

[Marek Behún]

Rename this variable so that it is easier to read and easier to write in
80 columns. Also rename variable of this type in lm36274_brightness_set
from led to chip, to be consistent.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm36274.c | 82 ++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index bfeee03a0053c..4a9f786bb9727 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -41,37 +41,36 @@ struct lm36274 {
 };
 
 static int lm36274_brightness_set(struct led_classdev *led_cdev,
-				enum led_brightness brt_val)
+				  enum led_brightness brt_val)
 {
-	struct lm36274 *led = container_of(led_cdev, struct lm36274, led_dev);
+	struct lm36274 *chip = container_of(led_cdev, struct lm36274, led_dev);
 
-	return ti_lmu_common_set_brightness(&led->lmu_data, brt_val);
+	return ti_lmu_common_set_brightness(&chip->lmu_data, brt_val);
 }
 
-static int lm36274_init(struct lm36274 *lm36274_data)
+static int lm36274_init(struct lm36274 *chip)
 {
 	int enable_val = 0;
 	int i;
 
-	for (i = 0; i < lm36274_data->num_leds; i++)
-		enable_val |= (1 << lm36274_data->led_sources[i]);
+	for (i = 0; i < chip->num_leds; i++)
+		enable_val |= (1 << chip->led_sources[i]);
 
 	if (!enable_val) {
-		dev_err(lm36274_data->dev, "No LEDs were enabled\n");
+		dev_err(chip->dev, "No LEDs were enabled\n");
 		return -EINVAL;
 	}
 
 	enable_val |= LM36274_BL_EN;
 
-	return regmap_write(lm36274_data->regmap, LM36274_REG_BL_EN,
-			    enable_val);
+	return regmap_write(chip->regmap, LM36274_REG_BL_EN, enable_val);
 }
 
-static int lm36274_parse_dt(struct lm36274 *lm36274_data)
+static int lm36274_parse_dt(struct lm36274 *chip)
 {
 	struct fwnode_handle *child = NULL;
 	char label[LED_MAX_NAME_SIZE];
-	struct device *dev = &lm36274_data->pdev->dev;
+	struct device *dev = &chip->pdev->dev;
 	const char *name;
 	int child_cnt;
 	int ret = -EINVAL;
@@ -84,37 +83,37 @@ static int lm36274_parse_dt(struct lm36274 *lm36274_data)
 	device_for_each_child_node(dev, child) {
 		ret = fwnode_property_read_string(child, "label", &name);
 		if (ret)
-			snprintf(label, sizeof(label),
-				"%s::", lm36274_data->pdev->name);
+			snprintf(label, sizeof(label), "%s::",
+				 chip->pdev->name);
 		else
-			snprintf(label, sizeof(label),
-				 "%s:%s", lm36274_data->pdev->name, name);
+			snprintf(label, sizeof(label), "%s:%s",
+				 chip->pdev->name, name);
 
-		lm36274_data->num_leds = fwnode_property_count_u32(child, "led-sources");
-		if (lm36274_data->num_leds <= 0)
+		chip->num_leds = fwnode_property_count_u32(child, "led-sources");
+		if (chip->num_leds <= 0)
 			return -ENODEV;
 
 		ret = fwnode_property_read_u32_array(child, "led-sources",
-						     lm36274_data->led_sources,
-						     lm36274_data->num_leds);
+						     chip->led_sources,
+						     chip->num_leds);
 		if (ret) {
 			dev_err(dev, "led-sources property missing\n");
 			return ret;
 		}
 
 		fwnode_property_read_string(child, "linux,default-trigger",
-					&lm36274_data->led_dev.default_trigger);
+					    &chip->led_dev.default_trigger);
 
 	}
 
-	lm36274_data->lmu_data.regmap = lm36274_data->regmap;
-	lm36274_data->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
-	lm36274_data->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
-	lm36274_data->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+	chip->lmu_data.regmap = chip->regmap;
+	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+	chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
 
-	lm36274_data->led_dev.name = label;
-	lm36274_data->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
-	lm36274_data->led_dev.brightness_set_blocking = lm36274_brightness_set;
+	chip->led_dev.name = label;
+	chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+	chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
 
 	return 0;
 }
@@ -122,39 +121,38 @@ static int lm36274_parse_dt(struct lm36274 *lm36274_data)
 static int lm36274_probe(struct platform_device *pdev)
 {
 	struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
-	struct lm36274 *lm36274_data;
+	struct lm36274 *chip;
 	int ret;
 
-	lm36274_data = devm_kzalloc(&pdev->dev, sizeof(*lm36274_data),
-				    GFP_KERNEL);
-	if (!lm36274_data)
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
 		return -ENOMEM;
 
-	lm36274_data->pdev = pdev;
-	lm36274_data->dev = lmu->dev;
-	lm36274_data->regmap = lmu->regmap;
-	platform_set_drvdata(pdev, lm36274_data);
+	chip->pdev = pdev;
+	chip->dev = lmu->dev;
+	chip->regmap = lmu->regmap;
+	platform_set_drvdata(pdev, chip);
 
-	ret = lm36274_parse_dt(lm36274_data);
+	ret = lm36274_parse_dt(chip);
 	if (ret) {
-		dev_err(lm36274_data->dev, "Failed to parse DT node\n");
+		dev_err(chip->dev, "Failed to parse DT node\n");
 		return ret;
 	}
 
-	ret = lm36274_init(lm36274_data);
+	ret = lm36274_init(chip);
 	if (ret) {
-		dev_err(lm36274_data->dev, "Failed to init the device\n");
+		dev_err(chip->dev, "Failed to init the device\n");
 		return ret;
 	}
 
-	return led_classdev_register(lm36274_data->dev, &lm36274_data->led_dev);
+	return led_classdev_register(chip->dev, &chip->led_dev);
 }
 
 static int lm36274_remove(struct platform_device *pdev)
 {
-	struct lm36274 *lm36274_data = platform_get_drvdata(pdev);
+	struct lm36274 *chip = platform_get_drvdata(pdev);
 
-	led_classdev_unregister(&lm36274_data->led_dev);
+	led_classdev_unregister(&chip->led_dev);
 
 	return 0;
 }
-- 
2.26.2

[PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one

[Marek Behún]

Do not use device_for_each_child_node. Since this driver works only with
once child node present, use device_get_next_child_node instead.
This also saves one level of indentation.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm36274.c | 50 +++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 4a9f786bb9727..e0fce74a76675 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -72,40 +72,36 @@ static int lm36274_parse_dt(struct lm36274 *chip)
 	char label[LED_MAX_NAME_SIZE];
 	struct device *dev = &chip->pdev->dev;
 	const char *name;
-	int child_cnt;
-	int ret = -EINVAL;
+	int ret;
 
 	/* There should only be 1 node */
-	child_cnt = device_get_child_node_count(dev);
-	if (child_cnt != 1)
+	if (device_get_child_node_count(dev) != 1)
 		return -EINVAL;
 
-	device_for_each_child_node(dev, child) {
-		ret = fwnode_property_read_string(child, "label", &name);
-		if (ret)
-			snprintf(label, sizeof(label), "%s::",
-				 chip->pdev->name);
-		else
-			snprintf(label, sizeof(label), "%s:%s",
-				 chip->pdev->name, name);
-
-		chip->num_leds = fwnode_property_count_u32(child, "led-sources");
-		if (chip->num_leds <= 0)
-			return -ENODEV;
-
-		ret = fwnode_property_read_u32_array(child, "led-sources",
-						     chip->led_sources,
-						     chip->num_leds);
-		if (ret) {
-			dev_err(dev, "led-sources property missing\n");
-			return ret;
-		}
-
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &chip->led_dev.default_trigger);
+	child = device_get_next_child_node(dev, NULL);
+
+	ret = fwnode_property_read_string(child, "label", &name);
+	if (ret)
+		snprintf(label, sizeof(label), "%s::", chip->pdev->name);
+	else
+		snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
 
+	chip->num_leds = fwnode_property_count_u32(child, "led-sources");
+	if (chip->num_leds <= 0)
+		return -ENODEV;
+
+	ret = fwnode_property_read_u32_array(child, "led-sources",
+					     chip->led_sources, chip->num_leds);
+	if (ret) {
+		dev_err(dev, "led-sources property missing\n");
+		return ret;
 	}
 
+	fwnode_property_read_string(child, "linux,default-trigger",
+				    &chip->led_dev.default_trigger);
+
+	fwnode_handle_put(child);
+
 	chip->lmu_data.regmap = chip->regmap;
 	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
 	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
-- 
2.26.2

[PATCH leds v3 3/9] leds: lm36274: use struct led_init_data when registering

[Marek Behún]

By using struct led_init_data when registering we do not need to parse
`label` DT property. Moreover `label` is deprecated and if it is not
present but `color` and `function` are, LED core will compose a name
from these properties instead.

Previously if the `label` DT property was not present, the code composed
name for the LED in the form
  "parent_name::"
For backwards compatibility we therefore set
  init_data->default_label = ":";
so that the LED will not get a different name if `label` property is not
present, nor are `color` and `function`.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm36274.c | 42 ++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index e0fce74a76675..ee4f90f04f195 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -66,12 +66,11 @@ static int lm36274_init(struct lm36274 *chip)
 	return regmap_write(chip->regmap, LM36274_REG_BL_EN, enable_val);
 }
 
-static int lm36274_parse_dt(struct lm36274 *chip)
+static int lm36274_parse_dt(struct lm36274 *chip,
+			    struct led_init_data *init_data)
 {
-	struct fwnode_handle *child = NULL;
-	char label[LED_MAX_NAME_SIZE];
 	struct device *dev = &chip->pdev->dev;
-	const char *name;
+	struct fwnode_handle *child;
 	int ret;
 
 	/* There should only be 1 node */
@@ -80,43 +79,45 @@ static int lm36274_parse_dt(struct lm36274 *chip)
 
 	child = device_get_next_child_node(dev, NULL);
 
-	ret = fwnode_property_read_string(child, "label", &name);
-	if (ret)
-		snprintf(label, sizeof(label), "%s::", chip->pdev->name);
-	else
-		snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
+	init_data->fwnode = child;
+	init_data->devicename = chip->pdev->name;
+	/* for backwards compatibility when `label` property is not present */
+	init_data->default_label = ":";
 
 	chip->num_leds = fwnode_property_count_u32(child, "led-sources");
-	if (chip->num_leds <= 0)
-		return -ENODEV;
+	if (chip->num_leds <= 0) {
+		ret = -ENODEV;
+		goto err;
+	}
 
 	ret = fwnode_property_read_u32_array(child, "led-sources",
 					     chip->led_sources, chip->num_leds);
 	if (ret) {
 		dev_err(dev, "led-sources property missing\n");
-		return ret;
+		goto err;
 	}
 
 	fwnode_property_read_string(child, "linux,default-trigger",
 				    &chip->led_dev.default_trigger);
 
-	fwnode_handle_put(child);
-
 	chip->lmu_data.regmap = chip->regmap;
 	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
 	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
 	chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
 
-	chip->led_dev.name = label;
 	chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
 	chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
 
 	return 0;
+err:
+	fwnode_handle_put(child);
+	return ret;
 }
 
 static int lm36274_probe(struct platform_device *pdev)
 {
 	struct ti_lmu *lmu = dev_get_drvdata(pdev->dev.parent);
+	struct led_init_data init_data = {};
 	struct lm36274 *chip;
 	int ret;
 
@@ -129,7 +130,7 @@ static int lm36274_probe(struct platform_device *pdev)
 	chip->regmap = lmu->regmap;
 	platform_set_drvdata(pdev, chip);
 
-	ret = lm36274_parse_dt(chip);
+	ret = lm36274_parse_dt(chip, &init_data);
 	if (ret) {
 		dev_err(chip->dev, "Failed to parse DT node\n");
 		return ret;
@@ -141,7 +142,14 @@ static int lm36274_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	return led_classdev_register(chip->dev, &chip->led_dev);
+	ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data);
+	if (ret)
+		dev_err(chip->dev, "Failed to register LED for node %pfw\n",
+			init_data.fwnode);
+
+	fwnode_handle_put(init_data.fwnode);
+
+	return ret;
 }
 
 static int lm36274_remove(struct platform_device *pdev)
-- 
2.26.2

[PATCH leds v3 4/9] leds: lm36274: do not set chip settings in DT parsing function

[Marek Behún]

These settings are not parsed from DT and therefore semantically should
not be set in function with a name lm36274_parse_dt.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm36274.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index ee4f90f04f195..85a58a5cbdf9c 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -100,14 +100,6 @@ static int lm36274_parse_dt(struct lm36274 *chip,
 	fwnode_property_read_string(child, "linux,default-trigger",
 				    &chip->led_dev.default_trigger);
 
-	chip->lmu_data.regmap = chip->regmap;
-	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
-	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
-	chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
-
-	chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
-	chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
-
 	return 0;
 err:
 	fwnode_handle_put(child);
@@ -142,6 +134,14 @@ static int lm36274_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	chip->lmu_data.regmap = chip->regmap;
+	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
+	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;
+	chip->lmu_data.lsb_brightness_reg = LM36274_REG_BRT_LSB;
+
+	chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
+	chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
+
 	ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data);
 	if (ret)
 		dev_err(chip->dev, "Failed to register LED for node %pfw\n",
-- 
2.26.2

[PATCH leds v3 5/9] leds: lm36274: use platform device as parent of LED

[Marek Behún]

Instead of registering LED under the MFD device, this driver sets the
parent of the LED it is registering to the parent of the MFD device (the
I2C client device).

Because of this we cannot use devres for LED registration, since it can
result in use-after-free, see commit
a0972fff0947 ("leds: lm36274: fix use-after-free on unbind").

The only other in-tree driver that also registers under the MFD device
(drivers/regulator/lm363x-regulator.c) sets the parent to the MFD
device.

Set the parent of this LED to the MFD device, instead of the I2C client
device.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm36274.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 85a58a5cbdf9c..74c236d1a60c8 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -69,7 +69,7 @@ static int lm36274_init(struct lm36274 *chip)
 static int lm36274_parse_dt(struct lm36274 *chip,
 			    struct led_init_data *init_data)
 {
-	struct device *dev = &chip->pdev->dev;
+	struct device *dev = chip->dev;
 	struct fwnode_handle *child;
 	int ret;
 
@@ -118,7 +118,7 @@ static int lm36274_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	chip->pdev = pdev;
-	chip->dev = lmu->dev;
+	chip->dev = &pdev->dev;
 	chip->regmap = lmu->regmap;
 	platform_set_drvdata(pdev, chip);
 
-- 
2.26.2

[PATCH leds v3 6/9] leds: lm36274: use devres LED registering function

[Marek Behún]

Now that the potential use-after-free issue is resolved we can use
devres for LED registration in this driver.

By using devres version of LED registering function we can remove the
.remove method from this driver.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Dan Murphy <dmurphy@ti.com>
---
 drivers/leds/leds-lm36274.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 74c236d1a60c8..10a63b7f2ecce 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -142,7 +142,8 @@ static int lm36274_probe(struct platform_device *pdev)
 	chip->led_dev.max_brightness = MAX_BRIGHTNESS_11BIT;
 	chip->led_dev.brightness_set_blocking = lm36274_brightness_set;
 
-	ret = led_classdev_register_ext(chip->dev, &chip->led_dev, &init_data);
+	ret = devm_led_classdev_register_ext(chip->dev, &chip->led_dev,
+					     &init_data);
 	if (ret)
 		dev_err(chip->dev, "Failed to register LED for node %pfw\n",
 			init_data.fwnode);
@@ -152,15 +153,6 @@ static int lm36274_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int lm36274_remove(struct platform_device *pdev)
-{
-	struct lm36274 *chip = platform_get_drvdata(pdev);
-
-	led_classdev_unregister(&chip->led_dev);
-
-	return 0;
-}
-
 static const struct of_device_id of_lm36274_leds_match[] = {
 	{ .compatible = "ti,lm36274-backlight", },
 	{},
@@ -169,7 +161,6 @@ MODULE_DEVICE_TABLE(of, of_lm36274_leds_match);
 
 static struct platform_driver lm36274_driver = {
 	.probe  = lm36274_probe,
-	.remove = lm36274_remove,
 	.driver = {
 		.name = "lm36274-leds",
 	},
-- 
2.26.2

[PATCH leds v3 7/9] leds: lm3532: don't parse label DT property

[Marek Behún]

This driver uses extended LED registration, so we do not need to parse
the `label` DT property on our own.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/leds/leds-lm3532.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 946ad67eaecb7..9b6973217cc0b 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -129,7 +129,6 @@ struct lm3532_als_data {
  * @full_scale_current - The full-scale current setting for the current sink.
  * @led_strings - The LED strings supported in this array
  * @enabled - Enabled status
- * @label - LED label
  */
 struct lm3532_led {
 	struct led_classdev led_dev;
@@ -142,7 +141,6 @@ struct lm3532_led {
 	int full_scale_current;
 	unsigned int enabled:1;
 	u32 led_strings[LM3532_MAX_CONTROL_BANKS];
-	char label[LED_MAX_NAME_SIZE];
 };
 
 /**
@@ -548,7 +546,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 {
 	struct fwnode_handle *child = NULL;
 	struct lm3532_led *led;
-	const char *name;
 	int control_bank;
 	u32 ramp_time;
 	size_t i = 0;
@@ -646,16 +643,7 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 		fwnode_property_read_string(child, "linux,default-trigger",
 					    &led->led_dev.default_trigger);
 
-		ret = fwnode_property_read_string(child, "label", &name);
-		if (ret)
-			snprintf(led->label, sizeof(led->label),
-				"%s::", priv->client->name);
-		else
-			snprintf(led->label, sizeof(led->label),
-				 "%s:%s", priv->client->name, name);
-
 		led->priv = priv;
-		led->led_dev.name = led->label;
 		led->led_dev.brightness_set_blocking = lm3532_brightness_set;
 
 		ret = devm_led_classdev_register_ext(priv->dev, &led->led_dev, &idata);
-- 
2.26.2

[PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering

[Marek Behún]

By using struct led_init_data when registering we do not need to parse
`label` DT property. Moreover `label` is deprecated and if it is not
present but `color` and `function` are, LED core will compose a name
from these properties instead.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/leds/leds-syscon.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
index d7230da815430..f54935fa650a7 100644
--- a/drivers/leds/leds-syscon.c
+++ b/drivers/leds/leds-syscon.c
@@ -55,6 +55,7 @@ static void syscon_led_set(struct led_classdev *led_cdev,
 
 static int syscon_led_probe(struct platform_device *pdev)
 {
+	struct led_init_data init_data = {};
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev_of_node(dev);
 	struct device *parent;
@@ -84,8 +85,6 @@ static int syscon_led_probe(struct platform_device *pdev)
 		return -EINVAL;
 	if (of_property_read_u32(np, "mask", &sled->mask))
 		return -EINVAL;
-	sled->cdev.name =
-		of_get_property(np, "label", NULL) ? : np->name;
 	sled->cdev.default_trigger =
 		of_get_property(np, "linux,default-trigger", NULL);
 
@@ -115,7 +114,9 @@ static int syscon_led_probe(struct platform_device *pdev)
 	}
 	sled->cdev.brightness_set = syscon_led_set;
 
-	ret = devm_led_classdev_register(dev, &sled->cdev);
+	init_data.fwnode = of_fwnode_handle(np);
+
+	ret = devm_led_classdev_register_ext(dev, &sled->cdev, &init_data);
 	if (ret < 0)
 		return ret;
 
-- 
2.26.2

[PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core

[Marek Behún]

Do the parsing of `linux,default-trigger` DT property to LED core.
Currently it is done in many different drivers and the code is repeated.

This patch removes the parsing from 23 drivers:
  an30259a, aw2013, bcm6328, bcm6358, cr0014114, el15203000, gpio,
  is31fl32xx, lm3532, lm36274, lm3692x, lm3697, lp50xx, lp8860, lt3593,
  max77650, mt6323, ns2, pm8058, pwm, syscon, tlc591xx and turris-omnia.

There is one driver in drivers/input which parses this property on it's
own. I shall send a separate patch there after this is applied.

There are still 8 drivers that parse this property on their own because
they do not pass the led_init_data structure to the registering
function. I will try to refactor those in the future.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/leds/led-class.c         | 5 +++++
 drivers/leds/leds-an30259a.c     | 3 ---
 drivers/leds/leds-aw2013.c       | 3 ---
 drivers/leds/leds-bcm6328.c      | 4 ----
 drivers/leds/leds-bcm6358.c      | 4 ----
 drivers/leds/leds-cr0014114.c    | 3 ---
 drivers/leds/leds-el15203000.c   | 3 ---
 drivers/leds/leds-gpio.c         | 3 ---
 drivers/leds/leds-is31fl32xx.c   | 3 ---
 drivers/leds/leds-lm3532.c       | 3 ---
 drivers/leds/leds-lm36274.c      | 3 ---
 drivers/leds/leds-lm3692x.c      | 3 ---
 drivers/leds/leds-lm3697.c       | 3 ---
 drivers/leds/leds-lp50xx.c       | 3 ---
 drivers/leds/leds-lp8860.c       | 4 ----
 drivers/leds/leds-lt3593.c       | 3 ---
 drivers/leds/leds-max77650.c     | 3 ---
 drivers/leds/leds-mt6323.c       | 4 ----
 drivers/leds/leds-ns2.c          | 3 ---
 drivers/leds/leds-pm8058.c       | 2 --
 drivers/leds/leds-pwm.c          | 5 -----
 drivers/leds/leds-syscon.c       | 2 --
 drivers/leds/leds-tlc591xx.c     | 3 ---
 drivers/leds/leds-turris-omnia.c | 2 --
 24 files changed, 5 insertions(+), 72 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index cc3929f858b68..131ca83f5fb38 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -354,6 +354,11 @@ int led_classdev_register_ext(struct device *parent,
 		ret = led_compose_name(parent, init_data, composed_name);
 		if (ret < 0)
 			return ret;
+
+		if (init_data->fwnode)
+			fwnode_property_read_string(init_data->fwnode,
+				"linux,default-trigger",
+				&led_cdev->default_trigger);
 	} else {
 		proposed_name = led_cdev->name;
 	}
diff --git a/drivers/leds/leds-an30259a.c b/drivers/leds/leds-an30259a.c
index 9749f1cc3e15f..a0df1fb28774d 100644
--- a/drivers/leds/leds-an30259a.c
+++ b/drivers/leds/leds-an30259a.c
@@ -238,9 +238,6 @@ static int an30259a_dt_init(struct i2c_client *client,
 				led->default_state = STATE_OFF;
 		}
 
-		of_property_read_string(child, "linux,default-trigger",
-					&led->cdev.default_trigger);
-
 		i++;
 	}
 
diff --git a/drivers/leds/leds-aw2013.c b/drivers/leds/leds-aw2013.c
index 927c5ba32592f..80d937454aeef 100644
--- a/drivers/leds/leds-aw2013.c
+++ b/drivers/leds/leds-aw2013.c
@@ -297,9 +297,6 @@ static int aw2013_probe_dt(struct aw2013 *chip)
 				 "DT property led-max-microamp is missing\n");
 		}
 
-		of_property_read_string(child, "linux,default-trigger",
-					&led->cdev.default_trigger);
-
 		led->cdev.brightness_set_blocking = aw2013_brightness_set;
 		led->cdev.blink_set = aw2013_blink_set;
 
diff --git a/drivers/leds/leds-bcm6328.c b/drivers/leds/leds-bcm6328.c
index d79aeb956c9b6..226d17d253ed3 100644
--- a/drivers/leds/leds-bcm6328.c
+++ b/drivers/leds/leds-bcm6328.c
@@ -346,10 +346,6 @@ static int bcm6328_led(struct device *dev, struct device_node *nc, u32 reg,
 	if (of_property_read_bool(nc, "active-low"))
 		led->active_low = true;
 
-	led->cdev.default_trigger = of_get_property(nc,
-						    "linux,default-trigger",
-						    NULL);
-
 	if (!of_property_read_string(nc, "default-state", &state)) {
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
diff --git a/drivers/leds/leds-bcm6358.c b/drivers/leds/leds-bcm6358.c
index 0fd495103a4d9..9d2e487fa08a3 100644
--- a/drivers/leds/leds-bcm6358.c
+++ b/drivers/leds/leds-bcm6358.c
@@ -110,10 +110,6 @@ static int bcm6358_led(struct device *dev, struct device_node *nc, u32 reg,
 	if (of_property_read_bool(nc, "active-low"))
 		led->active_low = true;
 
-	led->cdev.default_trigger = of_get_property(nc,
-						    "linux,default-trigger",
-						    NULL);
-
 	if (!of_property_read_string(nc, "default-state", &state)) {
 		if (!strcmp(state, "on")) {
 			led->cdev.brightness = LED_FULL;
diff --git a/drivers/leds/leds-cr0014114.c b/drivers/leds/leds-cr0014114.c
index 2da448ae718e9..d03cfd3c0bfbe 100644
--- a/drivers/leds/leds-cr0014114.c
+++ b/drivers/leds/leds-cr0014114.c
@@ -188,9 +188,6 @@ static int cr0014114_probe_dt(struct cr0014114 *priv)
 	device_for_each_child_node(priv->dev, child) {
 		led = &priv->leds[i];
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led->ldev.default_trigger);
-
 		led->priv			  = priv;
 		led->ldev.max_brightness	  = CR_MAX_BRIGHTNESS;
 		led->ldev.brightness_set_blocking = cr0014114_set_sync;
diff --git a/drivers/leds/leds-el15203000.c b/drivers/leds/leds-el15203000.c
index 298b13e4807a8..6ca47f2a20041 100644
--- a/drivers/leds/leds-el15203000.c
+++ b/drivers/leds/leds-el15203000.c
@@ -263,9 +263,6 @@ static int el15203000_probe_dt(struct el15203000 *priv)
 			return -EINVAL;
 		}
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led->ldev.default_trigger);
-
 		led->priv			  = priv;
 		led->ldev.max_brightness	  = LED_ON;
 		led->ldev.brightness_set_blocking = el15203000_set_blocking;
diff --git a/drivers/leds/leds-gpio.c b/drivers/leds/leds-gpio.c
index cf84096d88cec..93f5b1b60fdec 100644
--- a/drivers/leds/leds-gpio.c
+++ b/drivers/leds/leds-gpio.c
@@ -160,9 +160,6 @@ static struct gpio_leds_priv *gpio_leds_create(struct platform_device *pdev)
 
 		led_dat->gpiod = led.gpiod;
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led.default_trigger);
-
 		if (!fwnode_property_read_string(child, "default-state",
 						 &state)) {
 			if (!strcmp(state, "keep"))
diff --git a/drivers/leds/leds-is31fl32xx.c b/drivers/leds/leds-is31fl32xx.c
index ea589d1a89540..2180255ad3393 100644
--- a/drivers/leds/leds-is31fl32xx.c
+++ b/drivers/leds/leds-is31fl32xx.c
@@ -341,9 +341,6 @@ static int is31fl32xx_parse_child_dt(const struct device *dev,
 	}
 	led_data->channel = reg;
 
-	of_property_read_string(child, "linux,default-trigger",
-				&cdev->default_trigger);
-
 	cdev->brightness_set_blocking = is31fl32xx_brightness_set;
 
 	return 0;
diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
index 9b6973217cc0b..10ded9192fb13 100644
--- a/drivers/leds/leds-lm3532.c
+++ b/drivers/leds/leds-lm3532.c
@@ -640,9 +640,6 @@ static int lm3532_parse_node(struct lm3532_data *priv)
 			goto child_out;
 		}
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led->led_dev.default_trigger);
-
 		led->priv = priv;
 		led->led_dev.brightness_set_blocking = lm3532_brightness_set;
 
diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
index 10a63b7f2ecce..582c6a821dc85 100644
--- a/drivers/leds/leds-lm36274.c
+++ b/drivers/leds/leds-lm36274.c
@@ -97,9 +97,6 @@ static int lm36274_parse_dt(struct lm36274 *chip,
 		goto err;
 	}
 
-	fwnode_property_read_string(child, "linux,default-trigger",
-				    &chip->led_dev.default_trigger);
-
 	return 0;
 err:
 	fwnode_handle_put(child);
diff --git a/drivers/leds/leds-lm3692x.c b/drivers/leds/leds-lm3692x.c
index 1d7ea1b76a125..e945de45388ca 100644
--- a/drivers/leds/leds-lm3692x.c
+++ b/drivers/leds/leds-lm3692x.c
@@ -433,9 +433,6 @@ static int lm3692x_probe_dt(struct lm3692x_led *led)
 		return -ENODEV;
 	}
 
-	fwnode_property_read_string(child, "linux,default-trigger",
-				    &led->led_dev.default_trigger);
-
 	ret = fwnode_property_read_u32(child, "reg", &led->led_enable);
 	if (ret) {
 		dev_err(&led->client->dev, "reg DT property missing\n");
diff --git a/drivers/leds/leds-lm3697.c b/drivers/leds/leds-lm3697.c
index a7779ce931ab3..64c0794801e6d 100644
--- a/drivers/leds/leds-lm3697.c
+++ b/drivers/leds/leds-lm3697.c
@@ -268,9 +268,6 @@ static int lm3697_probe_dt(struct lm3697 *priv)
 		if (ret)
 			dev_warn(dev, "runtime-ramp properties missing\n");
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led->led_dev.default_trigger);
-
 		init_data.fwnode = child;
 		init_data.devicename = priv->client->name;
 		/* for backwards compatibility if `label` is not present */
diff --git a/drivers/leds/leds-lp50xx.c b/drivers/leds/leds-lp50xx.c
index 47144a37cb945..5fb4f24aeb2e8 100644
--- a/drivers/leds/leds-lp50xx.c
+++ b/drivers/leds/leds-lp50xx.c
@@ -508,9 +508,6 @@ static int lp50xx_probe_dt(struct lp50xx *priv)
 		led_cdev = &led->mc_cdev.led_cdev;
 		led_cdev->brightness_set_blocking = lp50xx_brightness_set;
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led_cdev->default_trigger);
-
 		ret = devm_led_classdev_multicolor_register_ext(&priv->client->dev,
 						       &led->mc_cdev,
 						       &init_data);
diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index e9b16f3358d6f..f0533a337bc15 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -392,10 +392,6 @@ static int lp8860_probe(struct i2c_client *client,
 	if (!child_node)
 		return -EINVAL;
 
-	led->led_dev.default_trigger = of_get_property(child_node,
-					    "linux,default-trigger",
-					    NULL);
-
 	led->enable_gpio = devm_gpiod_get_optional(&client->dev,
 						   "enable", GPIOD_OUT_LOW);
 	if (IS_ERR(led->enable_gpio)) {
diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index 061f02e3995ae..68e06434ac087 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -86,9 +86,6 @@ static int lt3593_led_probe(struct platform_device *pdev)
 
 	child = device_get_next_child_node(dev, NULL);
 
-	fwnode_property_read_string(child, "linux,default-trigger",
-				    &led_data->cdev.default_trigger);
-
 	if (!fwnode_property_read_string(child, "default-state", &tmp)) {
 		if (!strcmp(tmp, "on"))
 			state = LEDS_GPIO_DEFSTATE_ON;
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
index acc1174197c35..1eeac56b00146 100644
--- a/drivers/leds/leds-max77650.c
+++ b/drivers/leds/leds-max77650.c
@@ -100,9 +100,6 @@ static int max77650_led_probe(struct platform_device *pdev)
 		led->cdev.brightness_set_blocking = max77650_led_brightness_set;
 		led->cdev.max_brightness = MAX77650_LED_MAX_BRIGHTNESS;
 
-		fwnode_property_read_string(child, "linux,default-trigger",
-					    &led->cdev.default_trigger);
-
 		init_data.fwnode = child;
 		init_data.devicename = "max77650";
 		/* for backwards compatibility if `label` is not present */
diff --git a/drivers/leds/leds-mt6323.c b/drivers/leds/leds-mt6323.c
index a1fceeb41d7e6..f59e0e8bda8bb 100644
--- a/drivers/leds/leds-mt6323.c
+++ b/drivers/leds/leds-mt6323.c
@@ -342,10 +342,6 @@ static int mt6323_led_set_dt_default(struct led_classdev *cdev,
 	const char *state;
 	int ret = 0;
 
-	led->cdev.default_trigger = of_get_property(np,
-						    "linux,default-trigger",
-						    NULL);
-
 	state = of_get_property(np, "default-state", NULL);
 	if (state) {
 		if (!strcmp(state, "keep")) {
diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 030f426af3d75..e1ec5cbed07e8 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -185,9 +185,6 @@ static int ns2_led_register(struct device *dev, struct device_node *np,
 	if (IS_ERR(led->slow))
 		return PTR_ERR(led->slow);
 
-	of_property_read_string(np, "linux,default-trigger",
-				&led->cdev.default_trigger);
-
 	ret = of_property_count_u32_elems(np, "modes-map");
 	if (ret < 0 || ret % 3) {
 		dev_err(dev, "Missing or malformed modes-map for %pOF\n", np);
diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c
index 5247b04abb7ef..fb2ab72c0c40c 100644
--- a/drivers/leds/leds-pm8058.c
+++ b/drivers/leds/leds-pm8058.c
@@ -117,8 +117,6 @@ static int pm8058_led_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	led->cdev.default_trigger =
-		of_get_property(np, "linux,default-trigger", NULL);
 	led->cdev.brightness_set = pm8058_led_set;
 	led->cdev.brightness_get = pm8058_led_get;
 	if (led->ledtype == PM8058_LED_TYPE_COMMON)
diff --git a/drivers/leds/leds-pwm.c b/drivers/leds/leds-pwm.c
index 4a014d2964123..2a16ae0bf022b 100644
--- a/drivers/leds/leds-pwm.c
+++ b/drivers/leds/leds-pwm.c
@@ -20,7 +20,6 @@
 
 struct led_pwm {
 	const char	*name;
-	const char	*default_trigger;
 	u8		active_low;
 	unsigned int	max_brightness;
 };
@@ -70,7 +69,6 @@ static int led_pwm_add(struct device *dev, struct led_pwm_priv *priv,
 
 	led_data->active_low = led->active_low;
 	led_data->cdev.name = led->name;
-	led_data->cdev.default_trigger = led->default_trigger;
 	led_data->cdev.brightness = LED_OFF;
 	led_data->cdev.max_brightness = led->max_brightness;
 	led_data->cdev.flags = LED_CORE_SUSPENDRESUME;
@@ -124,9 +122,6 @@ static int led_pwm_create_fwnode(struct device *dev, struct led_pwm_priv *priv)
 			return -EINVAL;
 		}
 
-		fwnode_property_read_string(fwnode, "linux,default-trigger",
-					    &led.default_trigger);
-
 		led.active_low = fwnode_property_read_bool(fwnode,
 							   "active-low");
 		fwnode_property_read_u32(fwnode, "max-brightness",
diff --git a/drivers/leds/leds-syscon.c b/drivers/leds/leds-syscon.c
index f54935fa650a7..7eddb8ecb44ec 100644
--- a/drivers/leds/leds-syscon.c
+++ b/drivers/leds/leds-syscon.c
@@ -85,8 +85,6 @@ static int syscon_led_probe(struct platform_device *pdev)
 		return -EINVAL;
 	if (of_property_read_u32(np, "mask", &sled->mask))
 		return -EINVAL;
-	sled->cdev.default_trigger =
-		of_get_property(np, "linux,default-trigger", NULL);
 
 	state = of_get_property(np, "default-state", NULL);
 	if (state) {
diff --git a/drivers/leds/leds-tlc591xx.c b/drivers/leds/leds-tlc591xx.c
index 5e84a0c7aacbd..f24271337bd84 100644
--- a/drivers/leds/leds-tlc591xx.c
+++ b/drivers/leds/leds-tlc591xx.c
@@ -199,9 +199,6 @@ tlc591xx_probe(struct i2c_client *client,
 		led = &priv->leds[reg];
 
 		led->active = true;
-		led->ldev.default_trigger =
-			of_get_property(child, "linux,default-trigger", NULL);
-
 		led->priv = priv;
 		led->led_no = reg;
 		led->ldev.brightness_set_blocking = tlc591xx_brightness_set;
diff --git a/drivers/leds/leds-turris-omnia.c b/drivers/leds/leds-turris-omnia.c
index 117976cf75c8a..8c5bdc3847ee7 100644
--- a/drivers/leds/leds-turris-omnia.c
+++ b/drivers/leds/leds-turris-omnia.c
@@ -121,8 +121,6 @@ static int omnia_led_register(struct i2c_client *client, struct omnia_led *led,
 	cdev->max_brightness = 255;
 	cdev->brightness_set_blocking = omnia_led_brightness_set_blocking;
 
-	of_property_read_string(np, "linux,default-trigger", &cdev->default_trigger);
-
 	/* put the LED into software mode */
 	ret = i2c_smbus_write_byte_data(client, CMD_LED_MODE,
 					CMD_LED_MODE_LED(led->reg) |
-- 
2.26.2

Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function

[Pavel Machek]

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

Hi!

> Now that the potential use-after-free issue is resolved we can use
> devres for LED registration in this driver.
> 
> By using devres version of LED registering function we can remove the
> .remove method from this driver.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Dan Murphy <dmurphy@ti.com>

AFAICT this one is buggy, I sent explanation before. Why are you
resubmitting it?

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

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

Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function

[Marek Behun]

On Sun, 20 Sep 2020 23:45:32 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > Now that the potential use-after-free issue is resolved we can use
> > devres for LED registration in this driver.
> > 
> > By using devres version of LED registering function we can remove the
> > .remove method from this driver.
> > 
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > Cc: Dan Murphy <dmurphy@ti.com>  
> 
> AFAICT this one is buggy, I sent explanation before. Why are you
> resubmitting it?
> 
> 								Pavel

The previous patch in this series (v3 5/9) should solve this issue and
th commit message explains how.

Marek

Re: [PATCH leds v3 1/9] leds: lm36274: cosmetic: rename lm36274_data to chip

[Dan Murphy]

Marek

On 9/19/20 1:02 PM, Marek Behún wrote:
> Rename this variable so that it is easier to read and easier to write in
> 80 columns. Also rename variable of this type in lm36274_brightness_set
> from led to chip, to be consistent.

This patch seems a bit unnecessary.  The current variables fit fine with 
80 columns.

Dan

Re: [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one

[Dan Murphy]

Hello

On 9/19/20 1:02 PM, Marek Behún wrote:
> Do not use device_for_each_child_node. Since this driver works only with
> once child node present, use device_get_next_child_node instead.
> This also saves one level of indentation.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Dan Murphy <dmurphy@ti.com>
> ---
>   drivers/leds/leds-lm36274.c | 50 +++++++++++++++++--------------------
>   1 file changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
> index 4a9f786bb9727..e0fce74a76675 100644
> --- a/drivers/leds/leds-lm36274.c
> +++ b/drivers/leds/leds-lm36274.c
> @@ -72,40 +72,36 @@ static int lm36274_parse_dt(struct lm36274 *chip)
>   	char label[LED_MAX_NAME_SIZE];
>   	struct device *dev = &chip->pdev->dev;
>   	const char *name;
> -	int child_cnt;
> -	int ret = -EINVAL;
> +	int ret;
>   
>   	/* There should only be 1 node */
> -	child_cnt = device_get_child_node_count(dev);
> -	if (child_cnt != 1)
> +	if (device_get_child_node_count(dev) != 1)
>   		return -EINVAL;
>   
> -	device_for_each_child_node(dev, child) {
> -		ret = fwnode_property_read_string(child, "label", &name);
> -		if (ret)
> -			snprintf(label, sizeof(label), "%s::",
> -				 chip->pdev->name);
> -		else
> -			snprintf(label, sizeof(label), "%s:%s",
> -				 chip->pdev->name, name);
> -
> -		chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> -		if (chip->num_leds <= 0)
> -			return -ENODEV;
> -
> -		ret = fwnode_property_read_u32_array(child, "led-sources",
> -						     chip->led_sources,
> -						     chip->num_leds);
> -		if (ret) {
> -			dev_err(dev, "led-sources property missing\n");
> -			return ret;
> -		}
> -
> -		fwnode_property_read_string(child, "linux,default-trigger",
> -					    &chip->led_dev.default_trigger);
> +	child = device_get_next_child_node(dev, NULL);
> +
> +	ret = fwnode_property_read_string(child, "label", &name);
> +	if (ret)
> +		snprintf(label, sizeof(label), "%s::", chip->pdev->name);
> +	else
> +		snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
>   
> +	chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> +	if (chip->num_leds <= 0)
> +		return -ENODEV;
> +
> +	ret = fwnode_property_read_u32_array(child, "led-sources",
> +					     chip->led_sources, chip->num_leds);
> +	if (ret) {
> +		dev_err(dev, "led-sources property missing\n");
> +		return ret;
>   	}
>   
> +	fwnode_property_read_string(child, "linux,default-trigger",
> +				    &chip->led_dev.default_trigger);
> +
> +	fwnode_handle_put(child);
> +
>   	chip->lmu_data.regmap = chip->regmap;
>   	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
>   	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;

Question is this device on a piece of hardware you are testing on?

Just wondering how you functionally tested all these changes you submitted

Reviewed-by: Dan Murphy <dmurphy@ti.com>

Re: [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one

[Marek Behun]

On Tue, 22 Sep 2020 10:42:49 -0500
Dan Murphy <dmurphy@ti.com> wrote:

> Hello
> 
> On 9/19/20 1:02 PM, Marek Behún wrote:
> > Do not use device_for_each_child_node. Since this driver works only with
> > once child node present, use device_get_next_child_node instead.
> > This also saves one level of indentation.
> >
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > Cc: Dan Murphy <dmurphy@ti.com>
> > ---
> >   drivers/leds/leds-lm36274.c | 50 +++++++++++++++++--------------------
> >   1 file changed, 23 insertions(+), 27 deletions(-)
> >
> > diff --git a/drivers/leds/leds-lm36274.c b/drivers/leds/leds-lm36274.c
> > index 4a9f786bb9727..e0fce74a76675 100644
> > --- a/drivers/leds/leds-lm36274.c
> > +++ b/drivers/leds/leds-lm36274.c
> > @@ -72,40 +72,36 @@ static int lm36274_parse_dt(struct lm36274 *chip)
> >   	char label[LED_MAX_NAME_SIZE];
> >   	struct device *dev = &chip->pdev->dev;
> >   	const char *name;
> > -	int child_cnt;
> > -	int ret = -EINVAL;
> > +	int ret;
> >   
> >   	/* There should only be 1 node */
> > -	child_cnt = device_get_child_node_count(dev);
> > -	if (child_cnt != 1)
> > +	if (device_get_child_node_count(dev) != 1)
> >   		return -EINVAL;
> >   
> > -	device_for_each_child_node(dev, child) {
> > -		ret = fwnode_property_read_string(child, "label", &name);
> > -		if (ret)
> > -			snprintf(label, sizeof(label), "%s::",
> > -				 chip->pdev->name);
> > -		else
> > -			snprintf(label, sizeof(label), "%s:%s",
> > -				 chip->pdev->name, name);
> > -
> > -		chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> > -		if (chip->num_leds <= 0)
> > -			return -ENODEV;
> > -
> > -		ret = fwnode_property_read_u32_array(child, "led-sources",
> > -						     chip->led_sources,
> > -						     chip->num_leds);
> > -		if (ret) {
> > -			dev_err(dev, "led-sources property missing\n");
> > -			return ret;
> > -		}
> > -
> > -		fwnode_property_read_string(child, "linux,default-trigger",
> > -					    &chip->led_dev.default_trigger);
> > +	child = device_get_next_child_node(dev, NULL);
> > +
> > +	ret = fwnode_property_read_string(child, "label", &name);
> > +	if (ret)
> > +		snprintf(label, sizeof(label), "%s::", chip->pdev->name);
> > +	else
> > +		snprintf(label, sizeof(label), "%s:%s", chip->pdev->name, name);
> >   
> > +	chip->num_leds = fwnode_property_count_u32(child, "led-sources");
> > +	if (chip->num_leds <= 0)
> > +		return -ENODEV;
> > +
> > +	ret = fwnode_property_read_u32_array(child, "led-sources",
> > +					     chip->led_sources, chip->num_leds);
> > +	if (ret) {
> > +		dev_err(dev, "led-sources property missing\n");
> > +		return ret;
> >   	}
> >   
> > +	fwnode_property_read_string(child, "linux,default-trigger",
> > +				    &chip->led_dev.default_trigger);
> > +
> > +	fwnode_handle_put(child);
> > +
> >   	chip->lmu_data.regmap = chip->regmap;
> >   	chip->lmu_data.max_brightness = MAX_BRIGHTNESS_11BIT;
> >   	chip->lmu_data.msb_brightness_reg = LM36274_REG_BRT_MSB;  
> 
> Question is this device on a piece of hardware you are testing on?

No, unfortunately. But this driver is rather simple, in comparison to
the others.

As Linus said:
  "If it compiles, it is good; if it boots up, it is perfect."
:D

So if someone tested it, it would be perfect.

Marek

> Just wondering how you functionally tested all these changes you submitted
> 
> Reviewed-by: Dan Murphy <dmurphy@ti.com>
> 

Re: [PATCH leds v3 1/9] leds: lm36274: cosmetic: rename lm36274_data to chip

[Pavel Machek]

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

On Tue 2020-09-22 10:39:34, Dan Murphy wrote:
> Marek
> 
> On 9/19/20 1:02 PM, Marek Behún wrote:
> > Rename this variable so that it is easier to read and easier to write in
> > 80 columns. Also rename variable of this type in lm36274_brightness_set
> > from led to chip, to be consistent.
> 
> This patch seems a bit unnecessary.  The current variables fit fine with 80
> columns.

I like this patch... result is easier to read.

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

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

Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function

[Pavel Machek]

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

On Sun 2020-09-20 23:54:36, Marek Behun wrote:
> On Sun, 20 Sep 2020 23:45:32 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> > Hi!
> > 
> > > Now that the potential use-after-free issue is resolved we can use
> > > devres for LED registration in this driver.
> > > 
> > > By using devres version of LED registering function we can remove the
> > > .remove method from this driver.
> > > 
> > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > Cc: Dan Murphy <dmurphy@ti.com>  
> > 
> > AFAICT this one is buggy, I sent explanation before. Why are you
> > resubmitting it?
> 
> The previous patch in this series (v3 5/9) should solve this issue and
> th commit message explains how.

Aha, let me see.

Will 5/9 have some side-effects, like device appearing at different
place in sysfs?

First few patches look ok, but it would be really nice someone tested
complete sereies.

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

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

Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function

[Marek Behun]

On Tue, 22 Sep 2020 18:38:42 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> On Sun 2020-09-20 23:54:36, Marek Behun wrote:
> > On Sun, 20 Sep 2020 23:45:32 +0200
> > Pavel Machek <pavel@ucw.cz> wrote:
> >   
> > > Hi!
> > >   
> > > > Now that the potential use-after-free issue is resolved we can use
> > > > devres for LED registration in this driver.
> > > > 
> > > > By using devres version of LED registering function we can remove the
> > > > .remove method from this driver.
> > > > 
> > > > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > > > Cc: Dan Murphy <dmurphy@ti.com>    
> > > 
> > > AFAICT this one is buggy, I sent explanation before. Why are you
> > > resubmitting it?  
> > 
> > The previous patch in this series (v3 5/9) should solve this issue and
> > th commit message explains how.  
> 
> Aha, let me see.
> 
> Will 5/9 have some side-effects, like device appearing at different
> place in sysfs?

Yes, unfortunately. Before this path the led should be in
 /sys/devices/.../i2c-client/leds/led
or somthing like that, and after
 /sys/devices/..c/i2c-client/mfd/leds/led

But it should have been this way from beginning, I think. The other
driver, regulator, registers its device under the mfd device.

The question is whether this will break something for someone. I don't
think so, but...

> 
> First few patches look ok, but it would be really nice someone tested
> complete sereies.
> 
> Best regards,
> 									Pavel

Re: [PATCH leds v3 6/9] leds: lm36274: use devres LED registering function

[Dan Murphy]

Pavel

On 9/22/20 11:58 AM, Marek Behun wrote:
> On Tue, 22 Sep 2020 18:38:42 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
>
>> On Sun 2020-09-20 23:54:36, Marek Behun wrote:
>>> On Sun, 20 Sep 2020 23:45:32 +0200
>>> Pavel Machek <pavel@ucw.cz> wrote:
>>>    
>>>> Hi!
>>>>    
>>>>> Now that the potential use-after-free issue is resolved we can use
>>>>> devres for LED registration in this driver.
>>>>>
>>>>> By using devres version of LED registering function we can remove the
>>>>> .remove method from this driver.
>>>>>
>>>>> Signed-off-by: Marek Behún <marek.behun@nic.cz>
>>>>> Cc: Dan Murphy <dmurphy@ti.com>
>>>> AFAICT this one is buggy, I sent explanation before. Why are you
>>>> resubmitting it?
>>> The previous patch in this series (v3 5/9) should solve this issue and
>>> th commit message explains how.
>> Aha, let me see.
>>
>> Will 5/9 have some side-effects, like device appearing at different
>> place in sysfs?
> Yes, unfortunately. Before this path the led should be in
>   /sys/devices/.../i2c-client/leds/led
> or somthing like that, and after
>   /sys/devices/..c/i2c-client/mfd/leds/led
>
> But it should have been this way from beginning, I think. The other
> driver, regulator, registers its device under the mfd device.
>
> The question is whether this will break something for someone. I don't
> think so, but...
>
>> First few patches look ok, but it would be really nice someone tested
>> complete sereies.

For the LM36274 patches

Tested-by: Dan Murphy <dmurphy@ti.com>

Also found some other legacy issues I sent patches for.

Dan

Re: [PATCH leds v3 2/9] leds: lm36274: don't iterate through children since there is only one

[Dan Murphy]

Marek

On 9/22/20 10:58 AM, Marek Behun wrote:
> On Tue, 22 Sep 2020 10:42:49 -0500
> Dan Murphy <dmurphy@ti.com> wrote:
>
>> <snip>
>> Question is this device on a piece of hardware you are testing on?
> No, unfortunately. But this driver is rather simple, in comparison to
> the others.
>
> As Linus said:
>    "If it compiles, it is good; if it boots up, it is perfect."
> :D
>
> So if someone tested it, it would be perfect.

Not sure how a comment made in 1998 applies to the state of the kernel 
today.

With this much change to the driver there should have been some level of 
functional testing.

So I pulled out my hardware and gave it a whirl. Gave my TB on the 
LM36274 patches.

Dan

Re: [PATCH leds v3 3/9] leds: lm36274: use struct led_init_data when registering

[Pavel Machek]

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

Hi!

> By using struct led_init_data when registering we do not need to parse
> `label` DT property. Moreover `label` is deprecated and if it is not
> present but `color` and `function` are, LED core will compose a name
> from these properties instead.
> 
> Previously if the `label` DT property was not present, the code composed
> name for the LED in the form
>   "parent_name::"
> For backwards compatibility we therefore set
>   init_data->default_label = ":";
> so that the LED will not get a different name if `label` property is not
> present, nor are `color` and `function`.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Dan Murphy <dmurphy@ti.com>

I applied 1-3 of this series.

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

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

Re: [PATCH leds v3 9/9] leds: parse linux,default-trigger DT property in LED core

[Pavel Machek]

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

Hi!

> Do the parsing of `linux,default-trigger` DT property to LED core.
> Currently it is done in many different drivers and the code is repeated.
> 
> This patch removes the parsing from 23 drivers:
>   an30259a, aw2013, bcm6328, bcm6358, cr0014114, el15203000, gpio,
>   is31fl32xx, lm3532, lm36274, lm3692x, lm3697, lp50xx, lp8860, lt3593,
>   max77650, mt6323, ns2, pm8058, pwm, syscon, tlc591xx and turris-omnia.
> 
> There is one driver in drivers/input which parses this property on it's
> own. I shall send a separate patch there after this is applied.
> 
> There are still 8 drivers that parse this property on their own because
> they do not pass the led_init_data structure to the registering
> function. I will try to refactor those in the future.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Thanks, I applied the series.

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

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

Re: [PATCH leds v3 8/9] leds: syscon: use struct led_init_data when registering

[Linus Walleij]

On Sat, Sep 19, 2020 at 8:03 PM Marek Behún <marek.behun@nic.cz> wrote:

> By using struct led_init_data when registering we do not need to parse
> `label` DT property. Moreover `label` is deprecated and if it is not
> present but `color` and `function` are, LED core will compose a name
> from these properties instead.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Linus Walleij <linus.walleij@linaro.org>

Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij