Message ID | 20181004201747.56861-1-brandon.maier@rockwellcollins.com |
---|---|
State | New |
Headers | show |
Series | gpio: Ignore gpio names with empty strings | expand |
On Thu, Oct 4, 2018 at 10:18 PM Brandon Maier <brandon.maier@rockwellcollins.com> wrote: > If a gpiochip sets it's gpio_chip->names, it must give every gpio a > unique name. However we may not need to name all gpios, for example if > they are unused. gpio-line-names allows this by ignoring gpios with > empty names. But gpiochip_set_desc_names() will yell at us, as the empty > name will collide with other empty names. Fix this by skipping empty > strings, so that they look like unnamed gpios. > > Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com> Isn't the real problem that they should be passing NULL rather than "" empty string for line names? Yours, Linus Walleij
On Wed, Oct 10, 2018 at 4:12 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > Isn't the real problem that they should be passing NULL > rather than "" empty string for line names? In our case, we are using the gpio_chip->names in place of gpio-line-names. Because Xilinx's drivers/gpio/gpio-xilinx allocates multiple gpio_chips under a single devicetree node, gpio-line-names won't work. This is the same problem mentioned in Hedges Alexander's port of gpio-xilinx[1]. I worked around this by adding a gpio-line-names-2 that the second chip grabs and passes in through gpio_chip->names. This patch seemed appropriate because the intent of an "empty string" in gpio-line-names is to be an unset gpio name. It would also prevent name collisions from happening if we added name collision detection to gpio-line-names. But if it's better to leave this as-is, I could modify our code to replace "" -> NULL in gpio-xilinx. [1] https://www.spinics.net/lists/linux-gpio/msg31773.html Thanks, Brandon Maier
On Wed, Oct 10, 2018 at 5:03 PM Brandon Maier <brandon.maier@rockwellcollins.com> wrote: > On Wed, Oct 10, 2018 at 4:12 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > > > Isn't the real problem that they should be passing NULL > > rather than "" empty string for line names? > > In our case, we are using the gpio_chip->names in place of > gpio-line-names. Because Xilinx's drivers/gpio/gpio-xilinx allocates > multiple gpio_chips under a single devicetree node, gpio-line-names > won't work. This is the same problem mentioned in Hedges Alexander's > port of gpio-xilinx[1]. I worked around this by adding a > gpio-line-names-2 that the second chip grabs and passes in through > gpio_chip->names. > > This patch seemed appropriate because the intent of an "empty string" > in gpio-line-names is to be an unset gpio name. It would also prevent > name collisions from happening if we added name collision detection to > gpio-line-names. But if it's better to leave this as-is, I could > modify our code to replace "" -> NULL in gpio-xilinx. I think it is better to modify callers to pass NULL rather than "" to fix this. The reason is that it is very clear for a reader of the code that you do not want to name a line if it is just NULL. It has a certain elegance to it. (IMO) Yours, Linus Walleij
diff --git a/drivers/gpio/gpiolib-devprop.c b/drivers/gpio/gpiolib-devprop.c index dd517098ab95..2c56a0acd98b 100644 --- a/drivers/gpio/gpiolib-devprop.c +++ b/drivers/gpio/gpiolib-devprop.c @@ -52,7 +52,8 @@ void devprop_gpiochip_set_names(struct gpio_chip *chip, } for (i = 0; i < count; i++) - gdev->descs[i].name = names[i]; + if (names[i] && names[i][0]) + gdev->descs[i].name = names[i]; kfree(names); } diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 86c147b3f4af..7d359e560d81 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -341,7 +341,8 @@ static int gpiochip_set_desc_names(struct gpio_chip *gc) /* Then add all names to the GPIO descriptors */ for (i = 0; i != gc->ngpio; ++i) - gdev->descs[i].name = gc->names[i]; + if (gc->names[i] && gc->names[i][0]) + gdev->descs[i].name = gc->names[i]; return 0; }
If a gpiochip sets it's gpio_chip->names, it must give every gpio a unique name. However we may not need to name all gpios, for example if they are unused. gpio-line-names allows this by ignoring gpios with empty names. But gpiochip_set_desc_names() will yell at us, as the empty name will collide with other empty names. Fix this by skipping empty strings, so that they look like unnamed gpios. Signed-off-by: Brandon Maier <brandon.maier@rockwellcollins.com> --- drivers/gpio/gpiolib-devprop.c | 3 ++- drivers/gpio/gpiolib.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-)