Patchwork [v2,2/2] as3645a: Switch to fwnode property API

login
register
mail settings
Submitter Sakari Ailus
Date Feb. 25, 2019, 12:23 p.m.
Message ID <20190225122341.3407-3-sakari.ailus@linux.intel.com>
Download mbox | patch
Permalink /patch/735043/
State New
Headers show

Comments

Sakari Ailus - Feb. 25, 2019, 12:23 p.m.
Switch the as3645a from OF to the fwnode property API. Also add ACPI
support.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/leds/leds-as3645a.c | 93 +++++++++++++++++++++++++--------------------
 1 file changed, 52 insertions(+), 41 deletions(-)
Jacek Anaszewski - Feb. 27, 2019, 7:19 p.m.
Hi Sakari,

I was to queue it up for pushing upstream but noticed one trivial
issue when taking the last glance. In effect I'll target it for 5.2.

Please refer below.

On 2/25/19 1:23 PM, Sakari Ailus wrote:
> Switch the as3645a from OF to the fwnode property API. Also add ACPI
> support.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>   drivers/leds/leds-as3645a.c | 93 +++++++++++++++++++++++++--------------------
>   1 file changed, 52 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
> index 98a69b1a43f9..f44854a8e88e 100644
> --- a/drivers/leds/leds-as3645a.c
> +++ b/drivers/leds/leds-as3645a.c
> @@ -25,7 +25,7 @@
>   #include <linux/leds.h>
>   #include <linux/module.h>
>   #include <linux/mutex.h>
> -#include <linux/of.h>
> +#include <linux/property.h>
>   #include <linux/slab.h>
>   
>   #include <media/v4l2-flash-led-class.h>
> @@ -148,8 +148,8 @@ struct as3645a {
>   	struct v4l2_flash *vf;
>   	struct v4l2_flash *vfind;
>   
> -	struct device_node *flash_node;
> -	struct device_node *indicator_node;
> +	struct fwnode_handle *flash_node;
> +	struct fwnode_handle *indicator_node;
>   
>   	struct as3645a_config cfg;
>   
> @@ -493,30 +493,31 @@ static int as3645a_detect(struct as3645a *flash)
>   
>   static int as3645a_parse_node(struct as3645a *flash,
>   			      struct as3645a_names *names,
> -			      struct device_node *node)
> +			      struct fwnode_handle *fwnode)
>   {
>   	struct as3645a_config *cfg = &flash->cfg;
> -	struct device_node *child;
> +	struct fwnode_handle *child;
>   	const char *name;
>   	int rval;
>   
> -	for_each_child_of_node(node, child) {
> +	fwnode_for_each_child_node(fwnode, child) {
>   		u32 id = 0;
>   
> -		of_property_read_u32(child, "reg", &id);
> +		fwnode_property_read_u32(child, "reg", &id);
>   
>   		switch (id) {
>   		case AS_LED_FLASH:
> -			flash->flash_node = of_node_get(child);
> +			flash->flash_node = child;
>   			break;
>   		case AS_LED_INDICATOR:
> -			flash->indicator_node = of_node_get(child);
> +			flash->indicator_node = child;
>   			break;
>   		default:
>   			dev_warn(&flash->client->dev,
>   				 "unknown LED %u encountered, ignoring\n", id);
>   			break;
>   		}
> +		fwnode_handle_get(child);
>   	}
>   
>   	if (!flash->flash_node) {
> @@ -524,42 +525,46 @@ static int as3645a_parse_node(struct as3645a *flash,
>   		return -ENODEV;
>   	}
>   
> -	rval = of_property_read_string(flash->flash_node, "label", &name);
> -	if (!rval)
> +	rval = fwnode_property_read_string(flash->flash_node, "label", &name);
> +	if (!rval) {
>   		strlcpy(names->flash, name, sizeof(names->flash));
> -	else
> +	} else if (is_of_node(fwnode)) {
>   		snprintf(names->flash, sizeof(names->flash),
> -			 "%pOFn:flash", node);
> +			 "%pOFn:flash", to_of_node(fwnode));
> +	} else {
> +		dev_err(&flash->client->dev, "flash node has no label!\n");
> +		return -EINVAL;
> +	}
>   
> -	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
> -				    &cfg->flash_timeout_us);
> +	rval = fwnode_property_read_u32(flash->flash_node, "flash-timeout-us",
> +					&cfg->flash_timeout_us);
>   	if (rval < 0) {
>   		dev_err(&flash->client->dev,
>   			"can't read flash-timeout-us property for flash\n");
>   		goto out_err;
>   	}
>   
> -	rval = of_property_read_u32(flash->flash_node, "flash-max-microamp",
> -				    &cfg->flash_max_ua);
> +	rval = fwnode_property_read_u32(flash->flash_node, "flash-max-microamp",
> +					&cfg->flash_max_ua);
>   	if (rval < 0) {
>   		dev_err(&flash->client->dev,
>   			"can't read flash-max-microamp property for flash\n");
>   		goto out_err;
>   	}
>   
> -	rval = of_property_read_u32(flash->flash_node, "led-max-microamp",
> -				    &cfg->assist_max_ua);
> +	rval = fwnode_property_read_u32(flash->flash_node, "led-max-microamp",
> +					&cfg->assist_max_ua);
>   	if (rval < 0) {
>   		dev_err(&flash->client->dev,
>   			"can't read led-max-microamp property for flash\n");
>   		goto out_err;
>   	}
>   
> -	of_property_read_u32(flash->flash_node, "voltage-reference",
> -			     &cfg->voltage_reference);
> +	fwnode_property_read_u32(flash->flash_node, "voltage-reference",
> +				 &cfg->voltage_reference);
>   
> -	of_property_read_u32(flash->flash_node, "ams,input-max-microamp",
> -			     &cfg->peak);
> +	fwnode_property_read_u32(flash->flash_node, "ams,input-max-microamp",
> +				 &cfg->peak);
>   	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
>   
>   	if (!flash->indicator_node) {
> @@ -568,15 +573,21 @@ static int as3645a_parse_node(struct as3645a *flash,
>   		goto out_err;
>   	}
>   
> -	rval = of_property_read_string(flash->indicator_node, "label", &name);
> -	if (!rval)
> +	rval = fwnode_property_read_string(flash->indicator_node, "label",
> +					   &name);
> +	if (!rval) {
>   		strlcpy(names->indicator, name, sizeof(names->indicator));
> -	else
> +	} else if (is_of_node(fwnode)) {
>   		snprintf(names->indicator, sizeof(names->indicator),
> -			 "%pOFn:indicator", node);
> +			 "%pOFn:indicator", to_of_node(fwnode));
> +	} else {
> +		dev_err(&flash->client->dev, "flash node has no label!\n");
> +		return -EINVAL;
> +	}

s/flash node/indicator node/

If you agree, I can amend it myself.

Best regards,
Jacek Anaszewski

> -	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
> -				    &cfg->indicator_max_ua);
> +	rval = fwnode_property_read_u32(flash->indicator_node,
> +					"led-max-microamp",
> +					&cfg->indicator_max_ua);
>   	if (rval < 0) {
>   		dev_err(&flash->client->dev,
>   			"can't read led-max-microamp property for indicator\n");
> @@ -586,8 +597,8 @@ static int as3645a_parse_node(struct as3645a *flash,
>   	return 0;
>   
>   out_err:
> -	of_node_put(flash->flash_node);
> -	of_node_put(flash->indicator_node);
> +	fwnode_handle_put(flash->flash_node);
> +	fwnode_handle_put(flash->indicator_node);
>   
>   	return rval;
>   }
> @@ -668,14 +679,14 @@ static int as3645a_v4l2_setup(struct as3645a *flash)
>   	strlcpy(cfgind.dev_name, flash->iled_cdev.name, sizeof(cfg.dev_name));
>   
>   	flash->vf = v4l2_flash_init(
> -		&flash->client->dev, of_fwnode_handle(flash->flash_node),
> -		&flash->fled, NULL, &cfg);
> +		&flash->client->dev, flash->flash_node, &flash->fled, NULL,
> +		&cfg);
>   	if (IS_ERR(flash->vf))
>   		return PTR_ERR(flash->vf);
>   
>   	flash->vfind = v4l2_flash_indicator_init(
> -		&flash->client->dev, of_fwnode_handle(flash->indicator_node),
> -		&flash->iled_cdev, &cfgind);
> +		&flash->client->dev, flash->indicator_node, &flash->iled_cdev,
> +		&cfgind);
>   	if (IS_ERR(flash->vfind)) {
>   		v4l2_flash_release(flash->vf);
>   		return PTR_ERR(flash->vfind);
> @@ -690,7 +701,7 @@ static int as3645a_probe(struct i2c_client *client)
>   	struct as3645a *flash;
>   	int rval;
>   
> -	if (client->dev.of_node == NULL)
> +	if (!dev_fwnode(&client->dev))
>   		return -ENODEV;
>   
>   	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
> @@ -699,7 +710,7 @@ static int as3645a_probe(struct i2c_client *client)
>   
>   	flash->client = client;
>   
> -	rval = as3645a_parse_node(flash, &names, client->dev.of_node);
> +	rval = as3645a_parse_node(flash, &names, dev_fwnode(&client->dev));
>   	if (rval < 0)
>   		return rval;
>   
> @@ -731,8 +742,8 @@ static int as3645a_probe(struct i2c_client *client)
>   	mutex_destroy(&flash->mutex);
>   
>   out_put_nodes:
> -	of_node_put(flash->flash_node);
> -	of_node_put(flash->indicator_node);
> +	fwnode_handle_put(flash->flash_node);
> +	fwnode_handle_put(flash->indicator_node);
>   
>   	return rval;
>   }
> @@ -751,8 +762,8 @@ static int as3645a_remove(struct i2c_client *client)
>   
>   	mutex_destroy(&flash->mutex);
>   
> -	of_node_put(flash->flash_node);
> -	of_node_put(flash->indicator_node);
> +	fwnode_handle_put(flash->flash_node);
> +	fwnode_handle_put(flash->indicator_node);
>   
>   	return 0;
>   }
>
Sakari Ailus - March 1, 2019, 9:43 a.m.
Hi Jacek,

