| Message ID | 20220329152926.50958-8-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | gpiolib: Two new helpers and way toward fwnode | expand |
Hi Andy, On Tue, Mar 29, 2022 at 5:29 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > Switch the code to use for_each_gpiochip_node() helper. > > While at it, in order to avoid additional churn in the future, > switch to fwnode APIs where it makes sense. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Thanks for your patch! > --- a/drivers/pinctrl/renesas/pinctrl-rza1.c > +++ b/drivers/pinctrl/renesas/pinctrl-rza1.c > @@ -1166,17 +1167,17 @@ static const struct pinmux_ops rza1_pinmux_ops = { > * @range: pin range to register to pinctrl core > */ > static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl, > - struct device_node *np, > + struct fwnode_handle *fwnode, > struct gpio_chip *chip, > struct pinctrl_gpio_range *range) > { > const char *list_name = "gpio-ranges"; > - struct of_phandle_args of_args; > + struct fwnode_reference_args of_args; fw_args? > unsigned int gpioport; > u32 pinctrl_base; > int ret; > > - ret = of_parse_phandle_with_fixed_args(np, list_name, 3, 0, &of_args); > + ret = fwnode_property_get_reference_args(fwnode, list_name, NULL, 3, 0, &of_args); > if (ret) { > dev_err(rza1_pctl->dev, "Unable to parse %s list property\n", > list_name); > @@ -1197,13 +1198,12 @@ static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl, > > *chip = rza1_gpiochip_template; > chip->base = -1; > - chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pOFn", > - np); > + chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pfw", fwnode); This changes the label from e.g. "/soc/pinctrl@fcfe3000/gpio-11" to "gpio-11". %pfwP? > if (!chip->label) > return -ENOMEM; > > chip->ngpio = of_args.args[2]; > - chip->of_node = np; > + chip->fwnode = fwnode; > chip->parent = rza1_pctl->dev; > > range->id = gpioport; With the above fixed: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Wed, Mar 30, 2022 at 12:00:27PM +0200, Geert Uytterhoeven wrote: > On Tue, Mar 29, 2022 at 5:29 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: ... > > + struct fwnode_reference_args of_args; > > fw_args? Perhaps just args as other drivers do? ... > > - chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pOFn", > > - np); > > + chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pfw", fwnode); > > This changes the label from e.g. "/soc/pinctrl@fcfe3000/gpio-11" to "gpio-11". Hmm... It seems other way around, i.e. it changes from the name to full name. > %pfwP? But conclusion here is correct. Good catch! ... > With the above fixed: > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> > Acked-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks for review and testing!
Hi Andy, On Wed, Mar 30, 2022 at 2:17 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Wed, Mar 30, 2022 at 12:00:27PM +0200, Geert Uytterhoeven wrote: > > On Tue, Mar 29, 2022 at 5:29 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > ... > > > > + struct fwnode_reference_args of_args; > > > > fw_args? > > Perhaps just args as other drivers do? Fine for me. > > > - chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pOFn", > > > - np); > > > + chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pfw", fwnode); > > > > This changes the label from e.g. "/soc/pinctrl@fcfe3000/gpio-11" to "gpio-11". > > Hmm... It seems other way around, i.e. it changes from the name to full name. Oops, sorry about that. (I accidentally included the change from testing "%pfwP" ;-) > > > %pfwP? > > But conclusion here is correct. Good catch! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
diff --git a/drivers/pinctrl/renesas/pinctrl-rza1.c b/drivers/pinctrl/renesas/pinctrl-rza1.c index 5075d5cebe8c..74183f36567b 100644 --- a/drivers/pinctrl/renesas/pinctrl-rza1.c +++ b/drivers/pinctrl/renesas/pinctrl-rza1.c @@ -24,6 +24,7 @@ #include <linux/pinctrl/pinconf-generic.h> #include <linux/pinctrl/pinctrl.h> #include <linux/pinctrl/pinmux.h> +#include <linux/property.h> #include <linux/slab.h> #include "../core.h" @@ -1166,17 +1167,17 @@ static const struct pinmux_ops rza1_pinmux_ops = { * @range: pin range to register to pinctrl core */ static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl, - struct device_node *np, + struct fwnode_handle *fwnode, struct gpio_chip *chip, struct pinctrl_gpio_range *range) { const char *list_name = "gpio-ranges"; - struct of_phandle_args of_args; + struct fwnode_reference_args of_args; unsigned int gpioport; u32 pinctrl_base; int ret; - ret = of_parse_phandle_with_fixed_args(np, list_name, 3, 0, &of_args); + ret = fwnode_property_get_reference_args(fwnode, list_name, NULL, 3, 0, &of_args); if (ret) { dev_err(rza1_pctl->dev, "Unable to parse %s list property\n", list_name); @@ -1197,13 +1198,12 @@ static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl, *chip = rza1_gpiochip_template; chip->base = -1; - chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pOFn", - np); + chip->label = devm_kasprintf(rza1_pctl->dev, GFP_KERNEL, "%pfw", fwnode); if (!chip->label) return -ENOMEM; chip->ngpio = of_args.args[2]; - chip->of_node = np; + chip->fwnode = fwnode; chip->parent = rza1_pctl->dev; range->id = gpioport; @@ -1232,10 +1232,9 @@ static int rza1_parse_gpiochip(struct rza1_pinctrl *rza1_pctl, */ static int rza1_gpio_register(struct rza1_pinctrl *rza1_pctl) { - struct device_node *np = rza1_pctl->dev->of_node; struct pinctrl_gpio_range *gpio_ranges; struct gpio_chip *gpio_chips; - struct device_node *child; + struct fwnode_handle *child; unsigned int ngpiochips; unsigned int i; int ret; @@ -1254,14 +1253,11 @@ static int rza1_gpio_register(struct rza1_pinctrl *rza1_pctl) return -ENOMEM; i = 0; - for_each_child_of_node(np, child) { - if (!of_property_read_bool(child, "gpio-controller")) - continue; - + for_each_gpiochip_node(rza1_pctl->dev, child) { ret = rza1_parse_gpiochip(rza1_pctl, child, &gpio_chips[i], &gpio_ranges[i]); if (ret) { - of_node_put(child); + fwnode_handle_put(child); return ret; }
Switch the code to use for_each_gpiochip_node() helper. While at it, in order to avoid additional churn in the future, switch to fwnode APIs where it makes sense. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pinctrl/renesas/pinctrl-rza1.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-)