Message ID | 20210727144816.8697-2-sergio.paracuellos@gmail.com |
---|---|
State | New |
Headers | show |
Series | gpiolib: convert 'devprop_gpiochip_set_names' to support multiple gpiochip per device | expand |
On Tue, Jul 27, 2021 at 5:48 PM Sergio Paracuellos <sergio.paracuellos@gmail.com> wrote: > > The default gpiolib-of implementation does not work with the multiple > gpiochip banks per device structure used for example by the gpio-mt7621 > and gpio-brcmstb drivers. To fix these kind of situations driver code > is forced to fill the names to avoid the gpiolib code to set names > repeated along the banks. Instead of continue with that antipattern > fix the gpiolib core function to get expected behaviour for every > single situation adding a field 'offset' in the gpiochip structure. > Doing in this way, we can assume this offset will be zero for normal > driver code where only one gpiochip bank per device is used but > can be set explicitly in those drivers that really need more than > one gpiochip. ... > + dev_warn(&gdev->dev, "gpio-line-names too short (length %d) " > + "cannot map names for the gpiochip at offset %u\n", Reflow this that string literal will be on one line (it's fine to be over even 100). > + count, chip->offset); -- With Best Regards, Andy Shevchenko
On Tue, Jul 27, 2021 at 5:05 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Tue, Jul 27, 2021 at 5:48 PM Sergio Paracuellos > <sergio.paracuellos@gmail.com> wrote: > > > > The default gpiolib-of implementation does not work with the multiple > > gpiochip banks per device structure used for example by the gpio-mt7621 > > and gpio-brcmstb drivers. To fix these kind of situations driver code > > is forced to fill the names to avoid the gpiolib code to set names > > repeated along the banks. Instead of continue with that antipattern > > fix the gpiolib core function to get expected behaviour for every > > single situation adding a field 'offset' in the gpiochip structure. > > Doing in this way, we can assume this offset will be zero for normal > > driver code where only one gpiochip bank per device is used but > > can be set explicitly in those drivers that really need more than > > one gpiochip. > > ... > > > + dev_warn(&gdev->dev, "gpio-line-names too short (length %d) " > > + "cannot map names for the gpiochip at offset %u\n", > > Reflow this that string literal will be on one line (it's fine to be > over even 100). Ah, ok. Will fix this and send v3. Thanks, Sergio Paracuellos > > > + count, chip->offset); > > -- > With Best Regards, > Andy Shevchenko
diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index 27c07108496d..bdd96891e848 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -382,10 +382,19 @@ static int devprop_gpiochip_set_names(struct gpio_chip *chip) if (count < 0) return 0; - if (count > gdev->ngpio) { - dev_warn(&gdev->dev, "gpio-line-names is length %d but should be at most length %d", - count, gdev->ngpio); - count = gdev->ngpio; + /* + * When offset is set in the driver side we assume the driver internally + * is using more than one gpiochip per the same device. We have to stop + * setting friendly names if the specified ones with 'gpio-line-names' + * are less than the offset in the device itself. This means all the + * lines are not present for every single pin within all the internal + * gpiochips. + */ + if (count <= chip->offset) { + dev_warn(&gdev->dev, "gpio-line-names too short (length %d) " + "cannot map names for the gpiochip at offset %u\n", + count, chip->offset); + return 0; } names = kcalloc(count, sizeof(*names), GFP_KERNEL); @@ -400,8 +409,22 @@ static int devprop_gpiochip_set_names(struct gpio_chip *chip) return ret; } + /* + * When more that one gpiochip per device is used, 'count' can + * contain at most number gpiochips x chip->ngpio. We have to + * correctly distribute all defined lines taking into account + * chip->offset as starting point from where we will assign + * the names to pins from the 'names' array. Since property + * 'gpio-line-names' cannot contains gaps, we have to be sure + * we only assign those pins that really exists since chip->ngpio + * can be different of the chip->offset. + */ + count = (count > chip->offset) ? count - chip->offset : count; + if (count > chip->ngpio) + count = chip->ngpio; + for (i = 0; i < count; i++) - gdev->descs[i].name = names[i]; + gdev->descs[i].name = names[chip->offset + i]; kfree(names); diff --git a/include/linux/gpio/driver.h b/include/linux/gpio/driver.h index 3a268781fcec..a0f9901dcae6 100644 --- a/include/linux/gpio/driver.h +++ b/include/linux/gpio/driver.h @@ -312,6 +312,9 @@ struct gpio_irq_chip { * get rid of the static GPIO number space in the long run. * @ngpio: the number of GPIOs handled by this controller; the last GPIO * handled is (base + ngpio - 1). + * @offset: when multiple gpio chips belong to the same device this + * can be used as offset within the device so friendly names can + * be properly assigned. * @names: if set, must be an array of strings to use as alternative * names for the GPIOs in this chip. Any entry in the array * may be NULL if there is no alias for the GPIO, however the @@ -398,6 +401,7 @@ struct gpio_chip { int base; u16 ngpio; + u16 offset; const char *const *names; bool can_sleep;