[PATCH v3 0/5] leds: add devicetree functionality to lltc,lt3593 driver

[PATCH v3 0/5] leds: add devicetree functionality to lltc,lt3593 driver

[Daniel Mack]

This is v3 of the series that brings devicetree support for the
lltc,lt3593 LED driver.

Before this series, the driver supported controlling multiple LEDs
through an array in the platform data. IOW, a single instance of the
driver was able to control multiple hardware chips.

This series changes that, and requires a distinct platform device to
be set up for each of them, if a board has multiple of these hardware
chips. The reason is that in DT, nodes should represent hardware, and
it's much cleaner this way.

As stated earlier in the thread for v1, the driver currently only has
one user in mainline (the Raumfeld platform) which is soon to be
replaced by a devicetree file. This user only uses one LED via pdata, so
the change mentioned above does not cause a regression.

Once the platform is fully ported to DT, I'll send another patch that
removes pdata handling from this driver completely, but it's kept around
as legacy bridge for now.


Changelog:

v2 → v3:

* Fixed a typo in the commit log of 1/5
* Rebased onto 4.18-rc1

v1 → v2:

* Moved LED-specific properties into a sub-node in DT, as requested by
  Jacek Anaszewski.


Daniel Mack (5):
  dt-bindings: leds: Add bindings for lltc,lt3593
  leds: lt3593: merge functions and clean up code
  leds: lt3593: switch to gpiod interface
  leds: lt3593: Add device tree probing glue
  leds: lt3593: update email address

 .../devicetree/bindings/leds/leds-lt3593.txt  |  35 ++++
 drivers/leds/leds-lt3593.c                    | 175 +++++++++++-------
 2 files changed, 142 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lt3593.txt

-- 
2.17.1

[PATCH v3 1/5] dt-bindings: leds: Add bindings for lltc,lt3593

[Daniel Mack]

This patch add the bindings document for LT3593 LED drivers.
The binding is kept consistent with other LED driver bindings in that it
stores all the LED-specific properties in its own subnode. As the hardware
only supports one channel, there can consequently only be one sub-node.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 .../devicetree/bindings/leds/leds-lt3593.txt  | 35 +++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/leds/leds-lt3593.txt

diff --git a/Documentation/devicetree/bindings/leds/leds-lt3593.txt b/Documentation/devicetree/bindings/leds/leds-lt3593.txt
new file mode 100644
index 000000000000..9e7f308404f8
--- /dev/null
+++ b/Documentation/devicetree/bindings/leds/leds-lt3593.txt
@@ -0,0 +1,35 @@
+Bindings for Linear Technologies LT3593 LED controller
+
+Required properties:
+- compatible:	Should be "lltc,lt3593".
+
+The hardware supports only one LED. The properties of this LED are
+configured in a sub-node in the device node.
+
+Required sub-node properties:
+- gpios:	A handle to the GPIO that is connected to the 'CTRL'
+		pin of the chip.
+
+Optional sub-node properties:
+- label:	A label for the LED. If none is given, the LED will be
+		named "lt3595::".
+- linux,default-trigger: The default trigger for the LED.
+			See Documentation/devicetree/bindings/leds/common.txt
+- default-state:	The initial state of the LED.
+			See Documentation/devicetree/bindings/leds/common.txt
+
+If multiple chips of this type are found in a design, each one needs to
+be handled by its own device node.
+
+Example:
+
+led-controller {
+	compatible = "lltc,lt3593";
+
+	led {
+		label = "white:backlight";
+		gpios = <&gpio 0 GPIO_ACTIVE_HIGH>;
+		default-state = "on";
+	};
+};
+
-- 
2.17.1

[PATCH v3 2/5] leds: lt3593: merge functions and clean up code

[Daniel Mack]

In preparation to DT probe functionality, merge create_lt3593_led() into
its only call-site. The DT based setup code will be quite different, so this
internal helper function is of no help.

This also changes the way the driver works by only handling one entry inside
'struct gpio_led_platform_data'. If multiple devices of the same type are
found in a design, there should be a platform device for each of them. The
only mainline user of this driver is not affected by this change.

Last, use devm_led_classdev_register() instead of
led_classdev_register(), so the driver's remove callback can go away.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/leds/leds-lt3593.c | 103 +++++++++++++------------------------
 1 file changed, 35 insertions(+), 68 deletions(-)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index 5ec730a31b65..a99ba2741e39 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -71,104 +71,71 @@ static int lt3593_led_set(struct led_classdev *led_cdev,
 	return 0;
 }
 
-static int create_lt3593_led(const struct gpio_led *template,
-	struct lt3593_led_data *led_dat, struct device *parent)
+static struct lt3593_led_data *lt3593_led_probe_pdata(struct device *dev)
 {
+	struct gpio_led_platform_data *pdata = dev_get_platdata(dev);
+	const struct gpio_led *template = &pdata->leds[0];
+	struct lt3593_led_data *led_data;
 	int ret, state;
 
-	/* skip leds on GPIOs that aren't available */
+	if (pdata->num_leds != 1)
+		return ERR_PTR(-EINVAL);
+
+	led_data = devm_kcalloc(dev, sizeof(*led_data), GFP_KERNEL);
+	if (!led_data)
+		return ERR_PTR(-ENOMEM);
+
 	if (!gpio_is_valid(template->gpio)) {
-		dev_info(parent, "%s: skipping unavailable LT3593 LED at gpio %d (%s)\n",
-				KBUILD_MODNAME, template->gpio, template->name);
-		return 0;
+		dev_info(dev, "skipping unavailable LT3593 LED at gpio "
+			 "%d (%s)\n", template->gpio, template->name);
+		return ERR_PTR(-EINVAL);
 	}
 
-	led_dat->cdev.name = template->name;
-	led_dat->cdev.default_trigger = template->default_trigger;
-	led_dat->gpio = template->gpio;
-
-	led_dat->cdev.brightness_set_blocking = lt3593_led_set;
+	led_data->cdev.name = template->name;
+	led_data->cdev.default_trigger = template->default_trigger;
+	led_data->gpio = template->gpio;
+	led_data->cdev.brightness_set_blocking = lt3593_led_set;
 
 	state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
-	led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
+	led_data->cdev.brightness = state ? LED_FULL : LED_OFF;
 
 	if (!template->retain_state_suspended)
-		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
+		led_data->cdev.flags |= LED_CORE_SUSPENDRESUME;
 
-	ret = devm_gpio_request_one(parent, template->gpio, state ?
+	ret = devm_gpio_request_one(dev, template->gpio, state ?
 				    GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
 				    template->name);
 	if (ret < 0)
-		return ret;
+		return ERR_PTR(ret);
 
-	ret = led_classdev_register(parent, &led_dat->cdev);
+	ret = devm_led_classdev_register(dev, &led_data->cdev);
 	if (ret < 0)
-		return ret;
-
-	dev_info(parent, "%s: registered LT3593 LED '%s' at GPIO %d\n",
-		KBUILD_MODNAME, template->name, template->gpio);
+		return ERR_PTR(ret);
 
-	return 0;
-}
+	dev_info(dev, "registered LT3593 LED '%s' at GPIO %d\n",
+		 template->name, template->gpio);
 
-static void delete_lt3593_led(struct lt3593_led_data *led)
-{
-	if (!gpio_is_valid(led->gpio))
-		return;
-
-	led_classdev_unregister(&led->cdev);
+	return led_data;
 }
 
 static int lt3593_led_probe(struct platform_device *pdev)
 {
-	struct gpio_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
-	struct lt3593_led_data *leds_data;
-	int i, ret = 0;
-
-	if (!pdata)
-		return -EBUSY;
-
-	leds_data = devm_kcalloc(&pdev->dev,
-			pdata->num_leds, sizeof(struct lt3593_led_data),
-			GFP_KERNEL);
-	if (!leds_data)
-		return -ENOMEM;
-
-	for (i = 0; i < pdata->num_leds; i++) {
-		ret = create_lt3593_led(&pdata->leds[i], &leds_data[i],
-				      &pdev->dev);
-		if (ret < 0)
-			goto err;
-	}
+	struct device *dev = &pdev->dev;
+	struct lt3593_led_data *led_data;
 
-	platform_set_drvdata(pdev, leds_data);
-
-	return 0;
-
-err:
-	for (i = i - 1; i >= 0; i--)
-		delete_lt3593_led(&leds_data[i]);
-
-	return ret;
-}
-
-static int lt3593_led_remove(struct platform_device *pdev)
-{
-	int i;
-	struct gpio_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
-	struct lt3593_led_data *leds_data;
-
-	leds_data = platform_get_drvdata(pdev);
+	if (dev_get_platdata(dev)) {
+		led_data = lt3593_led_probe_pdata(dev);
+		if (IS_ERR(led_data))
+			return PTR_ERR(led_data);
+	}
 
-	for (i = 0; i < pdata->num_leds; i++)
-		delete_lt3593_led(&leds_data[i]);
+	platform_set_drvdata(pdev, led_data);
 
 	return 0;
 }
 
 static struct platform_driver lt3593_led_driver = {
 	.probe		= lt3593_led_probe,
-	.remove		= lt3593_led_remove,
 	.driver		= {
 		.name	= "leds-lt3593",
 	},
-- 
2.17.1

[PATCH v3 3/5] leds: lt3593: switch to gpiod interface

[Daniel Mack]

Clean up the code a bit and transition over to the gpiod based interface.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/leds/leds-lt3593.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index a99ba2741e39..a7bc1fab6584 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -21,12 +21,13 @@
 #include <linux/leds.h>
 #include <linux/delay.h>
 #include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
 
 struct lt3593_led_data {
 	struct led_classdev cdev;
-	unsigned gpio;
+	struct gpio_desc *gpiod;
 };
 
 static int lt3593_led_set(struct led_classdev *led_cdev,
@@ -46,25 +47,25 @@ static int lt3593_led_set(struct led_classdev *led_cdev,
 	 */
 
 	if (value == 0) {
-		gpio_set_value_cansleep(led_dat->gpio, 0);
+		gpiod_set_value_cansleep(led_dat->gpiod, 0);
 		return 0;
 	}
 
 	pulses = 32 - (value * 32) / 255;
 
 	if (pulses == 0) {
-		gpio_set_value_cansleep(led_dat->gpio, 0);
+		gpiod_set_value_cansleep(led_dat->gpiod, 0);
 		mdelay(1);
-		gpio_set_value_cansleep(led_dat->gpio, 1);
+		gpiod_set_value_cansleep(led_dat->gpiod, 1);
 		return 0;
 	}
 
-	gpio_set_value_cansleep(led_dat->gpio, 1);
+	gpiod_set_value_cansleep(led_dat->gpiod, 1);
 
 	while (pulses--) {
-		gpio_set_value_cansleep(led_dat->gpio, 0);
+		gpiod_set_value_cansleep(led_dat->gpiod, 0);
 		udelay(1);
-		gpio_set_value_cansleep(led_dat->gpio, 1);
+		gpiod_set_value_cansleep(led_dat->gpiod, 1);
 		udelay(1);
 	}
 
@@ -85,15 +86,8 @@ static struct lt3593_led_data *lt3593_led_probe_pdata(struct device *dev)
 	if (!led_data)
 		return ERR_PTR(-ENOMEM);
 
-	if (!gpio_is_valid(template->gpio)) {
-		dev_info(dev, "skipping unavailable LT3593 LED at gpio "
-			 "%d (%s)\n", template->gpio, template->name);
-		return ERR_PTR(-EINVAL);
-	}
-
 	led_data->cdev.name = template->name;
 	led_data->cdev.default_trigger = template->default_trigger;
-	led_data->gpio = template->gpio;
 	led_data->cdev.brightness_set_blocking = lt3593_led_set;
 
 	state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
@@ -108,6 +102,10 @@ static struct lt3593_led_data *lt3593_led_probe_pdata(struct device *dev)
 	if (ret < 0)
 		return ERR_PTR(ret);
 
+	led_data->gpiod = gpio_to_desc(template->gpio);
+	if (!led_data->gpiod)
+		return ERR_PTR(-EPROBE_DEFER);
+
 	ret = devm_led_classdev_register(dev, &led_data->cdev);
 	if (ret < 0)
 		return ERR_PTR(ret);
-- 
2.17.1

[PATCH v3 4/5] leds: lt3593: Add device tree probing glue

[Daniel Mack]

The binding details are described in an earlier commit that adds the
documentation.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/leds/leds-lt3593.c | 74 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index a7bc1fab6584..e45c26588799 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -24,8 +24,11 @@
 #include <linux/gpio/consumer.h>
 #include <linux/slab.h>
 #include <linux/module.h>
+#include <linux/of.h>
+#include <uapi/linux/uleds.h>
 
 struct lt3593_led_data {
+	char name[LED_MAX_NAME_SIZE];
 	struct led_classdev cdev;
 	struct gpio_desc *gpiod;
 };
@@ -120,22 +123,93 @@ static int lt3593_led_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct lt3593_led_data *led_data;
+	struct fwnode_handle *child;
+	int ret, state = LEDS_GPIO_DEFSTATE_OFF;
+	enum gpiod_flags flags = GPIOD_OUT_LOW;
+	const char *tmp;
 
 	if (dev_get_platdata(dev)) {
 		led_data = lt3593_led_probe_pdata(dev);
 		if (IS_ERR(led_data))
 			return PTR_ERR(led_data);
+
+		goto out;
+	}
+
+	if (!dev->of_node)
+		return -ENODEV;
+
+	led_data = devm_kzalloc(dev, sizeof(*led_data), GFP_KERNEL);
+	if (!led_data)
+		return -ENOMEM;
+
+	if (device_get_child_node_count(dev) != 1) {
+		dev_err(dev, "Device must have exactly one LED sub-node.");
+		return -EINVAL;
 	}
 
+	child = device_get_next_child_node(dev, NULL);
+
+	ret = fwnode_property_read_string(child, "label", &tmp);
+	if (ret < 0)
+		snprintf(led_data->name, sizeof(led_data->name),
+			 "lt3593::");
+	else
+		snprintf(led_data->name, sizeof(led_data->name),
+			 "lt3593:%s", tmp);
+
+	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, "keep")) {
+			state = LEDS_GPIO_DEFSTATE_KEEP;
+			flags = GPIOD_ASIS;
+		} else if (!strcmp(tmp, "on")) {
+			state = LEDS_GPIO_DEFSTATE_ON;
+			flags = GPIOD_OUT_HIGH;
+		}
+	}
+
+	led_data->gpiod = devm_fwnode_get_gpiod_from_child(dev, NULL, child,
+							   flags,
+							   led_data->name);
+	if (IS_ERR(led_data->gpiod)) {
+		fwnode_handle_put(child);
+		return PTR_ERR(led_data->gpiod);
+	}
+
+	led_data->cdev.name = led_data->name;
+	led_data->cdev.brightness_set_blocking = lt3593_led_set;
+	led_data->cdev.brightness = state ? LED_FULL : LED_OFF;
+
+	ret = devm_led_classdev_register(dev, &led_data->cdev);
+	if (ret < 0) {
+		fwnode_handle_put(child);
+		return ret;
+	}
+
+	led_data->cdev.dev->of_node = dev->of_node;
+
+out:
 	platform_set_drvdata(pdev, led_data);
 
 	return 0;
 }
 
+#ifdef CONFIG_OF
+static const struct of_device_id of_lt3593_leds_match[] = {
+	{ .compatible = "lltc,lt3593", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_lt3593_leds_match);
+#endif
+
 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),
 	},
 };
 
-- 
2.17.1

[PATCH v3 5/5] leds: lt3593: update email address

[Daniel Mack]

Update the email address in the module information and in the header.

Signed-off-by: Daniel Mack <daniel@zonque.org>
---
 drivers/leds/leds-lt3593.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
index e45c26588799..4ba633740b14 100644
--- a/drivers/leds/leds-lt3593.c
+++ b/drivers/leds/leds-lt3593.c
@@ -3,7 +3,7 @@
  *
  * See the datasheet at http://cds.linear.com/docs/Datasheet/3593f.pdf
  *
- * Copyright (c) 2009 Daniel Mack <daniel@caiaq.de>
+ * Copyright (c) 2009,2018 Daniel Mack <daniel@zonque.org>
  *
  * Based on leds-gpio.c,
  *
@@ -215,7 +215,7 @@ static struct platform_driver lt3593_led_driver = {
 
 module_platform_driver(lt3593_led_driver);
 
-MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
+MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>");
 MODULE_DESCRIPTION("LED driver for LT3593 controllers");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:leds-lt3593");
-- 
2.17.1

Re: [PATCH v3 2/5] leds: lt3593: merge functions and clean up code

[Jacek Anaszewski]

Hi Daniel,

You switched to using kcalloc, but apparently without compile-testing
it:

drivers/leds/leds-lt3593.c: In function ‘lt3593_led_probe_pdata’:
drivers/leds/leds-lt3593.c:88:13: error: too few arguments to function 
‘devm_kcalloc’
   led_data = devm_kcalloc(dev, sizeof(*led_data), GFP_KERNEL);

Once you'll be resending the patch set please also remove
empty line at the end of DT bindings.

Also you could think of switching to using SPDX license identifier
at the occasion of updating the email address.

Best regards,
acek Anaszewski

On 06/20/2018 10:13 PM, Daniel Mack wrote:
> In preparation to DT probe functionality, merge create_lt3593_led() into
> its only call-site. The DT based setup code will be quite different, so this
> internal helper function is of no help.
> 
> This also changes the way the driver works by only handling one entry inside
> 'struct gpio_led_platform_data'. If multiple devices of the same type are
> found in a design, there should be a platform device for each of them. The
> only mainline user of this driver is not affected by this change.
> 
> Last, use devm_led_classdev_register() instead of
> led_classdev_register(), so the driver's remove callback can go away.
> 
> Signed-off-by: Daniel Mack <daniel@zonque.org>
> ---
>   drivers/leds/leds-lt3593.c | 103 +++++++++++++------------------------
>   1 file changed, 35 insertions(+), 68 deletions(-)
> 
> diff --git a/drivers/leds/leds-lt3593.c b/drivers/leds/leds-lt3593.c
> index 5ec730a31b65..a99ba2741e39 100644
> --- a/drivers/leds/leds-lt3593.c
> +++ b/drivers/leds/leds-lt3593.c
> @@ -71,104 +71,71 @@ static int lt3593_led_set(struct led_classdev *led_cdev,
>   	return 0;
>   }
>   
> -static int create_lt3593_led(const struct gpio_led *template,
> -	struct lt3593_led_data *led_dat, struct device *parent)
> +static struct lt3593_led_data *lt3593_led_probe_pdata(struct device *dev)
>   {
> +	struct gpio_led_platform_data *pdata = dev_get_platdata(dev);
> +	const struct gpio_led *template = &pdata->leds[0];
> +	struct lt3593_led_data *led_data;
>   	int ret, state;
>   
> -	/* skip leds on GPIOs that aren't available */
> +	if (pdata->num_leds != 1)
> +		return ERR_PTR(-EINVAL);
> +
> +	led_data = devm_kcalloc(dev, sizeof(*led_data), GFP_KERNEL);
> +	if (!led_data)
> +		return ERR_PTR(-ENOMEM);
> +
>   	if (!gpio_is_valid(template->gpio)) {
> -		dev_info(parent, "%s: skipping unavailable LT3593 LED at gpio %d (%s)\n",
> -				KBUILD_MODNAME, template->gpio, template->name);
> -		return 0;
> +		dev_info(dev, "skipping unavailable LT3593 LED at gpio "
> +			 "%d (%s)\n", template->gpio, template->name);
> +		return ERR_PTR(-EINVAL);
>   	}
>   
> -	led_dat->cdev.name = template->name;
> -	led_dat->cdev.default_trigger = template->default_trigger;
> -	led_dat->gpio = template->gpio;
> -
> -	led_dat->cdev.brightness_set_blocking = lt3593_led_set;
> +	led_data->cdev.name = template->name;
> +	led_data->cdev.default_trigger = template->default_trigger;
> +	led_data->gpio = template->gpio;
> +	led_data->cdev.brightness_set_blocking = lt3593_led_set;
>   
>   	state = (template->default_state == LEDS_GPIO_DEFSTATE_ON);
> -	led_dat->cdev.brightness = state ? LED_FULL : LED_OFF;
> +	led_data->cdev.brightness = state ? LED_FULL : LED_OFF;
>   
>   	if (!template->retain_state_suspended)
> -		led_dat->cdev.flags |= LED_CORE_SUSPENDRESUME;
> +		led_data->cdev.flags |= LED_CORE_SUSPENDRESUME;
>   
> -	ret = devm_gpio_request_one(parent, template->gpio, state ?
> +	ret = devm_gpio_request_one(dev, template->gpio, state ?
>   				    GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW,
>   				    template->name);
>   	if (ret < 0)
> -		return ret;
> +		return ERR_PTR(ret);
>   
> -	ret = led_classdev_register(parent, &led_dat->cdev);
> +	ret = devm_led_classdev_register(dev, &led_data->cdev);
>   	if (ret < 0)
> -		return ret;
> -
> -	dev_info(parent, "%s: registered LT3593 LED '%s' at GPIO %d\n",
> -		KBUILD_MODNAME, template->name, template->gpio);
> +		return ERR_PTR(ret);
>   
> -	return 0;
> -}
> +	dev_info(dev, "registered LT3593 LED '%s' at GPIO %d\n",
> +		 template->name, template->gpio);
>   
> -static void delete_lt3593_led(struct lt3593_led_data *led)
> -{
> -	if (!gpio_is_valid(led->gpio))
> -		return;
> -
> -	led_classdev_unregister(&led->cdev);
> +	return led_data;
>   }
>   
>   static int lt3593_led_probe(struct platform_device *pdev)
>   {
> -	struct gpio_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
> -	struct lt3593_led_data *leds_data;
> -	int i, ret = 0;
> -
> -	if (!pdata)
> -		return -EBUSY;
> -
> -	leds_data = devm_kcalloc(&pdev->dev,
> -			pdata->num_leds, sizeof(struct lt3593_led_data),
> -			GFP_KERNEL);
> -	if (!leds_data)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < pdata->num_leds; i++) {
> -		ret = create_lt3593_led(&pdata->leds[i], &leds_data[i],
> -				      &pdev->dev);
> -		if (ret < 0)
> -			goto err;
> -	}
> +	struct device *dev = &pdev->dev;
> +	struct lt3593_led_data *led_data;
>   
> -	platform_set_drvdata(pdev, leds_data);
> -
> -	return 0;
> -
> -err:
> -	for (i = i - 1; i >= 0; i--)
> -		delete_lt3593_led(&leds_data[i]);
> -
> -	return ret;
> -}
> -
> -static int lt3593_led_remove(struct platform_device *pdev)
> -{
> -	int i;
> -	struct gpio_led_platform_data *pdata = dev_get_platdata(&pdev->dev);
> -	struct lt3593_led_data *leds_data;
> -
> -	leds_data = platform_get_drvdata(pdev);
> +	if (dev_get_platdata(dev)) {
> +		led_data = lt3593_led_probe_pdata(dev);
> +		if (IS_ERR(led_data))
> +			return PTR_ERR(led_data);
> +	}
>   
> -	for (i = 0; i < pdata->num_leds; i++)
> -		delete_lt3593_led(&leds_data[i]);
> +	platform_set_drvdata(pdev, led_data);
>   
>   	return 0;
>   }
>   
>   static struct platform_driver lt3593_led_driver = {
>   	.probe		= lt3593_led_probe,
> -	.remove		= lt3593_led_remove,
>   	.driver		= {
>   		.name	= "leds-lt3593",
>   	},
> 

Re: [PATCH v3 2/5] leds: lt3593: merge functions and clean up code

[Daniel Mack]

HI Jacek,

On Thursday, June 21, 2018 09:23 PM, Jacek Anaszewski wrote:
> You switched to using kcalloc, but apparently without compile-testing
> it:
> 
> drivers/leds/leds-lt3593.c: In function ‘lt3593_led_probe_pdata’:
> drivers/leds/leds-lt3593.c:88:13: error: too few arguments to function
> ‘devm_kcalloc’
>     led_data = devm_kcalloc(dev, sizeof(*led_data), GFP_KERNEL);

How embarrassing. I didn't mean to switch to kcalloc as I'm not 
allocation an array anymore. I forgot to stage a hunk when rebasing 
though. Oh well.

> Once you'll be resending the patch set please also remove
> empty line at the end of DT bindings.
> 
> Also you could think of switching to using SPDX license identifier
> at the occasion of updating the email address.

Will do. Sorry for the trouble. I'm sure v4 will be good finally :)



Thanks,
Daniel