[v3,01/14] gpio: regmap: set gpio_chip of_node
diff mbox series

Message ID 20210303142310.6371-2-noltari@gmail.com
State New, archived
Headers show
Series
  • pinctrl: add BCM63XX pincontrol support
Related show

Commit Message

Álvaro Fernández Rojas March 3, 2021, 2:22 p.m. UTC
This is needed for properly registering gpio regmap as a child of a regmap
pin controller.

Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
---
 v3: introduce patch needed for properly parsing gpio-ranges.

 drivers/gpio/gpio-regmap.c  | 1 +
 include/linux/gpio/regmap.h | 3 +++
 2 files changed, 4 insertions(+)

Comments

Linus Walleij March 3, 2021, 3:27 p.m. UTC | #1
On Wed, Mar 3, 2021 at 3:23 PM Álvaro Fernández Rojas <noltari@gmail.com> wrote:

> This is needed for properly registering gpio regmap as a child of a regmap
> pin controller.
>
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  v3: introduce patch needed for properly parsing gpio-ranges.

Oops a little bug. I suggest that I merge this into the pinctrl tree
together with the rest of the patches when we are done with review.

Yours.
Linus Walleij
Michael Walle March 3, 2021, 3:39 p.m. UTC | #2
Am 2021-03-03 16:27, schrieb Linus Walleij:
> On Wed, Mar 3, 2021 at 3:23 PM Álvaro Fernández Rojas 
> <noltari@gmail.com> wrote:
> 
>> This is needed for properly registering gpio regmap as a child of a 
>> regmap
>> pin controller.
>> 
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> ---
>>  v3: introduce patch needed for properly parsing gpio-ranges.
> 
> Oops a little bug. I suggest that I merge this into the pinctrl tree
> together with the rest of the patches when we are done with review.

Ha, I've just debugged this because it puzzled me why it was working
for me.

I was about to suggesting using the following instead:
chip->of_node = config->of_node ?: dev_of_node(config->parent);

It turns out this is already done in of_gpio_dev_init():
https://elixir.bootlin.com/linux/v5.12-rc1/source/drivers/gpio/gpiolib-of.c#L1043

So config->of_node is still optional. But I'm not sure if we
should add the line above for clarity in gpio-regmap.c.

-michael
Michael Walle March 3, 2021, 4:08 p.m. UTC | #3
Am 2021-03-03 15:22, schrieb Álvaro Fernández Rojas:
> This is needed for properly registering gpio regmap as a child of a 
> regmap
> pin controller.
> 
> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
> ---
>  v3: introduce patch needed for properly parsing gpio-ranges.
> 
>  drivers/gpio/gpio-regmap.c  | 1 +
>  include/linux/gpio/regmap.h | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
> index 5412cb3b0b2a..752ccd780b7d 100644
> --- a/drivers/gpio/gpio-regmap.c
> +++ b/drivers/gpio/gpio-regmap.c
> @@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const
> struct gpio_regmap_config *config
> 
>  	chip = &gpio->gpio_chip;
>  	chip->parent = config->parent;
> +	chip->of_node = config->of_node;

chip->of_node = config->of_node ?: dev_of_node(config->parent);

As mentioned in my previous reply in this thread, for clarity
reasons.

