diff mbox series

[v2,1/2] gpio: of: Extract of_gpiochip_add_hog()

Message ID 20200220130149.26283-2-geert+renesas@glider.be (mailing list archive)
State Mainlined
Commit bc21077e084b80c22072a40d32beb24ea33938ec
Delegated to: Geert Uytterhoeven
Headers show
Series gpio: of: Add DT overlay support for GPIO hogs | expand

Commit Message

Geert Uytterhoeven Feb. 20, 2020, 1:01 p.m. UTC
Extract the code to add all GPIO hogs of a gpio-hog node into its own
function, so it can be reused.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/gpio/gpiolib-of.c | 49 ++++++++++++++++++++++++++-------------
 1 file changed, 33 insertions(+), 16 deletions(-)

Comments

Frank Rowand Feb. 20, 2020, 6:50 p.m. UTC | #1
On 2/20/20 7:01 AM, Geert Uytterhoeven wrote:
> Extract the code to add all GPIO hogs of a gpio-hog node into its own
> function, so it can be reused.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - No changes.
> ---
>  drivers/gpio/gpiolib-of.c | 49 ++++++++++++++++++++++++++-------------
>  1 file changed, 33 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
> index c6d30f73df078e0b..2b47f93886075294 100644
> --- a/drivers/gpio/gpiolib-of.c
> +++ b/drivers/gpio/gpiolib-of.c
> @@ -604,6 +604,35 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
>  	return desc;
>  }
>  
> +/**
> + * of_gpiochip_add_hog - Add all hogs in a hog device node
> + * @chip:	gpio chip to act on
> + * @hog:	device node describing the hogs
> + *
> + * Returns error if it fails otherwise 0 on success.
> + */
> +static int of_gpiochip_add_hog(struct gpio_chip *chip, struct device_node *hog)
> +{
> +	enum gpiod_flags dflags;
> +	struct gpio_desc *desc;
> +	unsigned long lflags;
> +	const char *name;
> +	unsigned int i;
> +	int ret;
> +
> +	for (i = 0;; i++) {
> +		desc = of_parse_own_gpio(hog, chip, i, &name, &lflags, &dflags);
> +		if (IS_ERR(desc))
> +			break;
> +
> +		ret = gpiod_hog(desc, name, lflags, dflags);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * of_gpiochip_scan_gpios - Scan gpio-controller for gpio definitions
>   * @chip:	gpio chip to act on
> @@ -614,29 +643,17 @@ static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
>   */
>  static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
>  {
> -	struct gpio_desc *desc = NULL;
>  	struct device_node *np;
> -	const char *name;
> -	unsigned long lflags;
> -	enum gpiod_flags dflags;
> -	unsigned int i;
>  	int ret;
>  
>  	for_each_available_child_of_node(chip->of_node, np) {
>  		if (!of_property_read_bool(np, "gpio-hog"))
>  			continue;
>  
> -		for (i = 0;; i++) {
> -			desc = of_parse_own_gpio(np, chip, i, &name, &lflags,
> -						 &dflags);
> -			if (IS_ERR(desc))
> -				break;
> -
> -			ret = gpiod_hog(desc, name, lflags, dflags);
> -			if (ret < 0) {
> -				of_node_put(np);
> -				return ret;
> -			}
> +		ret = of_gpiochip_add_hog(chip, np);
> +		if (ret < 0) {
> +			of_node_put(np);
> +			return ret;
>  		}
>  	}
>  
> 

Reviewed-by: Frank Rowand <frank.rowand@sony.com>
Linus Walleij Feb. 21, 2020, 4:08 p.m. UTC | #2
On Thu, Feb 20, 2020 at 2:01 PM Geert Uytterhoeven
<geert+renesas@glider.be> wrote:

> Extract the code to add all GPIO hogs of a gpio-hog node into its own
> function, so it can be reused.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> v2:
>   - No changes.

Patch applied with Frank's Review tag.

Yours,
Linus Walleij
Frank Rowand Feb. 21, 2020, 5:18 p.m. UTC | #3
Hi Linus, Rob,

On 2/21/20 10:08 AM, Linus Walleij wrote:
> On Thu, Feb 20, 2020 at 2:01 PM Geert Uytterhoeven
> <geert+renesas@glider.be> wrote:
> 
>> Extract the code to add all GPIO hogs of a gpio-hog node into its own
>> function, so it can be reused.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>> v2:
>>   - No changes.
> 
> Patch applied with Frank's Review tag.

I created a devicetree unittest to show the problem that Geert's patches
fix.

I would prefer to have my unittest patch series applied somewhere,
immediately followed by Geert's patch series.  This way, after
applying my series, a test fail is reported, then after Geert's
series is applied, the test fail becomes a test pass.

Can you coordinate with Rob to accept both series either via
your tree or Rob's tree?

-Frank

> 
> Yours,
> Linus Walleij
>
Frank Rowand Feb. 21, 2020, 5:20 p.m. UTC | #4
On 2/21/20 11:18 AM, Frank Rowand wrote:
> Hi Linus, Rob,
> 
> On 2/21/20 10:08 AM, Linus Walleij wrote:
>> On Thu, Feb 20, 2020 at 2:01 PM Geert Uytterhoeven
>> <geert+renesas@glider.be> wrote:
>>
>>> Extract the code to add all GPIO hogs of a gpio-hog node into its own
>>> function, so it can be reused.
>>>
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>> v2:
>>>   - No changes.
>>
>> Patch applied with Frank's Review tag.
> 
> I created a devicetree unittest to show the problem that Geert's patches
> fix.

I left out the link to my patch series:

   https://lore.kernel.org/linux-devicetree/1582224021-12827-1-git-send-email-frowand.list@gmail.com/

   [PATCH v2 0/2] of: unittest: add overlay gpio test to catch gpio hog problem

-Frank

> 
> I would prefer to have my unittest patch series applied somewhere,
> immediately followed by Geert's patch series.  This way, after
> applying my series, a test fail is reported, then after Geert's
> series is applied, the test fail becomes a test pass.
> 
> Can you coordinate with Rob to accept both series either via
> your tree or Rob's tree?
> 
> -Frank
> 
>>
>> Yours,
>> Linus Walleij
>>
>
Linus Walleij Feb. 28, 2020, 10:43 p.m. UTC | #5
On Fri, Feb 21, 2020 at 6:18 PM Frank Rowand <frowand.list@gmail.com> wrote:
> On 2/21/20 10:08 AM, Linus Walleij wrote:

> > Patch applied with Frank's Review tag.
>
> I created a devicetree unittest to show the problem that Geert's patches
> fix.
>
> I would prefer to have my unittest patch series applied somewhere,
> immediately followed by Geert's patch series.  This way, after
> applying my series, a test fail is reported, then after Geert's
> series is applied, the test fail becomes a test pass.
>
> Can you coordinate with Rob to accept both series either via
> your tree or Rob's tree?

I see Rob already applied the test.

I do not personally bother much about which order problems
get solved but I guess if Rob can back out the patch I can apply
it to my tree instead, before these patches. for some time,
before I start pulling stuff on top.

Yours,
Linus Walleij
Frank Rowand March 2, 2020, 7:57 p.m. UTC | #6
On 2/28/20 4:43 PM, Linus Walleij wrote:
> On Fri, Feb 21, 2020 at 6:18 PM Frank Rowand <frowand.list@gmail.com> wrote:
>> On 2/21/20 10:08 AM, Linus Walleij wrote:
> 
>>> Patch applied with Frank's Review tag.
>>
>> I created a devicetree unittest to show the problem that Geert's patches
>> fix.
>>
>> I would prefer to have my unittest patch series applied somewhere,
>> immediately followed by Geert's patch series.  This way, after
>> applying my series, a test fail is reported, then after Geert's
>> series is applied, the test fail becomes a test pass.
>>
>> Can you coordinate with Rob to accept both series either via
>> your tree or Rob's tree?
> 
> I see Rob already applied the test.
> 
> I do not personally bother much about which order problems
> get solved but I guess if Rob can back out the patch I can apply
> it to my tree instead, before these patches. for some time,
> before I start pulling stuff on top.
> 
> Yours,
> Linus Walleij
> 

At this point, if it is a lot of trouble or confusion, I would not
bother undoing the commits that have been done.  I'll leave it up
to you and Rob to coordinate if you do decide to undo.

-Frank
diff mbox series

Patch

diff --git a/drivers/gpio/gpiolib-of.c b/drivers/gpio/gpiolib-of.c
index c6d30f73df078e0b..2b47f93886075294 100644
--- a/drivers/gpio/gpiolib-of.c
+++ b/drivers/gpio/gpiolib-of.c
@@ -604,6 +604,35 @@  static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
 	return desc;
 }
 
+/**
+ * of_gpiochip_add_hog - Add all hogs in a hog device node
+ * @chip:	gpio chip to act on
+ * @hog:	device node describing the hogs
+ *
+ * Returns error if it fails otherwise 0 on success.
+ */
+static int of_gpiochip_add_hog(struct gpio_chip *chip, struct device_node *hog)
+{
+	enum gpiod_flags dflags;
+	struct gpio_desc *desc;
+	unsigned long lflags;
+	const char *name;
+	unsigned int i;
+	int ret;
+
+	for (i = 0;; i++) {
+		desc = of_parse_own_gpio(hog, chip, i, &name, &lflags, &dflags);
+		if (IS_ERR(desc))
+			break;
+
+		ret = gpiod_hog(desc, name, lflags, dflags);
+		if (ret < 0)
+			return ret;
+	}
+
+	return 0;
+}
+
 /**
  * of_gpiochip_scan_gpios - Scan gpio-controller for gpio definitions
  * @chip:	gpio chip to act on
@@ -614,29 +643,17 @@  static struct gpio_desc *of_parse_own_gpio(struct device_node *np,
  */
 static int of_gpiochip_scan_gpios(struct gpio_chip *chip)
 {
-	struct gpio_desc *desc = NULL;
 	struct device_node *np;
-	const char *name;
-	unsigned long lflags;
-	enum gpiod_flags dflags;
-	unsigned int i;
 	int ret;
 
 	for_each_available_child_of_node(chip->of_node, np) {
 		if (!of_property_read_bool(np, "gpio-hog"))
 			continue;
 
-		for (i = 0;; i++) {
-			desc = of_parse_own_gpio(np, chip, i, &name, &lflags,
-						 &dflags);
-			if (IS_ERR(desc))
-				break;
-
-			ret = gpiod_hog(desc, name, lflags, dflags);
-			if (ret < 0) {
-				of_node_put(np);
-				return ret;
-			}
+		ret = of_gpiochip_add_hog(chip, np);
+		if (ret < 0) {
+			of_node_put(np);
+			return ret;
 		}
 	}