Re: [PATCH 11/25] leds: lp8860: Use led_compose_name()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dan,

On 3/11/19 1:28 PM, Dan Murphy wrote:
On 3/10/19 1:28 PM, Jacek Anaszewski wrote:
Switch to using generic LED support for composing LED class
device name.

While at it, avoid iterating through available child of nodes
in favor of obtaining single expected child node using single
call to of_get_next_available_child().

Signed-off-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>
Cc: Dan Murphy <dmurphy@xxxxxx>
---
  drivers/leds/leds-lp8860.c | 38 +++++++++++++++++++-------------------
  1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/leds/leds-lp8860.c b/drivers/leds/leds-lp8860.c
index 39c72a908f3b..7c12ccdc1f2f 100644
--- a/drivers/leds/leds-lp8860.c
+++ b/drivers/leds/leds-lp8860.c
@@ -22,7 +22,6 @@
  #include <linux/of_gpio.h>
  #include <linux/gpio/consumer.h>
  #include <linux/slab.h>
-#include <uapi/linux/uleds.h>
#define LP8860_DISP_CL1_BRT_MSB 0x00
  #define LP8860_DISP_CL1_BRT_LSB		0x01
@@ -87,6 +86,8 @@
#define LP8860_CLEAR_FAULTS 0x01 +#define LP8860_NAME "lp8860"
+
  /**
   * struct lp8860_led -
   * @lock - Lock for reading/writing the device
@@ -96,7 +97,6 @@
   * @eeprom_regmap - EEPROM register map
   * @enable_gpio - VDDIO/EN gpio to enable communication interface
   * @regulator - LED supply regulator pointer
- * @label - LED label
   */
  struct lp8860_led {
  	struct mutex lock;
@@ -106,7 +106,6 @@ struct lp8860_led {
  	struct regmap *eeprom_regmap;
  	struct gpio_desc *enable_gpio;
  	struct regulator *regulator;
-	char label[LED_MAX_NAME_SIZE];
  };
struct lp8860_eeprom_reg {
@@ -387,25 +386,26 @@ static int lp8860_probe(struct i2c_client *client,
  	struct lp8860_led *led;
  	struct device_node *np = client->dev.of_node;
  	struct device_node *child_node;
-	const char *name;
+	struct led_init_data init_data;
led = devm_kzalloc(&client->dev, sizeof(*led), GFP_KERNEL);
  	if (!led)
  		return -ENOMEM;
- for_each_available_child_of_node(np, child_node) {
-		led->led_dev.default_trigger = of_get_property(child_node,
-						    "linux,default-trigger",
-						    NULL);
-
-		ret = of_property_read_string(child_node, "label", &name);
-		if (!ret)
-			snprintf(led->label, sizeof(led->label), "%s:%s",
-				 id->name, name);
-		else
-			snprintf(led->label, sizeof(led->label),
-				"%s::display_cluster", id->name);
-	}
+	child_node = of_get_next_available_child(np, NULL);

Again this device has multiple outputs that can be banked or separated.

I would prefer to leave the iteration and just change the name generation.

In [0] you agreed on removing that. This code is simply broken since
there is only one instance of struct lp8860_led allocated.

+	if (!child_node)
+		return -EINVAL;
+
+	init_data.fwnode = of_fwnode_handle(child_node),
+
+	led->led_dev.default_trigger = of_get_property(child_node,
+					    "linux,default-trigger",
+					    NULL);
+
+	ret = led_compose_name(init_data.fwnode, LP8860_NAME,
+			       ":display_cluster", init_data.name);
+	if (ret)
+		return ret;
led->enable_gpio = devm_gpiod_get_optional(&client->dev,
  						   "enable", GPIOD_OUT_LOW);
@@ -420,7 +420,6 @@ static int lp8860_probe(struct i2c_client *client,
  		led->regulator = NULL;
led->client = client;
-	led->led_dev.name = led->label;
  	led->led_dev.brightness_set_blocking = lp8860_brightness_set;
mutex_init(&led->lock);
@@ -447,7 +446,8 @@ static int lp8860_probe(struct i2c_client *client,
  	if (ret)
  		return ret;
- ret = devm_led_classdev_register(&client->dev, &led->led_dev);
+	ret = devm_led_classdev_register_ext(&client->dev, &led->led_dev,
+					     &init_data);
  	if (ret) {
  		dev_err(&client->dev, "led register err: %d\n", ret);
  		return ret;




[0] https://lkml.org/lkml/2018/11/12/2231

--
Best regards,
Jacek Anaszewski



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux