Messages in this thread | | | From | Linus Walleij <> | Date | Wed, 11 Mar 2020 16:32:29 +0100 | Subject | Re: [PATCH V2] gpio: brcmstb: support gpio-line-names property |
| |
On Mon, Mar 9, 2020 at 8:02 PM Doug Berger <opendmb@gmail.com> wrote:
> The default handling of the gpio-line-names property by the > gpiolib-of implementation does not work with the multiple > gpiochip banks per device structure used by the gpio-brcmstb > driver. > > This commit adds driver level support for the device tree > property so that GPIO lines can be assigned friendly names. >
> Signed-off-by: Doug Berger <opendmb@gmail.com> > +static void brcmstb_gpio_set_names(struct device *dev, > + struct brcmstb_gpio_bank *bank) > +{ > + struct device_node *np = dev->of_node; > + const char **names; > + int nstrings, base;
I don't understand why that thing is named "base".
> + unsigned int i; > + > + base = bank->id * MAX_GPIO_PER_BANK;
That would be ngpios or something.
But you alread have what you need in bank->gc.ngpio, right?
So why calculate it?
> + nstrings = of_property_count_strings(np, "gpio-line-names"); > + if (nstrings <= base) > + /* Line names not present */ > + return; > + > + names = devm_kcalloc(dev, MAX_GPIO_PER_BANK, sizeof(*names), > + GFP_KERNEL); > + if (!names) > + return; > + > + /* > + * Make sure to not index beyond the end of the number of descriptors > + * of the GPIO device. > + */ > + for (i = 0; i < bank->width; i++) { > + const char *name; > + int ret; > + > + ret = of_property_read_string_index(np, "gpio-line-names", > + base + i, &name); > + if (ret) { > + if (ret != -ENODATA) > + dev_err(dev, "unable to name line %d: %d\n", > + base + i, ret); > + break; > + } > + if (*name) > + names[i] = name; > + } > + > + bank->gc.names = names; > +}
Why can't you just make the function devprop_gpiochip_set_names() public, (line in <linux/gpio/driver.h>) and convert your np to a fwnode and call that &bank->gc ?
Yours, Linus Walleij
| |