[PATCH v2 1/2] leds: max77650: Switch to fwnode property API

[PATCH v2 1/2] leds: max77650: Switch to fwnode property API

[Andy Shevchenko]

Switch the max77650 from OF to the fwnode property API.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/leds-max77650.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
index 5a14f9775b0e..4c2d0b3c6dad 100644
--- a/drivers/leds/leds-max77650.c
+++ b/drivers/leds/leds-max77650.c
@@ -62,7 +62,7 @@ static int max77650_led_brightness_set(struct led_classdev *cdev,
 
 static int max77650_led_probe(struct platform_device *pdev)
 {
-	struct device_node *of_node, *child;
+	struct fwnode_handle *child;
 	struct max77650_led *leds, *led;
 	struct device *dev;
 	struct regmap *map;
@@ -71,10 +71,6 @@ static int max77650_led_probe(struct platform_device *pdev)
 	u32 reg;
 
 	dev = &pdev->dev;
-	of_node = dev->of_node;
-
-	if (!of_node)
-		return -ENODEV;
 
 	leds = devm_kcalloc(dev, sizeof(*leds),
 			    MAX77650_LED_NUM_LEDS, GFP_KERNEL);
@@ -85,12 +81,12 @@ static int max77650_led_probe(struct platform_device *pdev)
 	if (!map)
 		return -ENODEV;
 
-	num_leds = of_get_child_count(of_node);
+	num_leds = device_get_child_node_count(dev);
 	if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS)
 		return -ENODEV;
 
-	for_each_child_of_node(of_node, child) {
-		rv = of_property_read_u32(child, "reg", &reg);
+	device_for_each_child_node(dev, child) {
+		rv = fwnode_property_read_u32(child, "reg", &reg);
 		if (rv || reg >= MAX77650_LED_NUM_LEDS) {
 			rv = -EINVAL;
 			goto err_node_put;
@@ -103,8 +99,8 @@ 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;
 
-		label = of_get_property(child, "label", NULL);
-		if (!label) {
+		rv = fwnode_property_read_string(child, "label", &label);
+		if (rv) {
 			led->cdev.name = "max77650::";
 		} else {
 			led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
@@ -115,8 +111,8 @@ static int max77650_led_probe(struct platform_device *pdev)
 			}
 		}
 
-		of_property_read_string(child, "linux,default-trigger",
-					&led->cdev.default_trigger);
+		fwnode_property_read_string(child, "linux,default-trigger",
+					    &led->cdev.default_trigger);
 
 		rv = devm_led_classdev_register(dev, &led->cdev);
 		if (rv)
@@ -135,7 +131,7 @@ static int max77650_led_probe(struct platform_device *pdev)
 			    MAX77650_REG_CNFG_LED_TOP,
 			    MAX77650_LED_TOP_DEFAULT);
 err_node_put:
-	of_node_put(child);
+	fwnode_handle_put(child);
 	return rv;
 }
 
-- 
2.23.0.rc1

[PATCH v2 2/2] leds: Switch to use fwnode instead of be stuck with OF one

[Andy Shevchenko]

There is no need to be stuck with OF node when we may use agnostic
firmware node instead.

It allows users to get property if needed independently of provider.

Note, some OF parts are left because %pfw [1] is in progress.

[1]: https://lore.kernel.org/patchwork/cover/1054863/

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/leds/led-class.c | 4 ++--
 drivers/leds/led-core.c  | 9 +++------
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
index d231240c2047..052c7571e188 100644
--- a/drivers/leds/led-class.c
+++ b/drivers/leds/led-class.c
@@ -14,7 +14,7 @@
 #include <linux/leds.h>
 #include <linux/list.h>
 #include <linux/module.h>
-#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/timer.h>
@@ -277,7 +277,7 @@ int led_classdev_register_ext(struct device *parent,
 		return PTR_ERR(led_cdev->dev);
 	}
 	if (init_data && init_data->fwnode)
-		led_cdev->dev->of_node = to_of_node(init_data->fwnode);
+		led_cdev->dev->fwnode = init_data->fwnode;
 
 	if (ret)
 		dev_warn(parent, "Led %s renamed to %s due to name collision",
diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
index f0c1c403f678..f1f718dbe0f8 100644
--- a/drivers/leds/led-core.c
+++ b/drivers/leds/led-core.c
@@ -324,14 +324,11 @@ EXPORT_SYMBOL_GPL(led_update_brightness);
 
 u32 *led_get_default_pattern(struct led_classdev *led_cdev, unsigned int *size)
 {
-	struct device_node *np = dev_of_node(led_cdev->dev);
+	struct fwnode_handle *fwnode = led_cdev->dev->fwnode;
 	u32 *pattern;
 	int count;
 
-	if (!np)
-		return NULL;
-
-	count = of_property_count_u32_elems(np, "led-pattern");
+	count = fwnode_property_count_u32(fwnode, "led-pattern");
 	if (count < 0)
 		return NULL;
 
@@ -339,7 +336,7 @@ u32 *led_get_default_pattern(struct led_classdev *led_cdev, unsigned int *size)
 	if (!pattern)
 		return NULL;
 
-	if (of_property_read_u32_array(np, "led-pattern", pattern, count)) {
+	if (fwnode_property_read_u32_array(fwnode, "led-pattern", pattern, count)) {
 		kfree(pattern);
 		return NULL;
 	}
-- 
2.23.0.rc1

Re: [PATCH v2 2/2] leds: Switch to use fwnode instead of be stuck with OF one

[Jacek Anaszewski]

Hi Andy,

Thank you for the patch. I have one issue below.

On 8/22/19 5:19 PM, Andy Shevchenko wrote:
> There is no need to be stuck with OF node when we may use agnostic
> firmware node instead.
> 
> It allows users to get property if needed independently of provider.
> 
> Note, some OF parts are left because %pfw [1] is in progress.
> 
> [1]: https://lore.kernel.org/patchwork/cover/1054863/
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/leds/led-class.c | 4 ++--
>  drivers/leds/led-core.c  | 9 +++------
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/leds/led-class.c b/drivers/leds/led-class.c
> index d231240c2047..052c7571e188 100644
> --- a/drivers/leds/led-class.c
> +++ b/drivers/leds/led-class.c
> @@ -14,7 +14,7 @@
>  #include <linux/leds.h>
>  #include <linux/list.h>
>  #include <linux/module.h>
> -#include <linux/of.h>
> +#include <linux/property.h>
>  #include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/timer.h>
> @@ -277,7 +277,7 @@ int led_classdev_register_ext(struct device *parent,
>  		return PTR_ERR(led_cdev->dev);
>  	}
>  	if (init_data && init_data->fwnode)
> -		led_cdev->dev->of_node = to_of_node(init_data->fwnode);
> +		led_cdev->dev->fwnode = init_data->fwnode;
>  
>  	if (ret)
>  		dev_warn(parent, "Led %s renamed to %s due to name collision",
> diff --git a/drivers/leds/led-core.c b/drivers/leds/led-core.c
> index f0c1c403f678..f1f718dbe0f8 100644
> --- a/drivers/leds/led-core.c
> +++ b/drivers/leds/led-core.c
> @@ -324,14 +324,11 @@ EXPORT_SYMBOL_GPL(led_update_brightness);
>  
>  u32 *led_get_default_pattern(struct led_classdev *led_cdev, unsigned int *size)
>  {
> -	struct device_node *np = dev_of_node(led_cdev->dev);
> +	struct fwnode_handle *fwnode = led_cdev->dev->fwnode;
>  	u32 *pattern;
>  	int count;
>  
> -	if (!np)
> -		return NULL;

Why aren't you checking fwnode for being not NULL?
It is not guaranteed to be always initialized.

> -
> -	count = of_property_count_u32_elems(np, "led-pattern");
> +	count = fwnode_property_count_u32(fwnode, "led-pattern");
>  	if (count < 0)
>  		return NULL;
>  
> @@ -339,7 +336,7 @@ u32 *led_get_default_pattern(struct led_classdev *led_cdev, unsigned int *size)
>  	if (!pattern)
>  		return NULL;
>  
> -	if (of_property_read_u32_array(np, "led-pattern", pattern, count)) {
> +	if (fwnode_property_read_u32_array(fwnode, "led-pattern", pattern, count)) {
>  		kfree(pattern);
>  		return NULL;
>  	}
> 

-- 
Best regards,
Jacek Anaszewski

Re: [PATCH v2 2/2] leds: Switch to use fwnode instead of be stuck with OF one

[Andy Shevchenko]

On Fri, Aug 23, 2019 at 08:14:49PM +0200, Jacek Anaszewski wrote:
> On 8/22/19 5:19 PM, Andy Shevchenko wrote:

> > -	if (!np)
> > -		return NULL;
> 
> Why aren't you checking fwnode for being not NULL?
> It is not guaranteed to be always initialized.

And this is handled by the below. Even in the original code the check was
redundant. Note, led_parse_fwnode_props() is written with the same redundancy.

> > -	count = of_property_count_u32_elems(np, "led-pattern");
> > +	count = fwnode_property_count_u32(fwnode, "led-pattern");
> >  	if (count < 0)
> >  		return NULL;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] leds: Switch to use fwnode instead of be stuck with OF one

[Jacek Anaszewski]

On 8/23/19 10:36 PM, Andy Shevchenko wrote:
> On Fri, Aug 23, 2019 at 08:14:49PM +0200, Jacek Anaszewski wrote:
>> On 8/22/19 5:19 PM, Andy Shevchenko wrote:
> 
>>> -	if (!np)
>>> -		return NULL;
>>
>> Why aren't you checking fwnode for being not NULL?
>> It is not guaranteed to be always initialized.
> 
> And this is handled by the below. Even in the original code the check was
> redundant. Note, led_parse_fwnode_props() is written with the same redundancy.

Indeed. Patch set applied, thanks.

>>> -	count = of_property_count_u32_elems(np, "led-pattern");
>>> +	count = fwnode_property_count_u32(fwnode, "led-pattern");
>>>  	if (count < 0)
>>>  		return NULL;
> 

-- 
Best regards,
Jacek Anaszewski