On Wed, Feb 27, 2019 at 08:19:14PM +0100, Jacek Anaszewski wrote:
> > @@ -568,15 +573,21 @@ static int as3645a_parse_node(struct as3645a *flash,
> >   		goto out_err;
> >   	}
> > -	rval = of_property_read_string(flash->indicator_node, "label", &name);
> > -	if (!rval)
> > +	rval = fwnode_property_read_string(flash->indicator_node, "label",
> > +					   &name);
> > +	if (!rval) {
> >   		strlcpy(names->indicator, name, sizeof(names->indicator));
> > -	else
> > +	} else if (is_of_node(fwnode)) {
> >   		snprintf(names->indicator, sizeof(names->indicator),
> > -			 "%pOFn:indicator", node);
> > +			 "%pOFn:indicator", to_of_node(fwnode));
> > +	} else {
> > +		dev_err(&flash->client->dev, "flash node has no label!\n");
> > +		return -EINVAL;
> > +	}
> 
> s/flash node/indicator node/
> 
> If you agree, I can amend it myself.

Yes, please do. Thanks!!
Jacek Anaszewski - March 1, 2019, 8:30 p.m.
Hi Sakari,

On 3/1/19 10:43 AM, Sakari Ailus wrote:
> Hi Jacek,
> 
> On Wed, Feb 27, 2019 at 08:19:14PM +0100, Jacek Anaszewski wrote:
>>> @@ -568,15 +573,21 @@ static int as3645a_parse_node(struct as3645a *flash,
>>>    		goto out_err;
>>>    	}
>>> -	rval = of_property_read_string(flash->indicator_node, "label", &name);
>>> -	if (!rval)
>>> +	rval = fwnode_property_read_string(flash->indicator_node, "label",
>>> +					   &name);
>>> +	if (!rval) {
>>>    		strlcpy(names->indicator, name, sizeof(names->indicator));
>>> -	else
>>> +	} else if (is_of_node(fwnode)) {
>>>    		snprintf(names->indicator, sizeof(names->indicator),
>>> -			 "%pOFn:indicator", node);
>>> +			 "%pOFn:indicator", to_of_node(fwnode));
>>> +	} else {
>>> +		dev_err(&flash->client->dev, "flash node has no label!\n");
>>> +		return -EINVAL;
>>> +	}
>>
>> s/flash node/indicator node/
>>
>> If you agree, I can amend it myself.
> 
> Yes, please do. Thanks!!

