The gpiochip_count() helper iterates over the device child nodes that have
the "gpio-controller" property set. It returns the number of such nodes
under given device.
Signed-off-by: Andy Shevchenko <[email protected]>
---
include/linux/gpio/driver.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h
index 894eab753fdf..52918ef5d288 100644
--- a/include/linux/gpio/driver.h
+++ b/include/linux/gpio/driver.h
@@ -9,6 +9,7 @@
#include <linux/lockdep.h>
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/property.h>
#include <linux/types.h>
struct gpio_desc;
@@ -750,4 +751,17 @@ static inline void gpiochip_unlock_as_irq(struct gpio_chip *gc,
}
#endif /* CONFIG_GPIOLIB */
+static inline unsigned int gpiochip_count(struct device *dev)
+{
+ struct fwnode_handle *child;
+ unsigned int count = 0;
+
+ device_for_each_child_node(dev, child) {
+ if (device_property_read_bool(child, "gpio-controller"))
+ count++;
+ }
+
+ return count;
+}
+
#endif /* __LINUX_GPIO_DRIVER_H */
--
2.35.1
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 <[email protected]>
---
drivers/pinctrl/renesas/pinctrl-rza1.c | 17 +----------------
1 file changed, 1 insertion(+), 16 deletions(-)
diff --git a/drivers/pinctrl/renesas/pinctrl-rza1.c b/drivers/pinctrl/renesas/pinctrl-rza1.c
index c1d6e9512c7a..c66e72e7dd71 100644
--- a/drivers/pinctrl/renesas/pinctrl-rza1.c
+++ b/drivers/pinctrl/renesas/pinctrl-rza1.c
@@ -1154,21 +1154,6 @@ static const struct pinmux_ops rza1_pinmux_ops = {
* RZ/A1 pin controller driver operations
*/
-static unsigned int rza1_count_gpio_chips(struct device_node *np)
-{
- struct device_node *child;
- unsigned int count = 0;
-
- for_each_child_of_node(np, child) {
- if (!of_property_read_bool(child, "gpio-controller"))
- continue;
-
- count++;
- }
-
- return count;
-}
-
/**
* rza1_parse_gpiochip() - parse and register a gpio chip and pin range
*
@@ -1255,7 +1240,7 @@ static int rza1_gpio_register(struct rza1_pinctrl *rza1_pctl)
unsigned int i;
int ret;
- ngpiochips = rza1_count_gpio_chips(np);
+ ngpiochips = gpiochip_count(rza1_pctl->dev);
if (ngpiochips == 0) {
dev_dbg(rza1_pctl->dev, "No gpiochip registered\n");
return 0;
--
2.35.1
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 <[email protected]>
---
drivers/pinctrl/meson/pinctrl-meson.c | 28 ++++++++++++---------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
index 49851444a6e3..7db70d4e214a 100644
--- a/drivers/pinctrl/meson/pinctrl-meson.c
+++ b/drivers/pinctrl/meson/pinctrl-meson.c
@@ -49,6 +49,7 @@
#include <linux/pinctrl/pinctrl.h>
#include <linux/pinctrl/pinmux.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
#include <linux/regmap.h>
#include <linux/seq_file.h>
@@ -662,27 +663,22 @@ static struct regmap *meson_map_resource(struct meson_pinctrl *pc,
return devm_regmap_init_mmio(pc->dev, base, &meson_regmap_config);
}
-static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
- struct device_node *node)
+static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc)
{
- struct device_node *np, *gpio_np = NULL;
+ struct device_node *gpio_np;
+ unsigned int chips;
- for_each_child_of_node(node, np) {
- if (!of_find_property(np, "gpio-controller", NULL))
- continue;
- if (gpio_np) {
- dev_err(pc->dev, "multiple gpio nodes\n");
- of_node_put(np);
- return -EINVAL;
- }
- gpio_np = np;
- }
-
- if (!gpio_np) {
+ chips = gpiochip_count(pc->dev);
+ if (!chips) {
dev_err(pc->dev, "no gpio node found\n");
return -EINVAL;
}
+ if (chips > 1) {
+ dev_err(pc->dev, "multiple gpio nodes\n");
+ return -EINVAL;
+ }
+ gpio_np = to_of_node(device_get_named_child_node(pc->dev, "gpio-controller"));
pc->of_node = gpio_np;
pc->reg_mux = meson_map_resource(pc, gpio_np, "mux");
@@ -751,7 +747,7 @@ int meson_pinctrl_probe(struct platform_device *pdev)
pc->dev = dev;
pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
- ret = meson_pinctrl_parse_dt(pc, dev->of_node);
+ ret = meson_pinctrl_parse_dt(pc);
if (ret)
return ret;
--
2.35.1
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 <[email protected]>
---
drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 24 +++++++++------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
index 08cad14042e2..ba94125f6566 100644
--- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
+++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
@@ -728,22 +728,18 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
struct gpio_irq_chip *girq = &gc->irq;
struct device *dev = &pdev->dev;
struct device_node *np;
- int ret = -ENODEV, i, nr_irq_parent;
+ unsigned int nr_child_nodes, i;
+ int ret;
/* Check if we have at least one gpio-controller child node */
- for_each_child_of_node(dev->of_node, np) {
- if (of_property_read_bool(np, "gpio-controller")) {
- ret = 0;
- break;
- }
- }
- if (ret)
- return dev_err_probe(dev, ret, "no gpio-controller child node\n");
+ nr_child_nodes = gpiochip_count(dev);
+ if (!nr_child_nodes)
+ return dev_err_probe(dev, -ENODEV, "no gpio-controller child node\n");
- nr_irq_parent = of_irq_count(np);
spin_lock_init(&info->irq_lock);
- if (!nr_irq_parent) {
+ nr_child_nodes = of_irq_count(np);
+ if (!nr_child_nodes) {
dev_err(dev, "invalid or no IRQ\n");
return 0;
}
@@ -766,11 +762,11 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
* controller. But we do not take advantage of this and use
* the chained irq with all of them.
*/
- girq->num_parents = nr_irq_parent;
- girq->parents = devm_kcalloc(dev, nr_irq_parent, sizeof(*girq->parents), GFP_KERNEL);
+ girq->num_parents = nr_child_nodes;
+ girq->parents = devm_kcalloc(dev, nr_child_nodes, sizeof(*girq->parents), GFP_KERNEL);
if (!girq->parents)
return -ENOMEM;
- for (i = 0; i < nr_irq_parent; i++) {
+ for (i = 0; i < nr_child_nodes; i++) {
int irq = irq_of_parse_and_map(np, i);
if (irq < 0)
--
2.35.1
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 <[email protected]>
---
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;
--
2.35.1
Hello!
On 3/25/22 11:03 PM, 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 <[email protected]>
> ---
> drivers/pinctrl/mvebu/pinctrl-armada-37xx.c | 24 +++++++++------------
> 1 file changed, 10 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> index 08cad14042e2..ba94125f6566 100644
> --- a/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> +++ b/drivers/pinctrl/mvebu/pinctrl-armada-37xx.c
> @@ -728,22 +728,18 @@ static int armada_37xx_irqchip_register(struct platform_device *pdev,
> struct gpio_irq_chip *girq = &gc->irq;
> struct device *dev = &pdev->dev;
> struct device_node *np;
> - int ret = -ENODEV, i, nr_irq_parent;
> + unsigned int nr_child_nodes, i;
> + int ret;
>
> /* Check if we have at least one gpio-controller child node */
> - for_each_child_of_node(dev->of_node, np) {
> - if (of_property_read_bool(np, "gpio-controller")) {
> - ret = 0;
> - break;
> - }
> - }
> - if (ret)
> - return dev_err_probe(dev, ret, "no gpio-controller child node\n");
> + nr_child_nodes = gpiochip_count(dev);
> + if (!nr_child_nodes)
> + return dev_err_probe(dev, -ENODEV, "no gpio-controller child node\n");
>
> - nr_irq_parent = of_irq_count(np);
> spin_lock_init(&info->irq_lock);
>
> - if (!nr_irq_parent) {
> + nr_child_nodes = of_irq_count(np);
Mhm, 'np' is no longer assigned to at this point...
> + if (!nr_child_nodes) {
> dev_err(dev, "invalid or no IRQ\n");
> return 0;
> }
[...]
MBR, Sergey
On Sat, Mar 26, 2022 at 7:02 PM Sergey Shtylyov <[email protected]> wrote:
> On 3/25/22 11:03 PM, Andy Shevchenko wrote:
>
> > Since we have a generic function to count GPIO controller nodes
> > under a given device, there is no need to open-code it. Replace
> > custom code by gpiochip_count() call.
...
> > + nr_child_nodes = of_irq_count(np);
>
> Mhm, 'np' is no longer assigned to it at this point...
Good catch! We may retrieve it by calling
np = to_of_node(device_get_named_child_node(dev, "gpio-controller"));
like it's done in the previous patch in the series.
--
With Best Regards,
Andy Shevchenko
Hi Andy,
On Fri, Mar 25, 2022 at 9:04 PM Andy Shevchenko
<[email protected]> wrote:
> The gpiochip_count() helper iterates over the device child nodes that have
> the "gpio-controller" property set. It returns the number of such nodes
> under given device.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
Thanks for your patch!
> --- a/include/linux/gpio/driver.h
> +++ b/include/linux/gpio/driver.h
> @@ -750,4 +751,17 @@ static inline void gpiochip_unlock_as_irq(struct gpio_chip *gc,
> }
> #endif /* CONFIG_GPIOLIB */
>
> +static inline unsigned int gpiochip_count(struct device *dev)
> +{
> + struct fwnode_handle *child;
> + unsigned int count = 0;
> +
> + device_for_each_child_node(dev, child) {
> + if (device_property_read_bool(child, "gpio-controller"))
error: passing argument 1 of ‘device_property_read_bool’ from
incompatible pointer type [-Werror=incompatible-pointer-types]
So I'm afraid I cannot test patch 3/5 yet ;-)
> + count++;
> + }
> +
> + return count;
> +}
> +
> #endif /* __LINUX_GPIO_DRIVER_H */
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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 Mon, Mar 28, 2022 at 09:35:25AM +0200, Geert Uytterhoeven wrote:
> On Fri, Mar 25, 2022 at 9:04 PM Andy Shevchenko
> <[email protected]> wrote:
...
> > + if (device_property_read_bool(child, "gpio-controller"))
>
> error: passing argument 1 of ‘device_property_read_bool’ from
> incompatible pointer type [-Werror=incompatible-pointer-types]
Oh, thanks! I will fix this in v2.
> So I'm afraid I cannot test patch 3/5 yet ;-)
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
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.
--
With Best Regards,
Andy Shevchenko
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.
--
With Best Regards,
Andy Shevchenko
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 <[email protected]>
> ---
> 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 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 <[email protected]>
> ---
> drivers/pinctrl/meson/pinctrl-meson.c | 28 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/pinctrl/meson/pinctrl-meson.c b/drivers/pinctrl/meson/pinctrl-meson.c
> index 49851444a6e3..7db70d4e214a 100644
> --- a/drivers/pinctrl/meson/pinctrl-meson.c
> +++ b/drivers/pinctrl/meson/pinctrl-meson.c
> @@ -49,6 +49,7 @@
> #include <linux/pinctrl/pinctrl.h>
> #include <linux/pinctrl/pinmux.h>
> #include <linux/platform_device.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/seq_file.h>
>
> @@ -662,27 +663,22 @@ static struct regmap *meson_map_resource(struct meson_pinctrl *pc,
> return devm_regmap_init_mmio(pc->dev, base, &meson_regmap_config);
> }
>
> -static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc,
> - struct device_node *node)
> +static int meson_pinctrl_parse_dt(struct meson_pinctrl *pc)
> {
> - struct device_node *np, *gpio_np = NULL;
> + struct device_node *gpio_np;
> + unsigned int chips;
>
> - for_each_child_of_node(node, np) {
> - if (!of_find_property(np, "gpio-controller", NULL))
> - continue;
> - if (gpio_np) {
> - dev_err(pc->dev, "multiple gpio nodes\n");
> - of_node_put(np);
> - return -EINVAL;
> - }
> - gpio_np = np;
> - }
> -
> - if (!gpio_np) {
> + chips = gpiochip_count(pc->dev);
> + if (!chips) {
> dev_err(pc->dev, "no gpio node found\n");
> return -EINVAL;
> }
> + if (chips > 1) {
> + dev_err(pc->dev, "multiple gpio nodes\n");
> + return -EINVAL;
> + }
>
> + gpio_np = to_of_node(device_get_named_child_node(pc->dev, "gpio-controller"));
> pc->of_node = gpio_np;
>
> pc->reg_mux = meson_map_resource(pc, gpio_np, "mux");
> @@ -751,7 +747,7 @@ int meson_pinctrl_probe(struct platform_device *pdev)
> pc->dev = dev;
> pc->data = (struct meson_pinctrl_data *) of_device_get_match_data(dev);
>
> - ret = meson_pinctrl_parse_dt(pc, dev->of_node);
> + ret = meson_pinctrl_parse_dt(pc);
> if (ret)
> return ret;
>
Reviewed-by: Neil Armstrong <[email protected]>
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 <[email protected]>
> ---
> 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 <[email protected]>