>  	chip->base = -1;
>  	chip->ngpio = config->ngpio;
>  	chip->names = config->names;
> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
> index ad76f3d0a6ba..f6e638e32d2a 100644
> --- a/include/linux/gpio/regmap.h
> +++ b/include/linux/gpio/regmap.h
> @@ -4,6 +4,7 @@
>  #define _LINUX_GPIO_REGMAP_H
> 
>  struct device;
> +struct device_node;
>  struct gpio_regmap;
>  struct irq_domain;
>  struct regmap;
> @@ -14,6 +15,7 @@ struct regmap;
>  /**
>   * struct gpio_regmap_config - Description of a generic regmap 
> gpio_chip.
>   * @parent:		The parent device
> + * @of_node:		The device node

Please add "(Optional)" and move it below @regmap. This should also
mention that if not supplied parent->of_node is used.

>   * @regmap:		The regmap used to access the registers
>   *			given, the name of the device is used
>   * @label:		(Optional) Descriptive name for GPIO controller.
> @@ -56,6 +58,7 @@ struct regmap;
>   */
>  struct gpio_regmap_config {
>  	struct device *parent;
> +	struct device_node *of_node;
>  	struct regmap *regmap;
> 
>  	const char *label;

With these changes:
Reviewed-by: Michael Walle <michael@walle.cc>

-michael
Álvaro Fernández Rojas March 3, 2021, 4:12 p.m. UTC | #4
Hi Linus,

Do you want me to send v4 with these changes?
Or maybe just this single patch?

Best regards,
Álvaro.

> El 3 mar 2021, a las 17:08, Michael Walle <michael@walle.cc> escribió:
> 
> Am 2021-03-03 15:22, schrieb Álvaro Fernández Rojas:
>> This is needed for properly registering gpio regmap as a child of a regmap
>> pin controller.
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> ---
>> v3: introduce patch needed for properly parsing gpio-ranges.
>> drivers/gpio/gpio-regmap.c  | 1 +
>> include/linux/gpio/regmap.h | 3 +++
>> 2 files changed, 4 insertions(+)
>> diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
>> index 5412cb3b0b2a..752ccd780b7d 100644
>> --- a/drivers/gpio/gpio-regmap.c
>> +++ b/drivers/gpio/gpio-regmap.c
>> @@ -249,6 +249,7 @@ struct gpio_regmap *gpio_regmap_register(const
>> struct gpio_regmap_config *config
>> 	chip = &gpio->gpio_chip;
>> 	chip->parent = config->parent;
>> +	chip->of_node = config->of_node;
> 
> chip->of_node = config->of_node ?: dev_of_node(config->parent);
> 
> As mentioned in my previous reply in this thread, for clarity
> reasons.
> 
>> 	chip->base = -1;
>> 	chip->ngpio = config->ngpio;
>> 	chip->names = config->names;
>> diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
>> index ad76f3d0a6ba..f6e638e32d2a 100644
>> --- a/include/linux/gpio/regmap.h
>> +++ b/include/linux/gpio/regmap.h
>> @@ -4,6 +4,7 @@
>> #define _LINUX_GPIO_REGMAP_H
>> struct device;
>> +struct device_node;
>> struct gpio_regmap;
>> struct irq_domain;
>> struct regmap;
>> @@ -14,6 +15,7 @@ struct regmap;
>> /**
>>  * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
>>  * @parent:		The parent device
>> + * @of_node:		The device node
> 
> Please add "(Optional)" and move it below @regmap. This should also
> mention that if not supplied parent->of_node is used.
> 
>>  * @regmap:		The regmap used to access the registers
>>  *			given, the name of the device is used
>>  * @label:		(Optional) Descriptive name for GPIO controller.
>> @@ -56,6 +58,7 @@ struct regmap;
>>  */
>> struct gpio_regmap_config {
>> 	struct device *parent;
>> +	struct device_node *of_node;
>> 	struct regmap *regmap;
>> 	const char *label;
> 
> With these changes:
> Reviewed-by: Michael Walle <michael@walle.cc>
> 
> -michael
Linus Walleij March 4, 2021, 8:13 a.m. UTC | #5
On Wed, Mar 3, 2021 at 5:12 PM Álvaro Fernández Rojas <noltari@gmail.com> wrote:

> Do you want me to send v4 with these changes?
> Or maybe just this single patch?

It's usually better to resend the series because then the b4 tool
will pick it all up properly.

Yours,
Linus Walleij
Álvaro Fernández Rojas March 4, 2021, 8:27 a.m. UTC | #6
Hi Linus,

> El 4 mar 2021, a las 9:13, Linus Walleij <linus.walleij@linaro.org> escribió:
> 
> On Wed, Mar 3, 2021 at 5:12 PM Álvaro Fernández Rojas <noltari@gmail.com> wrote:
> 
>> Do you want me to send v4 with these changes?
>> Or maybe just this single patch?
> 
> It's usually better to resend the series because then the b4 tool
> will pick it all up properly.

Ok, I didn’t know that :$.
Sorry for sending it as a separate patch.

> 
> Yours,
> Linus Walleij

Best regards,
Álvaro.

Patch
diff mbox series

diff --git a/drivers/gpio/gpio-regmap.c b/drivers/gpio/gpio-regmap.c
index 5412cb3b0b2a..752ccd780b7d 100644
--- a/drivers/gpio/gpio-regmap.c
+++ b/drivers/gpio/gpio-regmap.c
@@ -249,6 +249,7 @@  struct gpio_regmap *gpio_regmap_register(const struct gpio_regmap_config *config
 
 	chip = &gpio->gpio_chip;
 	chip->parent = config->parent;
+	chip->of_node = config->of_node;
 	chip->base = -1;
 	chip->ngpio = config->ngpio;
 	chip->names = config->names;
diff --git a/include/linux/gpio/regmap.h b/include/linux/gpio/regmap.h
index ad76f3d0a6ba..f6e638e32d2a 100644
--- a/include/linux/gpio/regmap.h
+++ b/include/linux/gpio/regmap.h
@@ -4,6 +4,7 @@ 
 #define _LINUX_GPIO_REGMAP_H
 
 struct device;
+struct device_node;
 struct gpio_regmap;
 struct irq_domain;
 struct regmap;
@@ -14,6 +15,7 @@  struct regmap;
 /**
  * struct gpio_regmap_config - Description of a generic regmap gpio_chip.
  * @parent:		The parent device
+ * @of_node:		The device node
  * @regmap:		The regmap used to access the registers
  *			given, the name of the device is used
  * @label:		(Optional) Descriptive name for GPIO controller.
@@ -56,6 +58,7 @@  struct regmap;
  */
 struct gpio_regmap_config {
 	struct device *parent;
+	struct device_node *of_node;
 	struct regmap *regmap;
 
 	const char *label;