[PATCH leds 0/2] leds: ns2: convert to fwnode API

[PATCH leds 0/2] leds: ns2: convert to fwnode API

[Marek Behún]

Hi Pavel,

Simon wants to bind this driver without device-tree, so this changes
the binding from specific OF API to generic fwnode API.

This applies to your for-next, but if you apply this, I will have to
respin my other series moving parsing of `linux,default-trigger` to
LED core.

Marek

Cc: Simon Guinot <simon.guinot@sequanux.org>

Marek Behún (2):
  leds: ns2: convert to fwnode API
  leds: ns2: do not guard OF match pointer with of_match_ptr

 drivers/leds/leds-ns2.c | 74 ++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 38 deletions(-)

-- 
2.26.2

[PATCH leds 1/2] leds: ns2: convert to fwnode API

[Marek Behún]

Convert from OF api to fwnode API, so that it is possible to bind this
driver without device-tree.

The fwnode API does not expose a function to read a specific element of
an array. We therefore change the types of the ns2_led_modval structure
so that we can read the whole modval array with one fwnode call.

Signed-off-by: Marek Behún <kabel@kernel.org>
Cc: Simon Guinot <simon.guinot@sequanux.org>
---
 drivers/leds/leds-ns2.c | 60 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 31 deletions(-)

diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 030f426af3d75..427b5059206a5 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -24,11 +24,16 @@ enum ns2_led_modes {
 	NS_V2_LED_SATA,
 };
 
