diff mbox series

[leds,v1,06/10] leds: pm8058: use struct led_init_data when registering

Message ID 20200916231650.11484-7-marek.behun@nic.cz
State New
Headers show
Series [leds,v1,01/10] leds: parse linux,default-trigger DT property in LED core | expand

Commit Message

Marek Behún Sept. 16, 2020, 11:16 p.m. UTC
By using struct led_init_data when registering we do not need to parse
`label` DT property nor `linux,default-trigger` property.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 drivers/leds/leds-pm8058.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

Bjorn Andersson Sept. 17, 2020, 12:46 a.m. UTC | #1
On Wed 16 Sep 18:16 CDT 2020, Marek Beh?n wrote:

> By using struct led_init_data when registering we do not need to parse
> `label` DT property nor `linux,default-trigger` property.
> 
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  drivers/leds/leds-pm8058.c | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c
> index 7869ccdf70ce6..f6190e4af60fe 100644
> --- a/drivers/leds/leds-pm8058.c
> +++ b/drivers/leds/leds-pm8058.c
> @@ -87,36 +87,37 @@ static enum led_brightness pm8058_led_get(struct led_classdev *cled)
>  
>  static int pm8058_led_probe(struct platform_device *pdev)
>  {
> +	struct led_init_data init_data = {};
> +	struct device *dev = &pdev->dev;
> +	enum led_brightness maxbright;
> +	struct device_node *np;
>  	struct pm8058_led *led;
> -	struct device_node *np = pdev->dev.of_node;
> -	int ret;
>  	struct regmap *map;
>  	const char *state;
> -	enum led_brightness maxbright;
> +	int ret;
>  
> -	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
> +	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);

The pdev->dev -> dev and of_node changes are reasonable, but shouldn't
be part of this patch. It simply makes it hard to reason about he actual
change.

Please respin this with only the introduction of led_init_data.

Thanks,
Bjorn

>  	if (!led)
>  		return -ENOMEM;
>  
> -	led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev);
> +	led->ledtype = (u32)(unsigned long)device_get_match_data(dev);
>  
> -	map = dev_get_regmap(pdev->dev.parent, NULL);
> +	map = dev_get_regmap(dev->parent, NULL);
>  	if (!map) {
> -		dev_err(&pdev->dev, "Parent regmap unavailable.\n");
> +		dev_err(dev, "Parent regmap unavailable.\n");
>  		return -ENXIO;
>  	}
>  	led->map = map;
>  
> +	np = dev_of_node(dev);
> +
>  	ret = of_property_read_u32(np, "reg", &led->reg);
>  	if (ret) {
> -		dev_err(&pdev->dev, "no register offset specified\n");
> +		dev_err(dev, "no register offset specified\n");
>  		return -EINVAL;
>  	}
>  
>  	/* Use label else node name */
> -	led->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
> -	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)
> @@ -142,14 +143,13 @@ static int pm8058_led_probe(struct platform_device *pdev)
>  	    led->ledtype == PM8058_LED_TYPE_FLASH)
>  		led->cdev.flags	= LED_CORE_SUSPENDRESUME;
>  
> -	ret = devm_led_classdev_register(&pdev->dev, &led->cdev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "unable to register led \"%s\"\n",
> -			led->cdev.name);
> -		return ret;
> -	}
> +	init_data.fwnode = of_fwnode_handle(np);
> +
> +	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);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  static const struct of_device_id pm8058_leds_id_table[] = {
> @@ -173,7 +173,7 @@ static struct platform_driver pm8058_led_driver = {
>  	.probe		= pm8058_led_probe,
>  	.driver		= {
>  		.name	= "pm8058-leds",
> -		.of_match_table = pm8058_leds_id_table,
> +		.of_match_table = of_match_ptr(pm8058_leds_id_table),
>  	},
>  };
>  module_platform_driver(pm8058_led_driver);
> -- 
> 2.26.2
>
Marek Behún Sept. 17, 2020, 3:24 p.m. UTC | #2
On Wed, 16 Sep 2020 19:46:25 -0500
Bjorn Andersson <bjorn.andersson@linaro.org> wrote:

> The pdev->dev -> dev and of_node changes are reasonable, but shouldn't
> be part of this patch. It simply makes it hard to reason about he actual
> change.
> 
> Please respin this with only the introduction of led_init_data.
> 
> Thanks,
> Bjorn
> 
Am working on this :)
diff mbox series

Patch

diff --git a/drivers/leds/leds-pm8058.c b/drivers/leds/leds-pm8058.c
index 7869ccdf70ce6..f6190e4af60fe 100644
--- a/drivers/leds/leds-pm8058.c
+++ b/drivers/leds/leds-pm8058.c
@@ -87,36 +87,37 @@  static enum led_brightness pm8058_led_get(struct led_classdev *cled)
 
 static int pm8058_led_probe(struct platform_device *pdev)
 {
+	struct led_init_data init_data = {};
+	struct device *dev = &pdev->dev;
+	enum led_brightness maxbright;
+	struct device_node *np;
 	struct pm8058_led *led;
-	struct device_node *np = pdev->dev.of_node;
-	int ret;
 	struct regmap *map;
 	const char *state;
-	enum led_brightness maxbright;
+	int ret;
 
-	led = devm_kzalloc(&pdev->dev, sizeof(*led), GFP_KERNEL);
+	led = devm_kzalloc(dev, sizeof(*led), GFP_KERNEL);
 	if (!led)
 		return -ENOMEM;
 
-	led->ledtype = (u32)(unsigned long)of_device_get_match_data(&pdev->dev);
+	led->ledtype = (u32)(unsigned long)device_get_match_data(dev);
 
-	map = dev_get_regmap(pdev->dev.parent, NULL);
+	map = dev_get_regmap(dev->parent, NULL);
 	if (!map) {
-		dev_err(&pdev->dev, "Parent regmap unavailable.\n");
+		dev_err(dev, "Parent regmap unavailable.\n");
 		return -ENXIO;
 	}
 	led->map = map;
 
+	np = dev_of_node(dev);
+
 	ret = of_property_read_u32(np, "reg", &led->reg);
 	if (ret) {
-		dev_err(&pdev->dev, "no register offset specified\n");
+		dev_err(dev, "no register offset specified\n");
 		return -EINVAL;
 	}
 
 	/* Use label else node name */
-	led->cdev.name = of_get_property(np, "label", NULL) ? : np->name;
-	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)
@@ -142,14 +143,13 @@  static int pm8058_led_probe(struct platform_device *pdev)
 	    led->ledtype == PM8058_LED_TYPE_FLASH)
 		led->cdev.flags	= LED_CORE_SUSPENDRESUME;
 
-	ret = devm_led_classdev_register(&pdev->dev, &led->cdev);
-	if (ret) {
-		dev_err(&pdev->dev, "unable to register led \"%s\"\n",
-			led->cdev.name);
-		return ret;
-	}
+	init_data.fwnode = of_fwnode_handle(np);
+
+	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);
 
-	return 0;
+	return ret;
 }
 
 static const struct of_device_id pm8058_leds_id_table[] = {
@@ -173,7 +173,7 @@  static struct platform_driver pm8058_led_driver = {
 	.probe		= pm8058_led_probe,
 	.driver		= {
 		.name	= "pm8058-leds",
-		.of_match_table = pm8058_leds_id_table,
+		.of_match_table = of_match_ptr(pm8058_leds_id_table),
 	},
 };
 module_platform_driver(pm8058_led_driver);