| Message ID | 20220325200338.54270-2-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [v1,1/5] gpiolib: Introduce gpiochip_count() helper | expand |
Hi Andy, On 25/03/2022 21:03, Andy Shevchenko wrote: > Since we have generic function to count GPIO controller nodes > under given device, there is no need to open code it. Replace > custom code by gpiochip_count() call. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/stm32/pinctrl-stm32.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c > index 9ed764731570..d4bbeec82c1f 100644 > --- a/drivers/pinctrl/stm32/pinctrl-stm32.c > +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c > @@ -1423,7 +1423,8 @@ int stm32_pctl_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct stm32_pinctrl *pctl; > struct pinctrl_pin_desc *pins; > - int i, ret, hwlock_id, banks = 0; > + int i, ret, hwlock_id; > + unsigned int banks; > > if (!np) > return -EINVAL; > @@ -1513,10 +1514,7 @@ int stm32_pctl_probe(struct platform_device *pdev) > return PTR_ERR(pctl->pctl_dev); > } > > - for_each_available_child_of_node(np, child) Here we look for "available" child, while the new generic helper gpiochip_count() looks for any child, available or not. Would it be possible to hav gpiochip_count() looking for available child as well? It looks like there is '_available_' version of 'device_for_each_child_node', maybe this shall be added too. > - if (of_property_read_bool(child, "gpio-controller")) > - banks++; > - > + banks = gpiochip_count(dev); > if (!banks) { > dev_err(dev, "at least one GPIO bank is required\n"); > return -EINVAL; Fabien
On Tue, Mar 29, 2022 at 09:59:32AM +0200, Fabien DESSENNE wrote: > On 25/03/2022 21:03, Andy Shevchenko wrote: Thanks for review, my answers below. > > - for_each_available_child_of_node(np, child) > > Here we look for "available" child, while the new generic helper > gpiochip_count() looks for any child, available or not. > Would it be possible to hav gpiochip_count() looking for available child as > well? It's done already that way. The fwnode loop is done against available children. > It looks like there is '_available_' version of > 'device_for_each_child_node', maybe this shall be added too. No need.
Hi Andy Thank you for your the clarification. On 25/03/2022 21:03, Andy Shevchenko wrote: > Since we have generic function to count GPIO controller nodes > under given device, there is no need to open code it. Replace > custom code by gpiochip_count() call. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/pinctrl/stm32/pinctrl-stm32.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c > index 9ed764731570..d4bbeec82c1f 100644 > --- a/drivers/pinctrl/stm32/pinctrl-stm32.c > +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c > @@ -1423,7 +1423,8 @@ int stm32_pctl_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct stm32_pinctrl *pctl; > struct pinctrl_pin_desc *pins; > - int i, ret, hwlock_id, banks = 0; > + int i, ret, hwlock_id; > + unsigned int banks; > > if (!np) > return -EINVAL; > @@ -1513,10 +1514,7 @@ int stm32_pctl_probe(struct platform_device *pdev) > return PTR_ERR(pctl->pctl_dev); > } > > - for_each_available_child_of_node(np, child) > - if (of_property_read_bool(child, "gpio-controller")) > - banks++; > - > + banks = gpiochip_count(dev); > if (!banks) { > dev_err(dev, "at least one GPIO bank is required\n"); > return -EINVAL; Reviewed-by: Fabien Dessenne <fabien.dessenne@foss.st.com>
On Tue, Mar 29, 2022 at 02:07:01PM +0200, Fabien DESSENNE wrote: > Hi Andy > > Thank you for your the clarification. > Reviewed-by: Fabien Dessenne <fabien.dessenne@foss.st.com> Thanks! In v2 I'm going to add another patch and the first (against gpiolib) will be split to two. This patch will be almost unchanged: I've decided to rename gpiochip_count() to gpiochip_node_count(), otherwise it's the same. So, I'll keep your tag.
diff --git a/drivers/pinctrl/stm32/pinctrl-stm32.c b/drivers/pinctrl/stm32/pinctrl-stm32.c index 9ed764731570..d4bbeec82c1f 100644 --- a/drivers/pinctrl/stm32/pinctrl-stm32.c +++ b/drivers/pinctrl/stm32/pinctrl-stm32.c @@ -1423,7 +1423,8 @@ int stm32_pctl_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct stm32_pinctrl *pctl; struct pinctrl_pin_desc *pins; - int i, ret, hwlock_id, banks = 0; + int i, ret, hwlock_id; + unsigned int banks; if (!np) return -EINVAL; @@ -1513,10 +1514,7 @@ int stm32_pctl_probe(struct platform_device *pdev) return PTR_ERR(pctl->pctl_dev); } - for_each_available_child_of_node(np, child) - if (of_property_read_bool(child, "gpio-controller")) - banks++; - + banks = gpiochip_count(dev); if (!banks) { dev_err(dev, "at least one GPIO bank is required\n"); return -EINVAL;
Since we have generic function to count GPIO controller nodes under given device, there is no need to open code it. Replace custom code by gpiochip_count() call. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/pinctrl/stm32/pinctrl-stm32.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)