+/*
+ * If the size of this structure or types of its members is changed,
+ * the filling of array modval in function ns2_led_register must be changed
+ * accordingly.
+ */
 struct ns2_led_modval {
-	enum ns2_led_modes	mode;
-	int			cmd_level;
-	int			slow_level;
-};
+	u32			mode;
+	u32			cmd_level;
+	u32			slow_level;
+} __packed;
 
 /*
  * The Network Space v2 dual-GPIO LED is wired to a CPLD. Three different LED
@@ -167,30 +172,32 @@ static struct attribute *ns2_led_attrs[] = {
 };
 ATTRIBUTE_GROUPS(ns2_led);
 
-static int ns2_led_register(struct device *dev, struct device_node *np,
+static int ns2_led_register(struct device *dev, struct fwnode_handle *node,
 			    struct ns2_led *led)
 {
 	struct led_init_data init_data = {};
 	struct ns2_led_modval *modval;
 	enum ns2_led_modes mode;
-	int nmodes, ret, i;
+	int nmodes, ret;
 
-	led->cmd = devm_gpiod_get_from_of_node(dev, np, "cmd-gpio", 0,
-					       GPIOD_ASIS, np->name);
+	led->cmd = devm_fwnode_gpiod_get_index(dev, node, "cmd-gpio", 0,
+					       GPIOD_ASIS,
+					       fwnode_get_name(node));
 	if (IS_ERR(led->cmd))
 		return PTR_ERR(led->cmd);
 
-	led->slow = devm_gpiod_get_from_of_node(dev, np, "slow-gpio", 0,
-						GPIOD_ASIS, np->name);
+	led->slow = devm_fwnode_gpiod_get_index(dev, node, "slow-gpio", 0,
+						GPIOD_ASIS,
+						fwnode_get_name(node));
 	if (IS_ERR(led->slow))
 		return PTR_ERR(led->slow);
 
-	of_property_read_string(np, "linux,default-trigger",
-				&led->cdev.default_trigger);
+	fwnode_property_read_string(node, "linux,default-trigger",
+				    &led->cdev.default_trigger);
 
-	ret = of_property_count_u32_elems(np, "modes-map");
+	ret = fwnode_property_count_u32(node, "modes-map");
 	if (ret < 0 || ret % 3) {
-		dev_err(dev, "Missing or malformed modes-map for %pOF\n", np);
+		dev_err(dev, "Missing or malformed modes-map for %pfw\n", node);
 		return -EINVAL;
 	}
 
@@ -199,16 +206,8 @@ static int ns2_led_register(struct device *dev, struct device_node *np,
 	if (!modval)
 		return -ENOMEM;
 
-	for (i = 0; i < nmodes; i++) {
-		u32 val;
-
-		of_property_read_u32_index(np, "modes-map", 3 * i, &val);
-		modval[i].mode = val;
-		of_property_read_u32_index(np, "modes-map", 3 * i + 1, &val);
-		modval[i].cmd_level = val;
-		of_property_read_u32_index(np, "modes-map", 3 * i + 2, &val);
-		modval[i].slow_level = val;
-	}
+	fwnode_property_read_u32_array(node, "modes-map", (void *)modval,
+				       nmodes * 3);
 
 	rwlock_init(&led->rw_lock);
 
@@ -231,11 +230,11 @@ static int ns2_led_register(struct device *dev, struct device_node *np,
 	led->sata = (mode == NS_V2_LED_SATA) ? 1 : 0;
 	led->cdev.brightness = (mode == NS_V2_LED_OFF) ? LED_OFF : LED_FULL;
 
-	init_data.fwnode = of_fwnode_handle(np);
+	init_data.fwnode = 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);
+		dev_err(dev, "Failed to register LED for node %pfw\n", node);
 
 	return ret;
 }
@@ -249,13 +248,12 @@ MODULE_DEVICE_TABLE(of, of_ns2_leds_match);
 static int ns2_led_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct device_node *np, *child;
+	struct fwnode_handle *child;
 	struct ns2_led *leds;
 	int count;
 	int ret;
 
-	np = dev_of_node(dev);
-	count = of_get_available_child_count(np);
+	count = device_get_child_node_count(dev);
 	if (!count)
 		return -ENODEV;
 
@@ -263,10 +261,10 @@ static int ns2_led_probe(struct platform_device *pdev)
 	if (!leds)
 		return -ENOMEM;
 
-	for_each_available_child_of_node(np, child) {
+	device_for_each_child_node(dev, child) {
 		ret = ns2_led_register(dev, child, leds++);
 		if (ret) {
-			of_node_put(child);
+			fwnode_handle_put(child);
 			return ret;
 		}
 	}
-- 
2.26.2

[PATCH leds 2/2] leds: ns2: do not guard OF match pointer with of_match_ptr

[Marek Behún]

Do not match OF match pointer with of_match_ptr, so that even if
CONFIG_OF is disabled, the driver can still be bound via another method.

Move definition of of_ns2_leds_match just before ns2_led_driver
definition, since it is not needed sooner.

Signed-off-by: Marek Behún <kabel@kernel.org>
Cc: Simon Guinot <simon.guinot@sequanux.org>
---
 drivers/leds/leds-ns2.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/leds/leds-ns2.c b/drivers/leds/leds-ns2.c
index 427b5059206a5..4ea6e61aacddf 100644
--- a/drivers/leds/leds-ns2.c
+++ b/drivers/leds/leds-ns2.c
@@ -239,12 +239,6 @@ static int ns2_led_register(struct device *dev, struct fwnode_handle *node,
 	return ret;
 }
 
-static const struct of_device_id of_ns2_leds_match[] = {
-	{ .compatible = "lacie,ns2-leds", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, of_ns2_leds_match);
-
 static int ns2_led_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -272,11 +266,17 @@ static int ns2_led_probe(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id of_ns2_leds_match[] = {
+	{ .compatible = "lacie,ns2-leds", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, of_ns2_leds_match);
+
 static struct platform_driver ns2_led_driver = {
 	.probe		= ns2_led_probe,
 	.driver		= {
 		.name		= "leds-ns2",
-		.of_match_table	= of_match_ptr(of_ns2_leds_match),
+		.of_match_table	= of_ns2_leds_match,
 	},
 };
 
-- 
2.26.2

Re: [PATCH leds 0/2] leds: ns2: convert to fwnode API

[Simon Guinot]

[-- Attachment #1: Type: text/plain, Size: 793 bytes --]

On Wed, Sep 23, 2020 at 04:18:38PM +0200, Marek Behún wrote:
> Hi Pavel,
> 
> Simon wants to bind this driver without device-tree, so this changes
> the binding from specific OF API to generic fwnode API.
> 
> This applies to your for-next, but if you apply this, I will have to
> respin my other series moving parsing of `linux,default-trigger` to
> LED core.
> 
> Marek
> 
> Cc: Simon Guinot <simon.guinot@sequanux.org>
> 
> Marek Behún (2):
>   leds: ns2: convert to fwnode API
>   leds: ns2: do not guard OF match pointer with of_match_ptr
> 
>  drivers/leds/leds-ns2.c | 74 ++++++++++++++++++++---------------------
>  1 file changed, 36 insertions(+), 38 deletions(-)
> 
> -- 
> 2.26.2

Hi Marek,

Thanks for this patches. I'll test them this week.

Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH leds 1/2] leds: ns2: convert to fwnode API

[Simon Guinot]

[-- Attachment #1: Type: text/plain, Size: 2046 bytes --]

On Wed, Sep 23, 2020 at 04:18:39PM +0200, Marek Behún wrote:
> Convert from OF api to fwnode API, so that it is possible to bind this
> driver without device-tree.
> 
> The fwnode API does not expose a function to read a specific element of
> an array. We therefore change the types of the ns2_led_modval structure
> so that we can read the whole modval array with one fwnode call.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> Cc: Simon Guinot <simon.guinot@sequanux.org>
> ---
>  drivers/leds/leds-ns2.c | 60 ++++++++++++++++++++---------------------
>  1 file changed, 29 insertions(+), 31 deletions(-)

...

> -static int ns2_led_register(struct device *dev, struct device_node *np,
> +static int ns2_led_register(struct device *dev, struct fwnode_handle *node,
>  			    struct ns2_led *led)
>  {
>  	struct led_init_data init_data = {};
>  	struct ns2_led_modval *modval;
>  	enum ns2_led_modes mode;
> -	int nmodes, ret, i;
> +	int nmodes, ret;
>  
> -	led->cmd = devm_gpiod_get_from_of_node(dev, np, "cmd-gpio", 0,
> -					       GPIOD_ASIS, np->name);
> +	led->cmd = devm_fwnode_gpiod_get_index(dev, node, "cmd-gpio", 0,
> +					       GPIOD_ASIS,
> +					       fwnode_get_name(node));
>  	if (IS_ERR(led->cmd))
>  		return PTR_ERR(led->cmd);
>  
> -	led->slow = devm_gpiod_get_from_of_node(dev, np, "slow-gpio", 0,
> -						GPIOD_ASIS, np->name);
> +	led->slow = devm_fwnode_gpiod_get_index(dev, node, "slow-gpio", 0,
> +						GPIOD_ASIS,
> +						fwnode_get_name(node));

Hi Marek,

You need to remove the "-gpio" suffix for the con_id parameter. It is
automatically and systematically appended in the fwnode_gpiod_get_index
function...

With this change, I can confirm that the led-ns2 driver is still working
using the DT path after applying the two fwnode patches (merged on the
top of the "linux,default-trigger" series). I tested it on a d2 Network
board.

I need a little bit more time to test the fwnode support on my x86
boards (with board setup files). 

Simon

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]