Amended as above and applied to the for-5.2 branch of linux-leds.git, 
thanks.

Patch

diff --git a/drivers/leds/leds-as3645a.c b/drivers/leds/leds-as3645a.c
index 98a69b1a43f9..f44854a8e88e 100644
--- a/drivers/leds/leds-as3645a.c
+++ b/drivers/leds/leds-as3645a.c
@@ -25,7 +25,7 @@ 
 #include <linux/leds.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
-#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/slab.h>
 
 #include <media/v4l2-flash-led-class.h>
@@ -148,8 +148,8 @@  struct as3645a {
 	struct v4l2_flash *vf;
 	struct v4l2_flash *vfind;
 
-	struct device_node *flash_node;
-	struct device_node *indicator_node;
+	struct fwnode_handle *flash_node;
+	struct fwnode_handle *indicator_node;
 
 	struct as3645a_config cfg;
 
@@ -493,30 +493,31 @@  static int as3645a_detect(struct as3645a *flash)
 
 static int as3645a_parse_node(struct as3645a *flash,
 			      struct as3645a_names *names,
-			      struct device_node *node)
+			      struct fwnode_handle *fwnode)
 {
 	struct as3645a_config *cfg = &flash->cfg;
-	struct device_node *child;
+	struct fwnode_handle *child;
 	const char *name;
 	int rval;
 
-	for_each_child_of_node(node, child) {
+	fwnode_for_each_child_node(fwnode, child) {
 		u32 id = 0;
 
-		of_property_read_u32(child, "reg", &id);
+		fwnode_property_read_u32(child, "reg", &id);
 
 		switch (id) {
 		case AS_LED_FLASH:
-			flash->flash_node = of_node_get(child);
+			flash->flash_node = child;
 			break;
 		case AS_LED_INDICATOR:
-			flash->indicator_node = of_node_get(child);
+			flash->indicator_node = child;
 			break;
 		default:
 			dev_warn(&flash->client->dev,
 				 "unknown LED %u encountered, ignoring\n", id);
 			break;
 		}
+		fwnode_handle_get(child);
 	}
 
 	if (!flash->flash_node) {
@@ -524,42 +525,46 @@  static int as3645a_parse_node(struct as3645a *flash,
 		return -ENODEV;
 	}
 
-	rval = of_property_read_string(flash->flash_node, "label", &name);
-	if (!rval)
+	rval = fwnode_property_read_string(flash->flash_node, "label", &name);
+	if (!rval) {
 		strlcpy(names->flash, name, sizeof(names->flash));
-	else
+	} else if (is_of_node(fwnode)) {
 		snprintf(names->flash, sizeof(names->flash),
-			 "%pOFn:flash", node);
+			 "%pOFn:flash", to_of_node(fwnode));
+	} else {
+		dev_err(&flash->client->dev, "flash node has no label!\n");
+		return -EINVAL;
+	}
 
-	rval = of_property_read_u32(flash->flash_node, "flash-timeout-us",
-				    &cfg->flash_timeout_us);
+	rval = fwnode_property_read_u32(flash->flash_node, "flash-timeout-us",
+					&cfg->flash_timeout_us);
 	if (rval < 0) {
 		dev_err(&flash->client->dev,
 			"can't read flash-timeout-us property for flash\n");
 		goto out_err;
 	}
 
-	rval = of_property_read_u32(flash->flash_node, "flash-max-microamp",
-				    &cfg->flash_max_ua);
+	rval = fwnode_property_read_u32(flash->flash_node, "flash-max-microamp",
+					&cfg->flash_max_ua);
 	if (rval < 0) {
 		dev_err(&flash->client->dev,
 			"can't read flash-max-microamp property for flash\n");
 		goto out_err;
 	}
 
-	rval = of_property_read_u32(flash->flash_node, "led-max-microamp",
-				    &cfg->assist_max_ua);
+	rval = fwnode_property_read_u32(flash->flash_node, "led-max-microamp",
+					&cfg->assist_max_ua);
 	if (rval < 0) {
 		dev_err(&flash->client->dev,
 			"can't read led-max-microamp property for flash\n");
 		goto out_err;
 	}
 
-	of_property_read_u32(flash->flash_node, "voltage-reference",
-			     &cfg->voltage_reference);
+	fwnode_property_read_u32(flash->flash_node, "voltage-reference",
+				 &cfg->voltage_reference);
 
-	of_property_read_u32(flash->flash_node, "ams,input-max-microamp",
-			     &cfg->peak);
+	fwnode_property_read_u32(flash->flash_node, "ams,input-max-microamp",
+				 &cfg->peak);
 	cfg->peak = AS_PEAK_mA_TO_REG(cfg->peak);
 
 	if (!flash->indicator_node) {
@@ -568,15 +573,21 @@  static int as3645a_parse_node(struct as3645a *flash,
 		goto out_err;
 	}
 
-	rval = of_property_read_string(flash->indicator_node, "label", &name);
-	if (!rval)
+	rval = fwnode_property_read_string(flash->indicator_node, "label",
+					   &name);
+	if (!rval) {
 		strlcpy(names->indicator, name, sizeof(names->indicator));
-	else
+	} else if (is_of_node(fwnode)) {
 		snprintf(names->indicator, sizeof(names->indicator),
-			 "%pOFn:indicator", node);
+			 "%pOFn:indicator", to_of_node(fwnode));
+	} else {
+		dev_err(&flash->client->dev, "flash node has no label!\n");
+		return -EINVAL;
+	}
 
-	rval = of_property_read_u32(flash->indicator_node, "led-max-microamp",
-				    &cfg->indicator_max_ua);
+	rval = fwnode_property_read_u32(flash->indicator_node,
+					"led-max-microamp",
+					&cfg->indicator_max_ua);
 	if (rval < 0) {
 		dev_err(&flash->client->dev,
 			"can't read led-max-microamp property for indicator\n");
@@ -586,8 +597,8 @@  static int as3645a_parse_node(struct as3645a *flash,
 	return 0;
 
 out_err:
-	of_node_put(flash->flash_node);
-	of_node_put(flash->indicator_node);
+	fwnode_handle_put(flash->flash_node);
+	fwnode_handle_put(flash->indicator_node);
 
 	return rval;
 }
@@ -668,14 +679,14 @@  static int as3645a_v4l2_setup(struct as3645a *flash)
 	strlcpy(cfgind.dev_name, flash->iled_cdev.name, sizeof(cfg.dev_name));
 
 	flash->vf = v4l2_flash_init(
-		&flash->client->dev, of_fwnode_handle(flash->flash_node),
-		&flash->fled, NULL, &cfg);
+		&flash->client->dev, flash->flash_node, &flash->fled, NULL,
+		&cfg);
 	if (IS_ERR(flash->vf))
 		return PTR_ERR(flash->vf);
 
 	flash->vfind = v4l2_flash_indicator_init(
-		&flash->client->dev, of_fwnode_handle(flash->indicator_node),
-		&flash->iled_cdev, &cfgind);
+		&flash->client->dev, flash->indicator_node, &flash->iled_cdev,
+		&cfgind);
 	if (IS_ERR(flash->vfind)) {
 		v4l2_flash_release(flash->vf);
 		return PTR_ERR(flash->vfind);
@@ -690,7 +701,7 @@  static int as3645a_probe(struct i2c_client *client)
 	struct as3645a *flash;
 	int rval;
 
-	if (client->dev.of_node == NULL)
+	if (!dev_fwnode(&client->dev))
 		return -ENODEV;
 
 	flash = devm_kzalloc(&client->dev, sizeof(*flash), GFP_KERNEL);
@@ -699,7 +710,7 @@  static int as3645a_probe(struct i2c_client *client)
 
 	flash->client = client;
 
-	rval = as3645a_parse_node(flash, &names, client->dev.of_node);
+	rval = as3645a_parse_node(flash, &names, dev_fwnode(&client->dev));
 	if (rval < 0)
 		return rval;
 
@@ -731,8 +742,8 @@  static int as3645a_probe(struct i2c_client *client)
 	mutex_destroy(&flash->mutex);
 
 out_put_nodes:
-	of_node_put(flash->flash_node);
-	of_node_put(flash->indicator_node);
+	fwnode_handle_put(flash->flash_node);
+	fwnode_handle_put(flash->indicator_node);
 
 	return rval;
 }
@@ -751,8 +762,8 @@  static int as3645a_remove(struct i2c_client *client)
 
 	mutex_destroy(&flash->mutex);
 
-	of_node_put(flash->flash_node);
-	of_node_put(flash->indicator_node);
+	fwnode_handle_put(flash->flash_node);
+	fwnode_handle_put(flash->indicator_node);
 
 	return 0;
 }