[PATCH leds v1 08/10] leds: is31fl319x: 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 nor `linux,default-trigger` property.

This driver needed bigger refactor because it first parsed DT for all
LEDs and only after that started registering them.

Signed-off-by: Marek Behún <marek.behun@xxxxxx>
Cc: H. Nikolaus Schaller <hns@xxxxxxxxxxxxx>
---
 drivers/leds/leds-is31fl319x.c | 204 ++++++++++++++-------------------
 1 file changed, 89 insertions(+), 115 deletions(-)

diff --git a/drivers/leds/leds-is31fl319x.c b/drivers/leds/leds-is31fl319x.c
index 54ac50740d43d..3995f41687e16 100644
--- a/drivers/leds/leds-is31fl319x.c
+++ b/drivers/leds/leds-is31fl319x.c
@@ -63,7 +63,6 @@
 struct is31fl319x_chip {
 	const struct is31fl319x_chipdef *cdef;
 	struct i2c_client               *client;
-	struct gpio_desc		*shutdown_gpio;
 	struct regmap                   *regmap;
 	struct mutex                    lock;
 	u32                             audio_gain_db;
@@ -71,7 +70,6 @@ struct is31fl319x_chip {
 	struct is31fl319x_led {
 		struct is31fl319x_chip  *chip;
 		struct led_classdev     cdev;
-		u32                     max_microamp;
 		bool                    configured;
 	} leds[IS31FL319X_MAX_LEDS];
 };
@@ -171,117 +169,100 @@ static int is31fl319x_brightness_set(struct led_classdev *cdev,
 	return ret;
 }
 
-static int is31fl319x_parse_child_dt(const struct device *dev,
-				     const struct device_node *child,
-				     struct is31fl319x_led *led)
+static int is31fl319x_parse_max_current(struct device *dev, u32 *aggregated)
 {
-	struct led_classdev *cdev = &led->cdev;
+	struct device_node *np;
+	u32 max_microamp;
 	int ret;
 
-	if (of_property_read_string(child, "label", &cdev->name))
-		cdev->name = child->name;
-
-	ret = of_property_read_string(child, "linux,default-trigger",
-				      &cdev->default_trigger);
-	if (ret < 0 && ret != -EINVAL) /* is optional */
-		return ret;
+	*aggregated = IS31FL319X_CURRENT_MAX;
+
+	for_each_child_of_node(dev_of_node(dev), np) {
+		max_microamp = IS31FL319X_CURRENT_DEFAULT;
+		ret = of_property_read_u32(np, "led-max-microamp",
+					   &max_microamp);
+		if (!ret)
+			max_microamp = min(max_microamp,
+					   IS31FL319X_CURRENT_MAX);
+
+		if (max_microamp < IS31FL319X_CURRENT_MIN) {
+			dev_err(dev,
+				"Node %pOF led-max-microamp below supported value\n",
+				np);
+			of_node_put(np);
+			return -EINVAL;
+		}
 
-	led->max_microamp = IS31FL319X_CURRENT_DEFAULT;
-	ret = of_property_read_u32(child, "led-max-microamp",
-				   &led->max_microamp);
-	if (!ret) {
-		if (led->max_microamp < IS31FL319X_CURRENT_MIN)
-			return -EINVAL;	/* not supported */
-		led->max_microamp = min(led->max_microamp,
-					  IS31FL319X_CURRENT_MAX);
+		if (max_microamp < *aggregated)
+			*aggregated = max_microamp;
 	}
 
 	return 0;
 }
 
-static int is31fl319x_parse_dt(struct device *dev,
-			       struct is31fl319x_chip *is31)
+static int is31fl319x_led_register(struct device *dev,
+				   struct is31fl319x_chip *is31,
+				   struct device_node *np)
 {
-	struct device_node *np = dev->of_node, *child;
-	const struct of_device_id *of_dev_id;
-	int count;
+	struct led_init_data init_data = {};
+	struct is31fl319x_led *led;
+	u32 reg;
 	int ret;
 
-	if (!np)
-		return -ENODEV;
-
-	is31->shutdown_gpio = devm_gpiod_get_optional(dev,
-						"shutdown",
-						GPIOD_OUT_HIGH);
-	if (IS_ERR(is31->shutdown_gpio)) {
-		ret = PTR_ERR(is31->shutdown_gpio);
-		dev_err(dev, "Failed to get shutdown gpio: %d\n", ret);
+	ret = of_property_read_u32(np, "reg", &reg);
+	if (ret) {
+		dev_err(dev, "Cannot read reg property of %pOF\n", np);
 		return ret;
 	}
 
-	of_dev_id = of_match_device(of_is31fl319x_match, dev);
-	if (!of_dev_id) {
-		dev_err(dev, "Failed to match device with supported chips\n");
+	if (reg < 1 || reg > is31->cdef->num_leds) {
+		dev_err(dev, "Node %pOF has invalid reg property\n", np);
 		return -EINVAL;
 	}
 
-	is31->cdef = of_dev_id->data;
-
-	count = of_get_child_count(np);
+	led = &is31->leds[reg - 1];
 
-	dev_dbg(dev, "probe %s with %d leds defined in DT\n",
-		of_dev_id->compatible, count);
-
-	if (!count || count > is31->cdef->num_leds) {
-		dev_err(dev, "Number of leds defined must be between 1 and %u\n",
-			is31->cdef->num_leds);
-		return -ENODEV;
+	if (led->configured) {
+		dev_err(dev, "Node %pOF reg property already used\n", np);
+		return -EEXIST;
 	}
 
-	for_each_child_of_node(np, child) {
-		struct is31fl319x_led *led;
-		u32 reg;
+	led->configured = true;
+	led->chip = is31;
+	led->cdev.brightness_set_blocking = is31fl319x_brightness_set;
+	init_data.fwnode = of_fwnode_handle(np);
 
-		ret = of_property_read_u32(child, "reg", &reg);
-		if (ret) {
-			dev_err(dev, "Failed to read led 'reg' property\n");
-			goto put_child_node;
-		}
-
-		if (reg < 1 || reg > is31->cdef->num_leds) {
-			dev_err(dev, "invalid led reg %u\n", reg);
-			ret = -EINVAL;
-			goto put_child_node;
-		}
+	ret = devm_led_classdev_register_ext(dev, &led->cdev, &init_data);
+	if (ret)
+		dev_err(dev, "Failed to register LED for node %pOF\n", np);
 
-		led = &is31->leds[reg - 1];
-
-		if (led->configured) {
-			dev_err(dev, "led %u is already configured\n", reg);
-			ret = -EINVAL;
-			goto put_child_node;
-		}
+	return ret;
+}
 
-		ret = is31fl319x_parse_child_dt(dev, child, led);
-		if (ret) {
-			dev_err(dev, "led %u DT parsing failed\n", reg);
-			goto put_child_node;
-		}
+static int is31fl319x_parse_and_register(struct device *dev,
+					 struct is31fl319x_chip *is31)
+{
+	struct device_node *np = dev_of_node(dev), *child;
+	int ret, count;
 
-		led->configured = true;
+	count = of_get_child_count(np);
+	if (count < 1 || count > is31->cdef->num_leds) {
+		dev_err(dev, "Between 1 and %u LEDs have to be defined\n",
+			is31->cdef->num_leds);
+		return -ENODEV;
 	}
 
-	is31->audio_gain_db = 0;
-	ret = of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db);
-	if (!ret)
+	if (!of_property_read_u32(np, "audio-gain-db", &is31->audio_gain_db))
 		is31->audio_gain_db = min(is31->audio_gain_db,
 					  IS31FL319X_AUDIO_GAIN_DB_MAX);
 
-	return 0;
+	for_each_child_of_node(np, child) {
+		ret = is31fl319x_led_register(dev, is31, child);
+		if (ret)
+			return ret;
+	}
 
-put_child_node:
-	of_node_put(child);
-	return ret;
+	return 0;
 }
 
 static bool is31fl319x_readable_reg(struct device *dev, unsigned int reg)
@@ -345,30 +326,43 @@ static int is31fl319x_probe(struct i2c_client *client,
 {
 	struct is31fl319x_chip *is31;
 	struct device *dev = &client->dev;
+	struct gpio_desc *shutdown_gpio;
+	u32 aggregated_led_microamp;
 	int err;
-	int i = 0;
-	u32 aggregated_led_microamp = IS31FL319X_CURRENT_MAX;
 
 	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
 		return -EIO;
 
+	/*
+	 * Kernel conventions require per-LED led-max-microamp property.
+	 * But the chip does not allow to limit individual LEDs.
+	 * So we take minimum from all subnodes for safety of hardware.
+	 */
+	err = is31fl319x_parse_max_current(dev, &aggregated_led_microamp);
+	if (err)
+		return err;
+
 	is31 = devm_kzalloc(&client->dev, sizeof(*is31), GFP_KERNEL);
 	if (!is31)
 		return -ENOMEM;
 
-	mutex_init(&is31->lock);
-
-	err = is31fl319x_parse_dt(&client->dev, is31);
-	if (err)
-		goto free_mutex;
+	shutdown_gpio = gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH);
+	if (IS_ERR(shutdown_gpio)) {
+		dev_err(dev, "Failed to get shutdown gpio\n");
+		return PTR_ERR(shutdown_gpio);
+	}
 
-	if (is31->shutdown_gpio) {
-		gpiod_direction_output(is31->shutdown_gpio, 0);
+	if (shutdown_gpio) {
+		gpiod_direction_output(shutdown_gpio, 0);
 		mdelay(5);
-		gpiod_direction_output(is31->shutdown_gpio, 1);
+		gpiod_direction_output(shutdown_gpio, 1);
+		gpiod_put(shutdown_gpio);
 	}
 
+	mutex_init(&is31->lock);
+
 	is31->client = client;
+	is31->cdef = device_get_match_data(dev);
 	is31->regmap = devm_regmap_init_i2c(client, &regmap_config);
 	if (IS_ERR(is31->regmap)) {
 		dev_err(&client->dev, "failed to allocate register map\n");
@@ -387,33 +381,13 @@ static int is31fl319x_probe(struct i2c_client *client,
 		goto free_mutex;
 	}
 
-	/*
-	 * Kernel conventions require per-LED led-max-microamp property.
-	 * But the chip does not allow to limit individual LEDs.
-	 * So we take minimum from all subnodes for safety of hardware.
-	 */
-	for (i = 0; i < is31->cdef->num_leds; i++)
-		if (is31->leds[i].configured &&
-		    is31->leds[i].max_microamp < aggregated_led_microamp)
-			aggregated_led_microamp = is31->leds[i].max_microamp;
-
 	regmap_write(is31->regmap, IS31FL319X_CONFIG2,
 		     is31fl319x_microamp_to_cs(dev, aggregated_led_microamp) |
 		     is31fl319x_db_to_gain(is31->audio_gain_db));
 
-	for (i = 0; i < is31->cdef->num_leds; i++) {
-		struct is31fl319x_led *led = &is31->leds[i];
-
-		if (!led->configured)
-			continue;
-
-		led->chip = is31;
-		led->cdev.brightness_set_blocking = is31fl319x_brightness_set;
-
-		err = devm_led_classdev_register(&client->dev, &led->cdev);
-		if (err < 0)
-			goto free_mutex;
-	}
+	err = is31fl319x_parse_and_register(dev, is31);
+	if (err)
+		goto free_mutex;
 
 	return 0;
 
-- 
2.26.2