| Message ID | 20230615132023.13801-4-andriy.shevchenko@linux.intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: aggregator: Incorporate gpio-delay functionality | expand |
Hi Andy, Thanks for your patch! On Thu, Jun 15, 2023 at 3:51 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > In case we have a device instantiated via DT or other means than > via new_device sysfs node, the collision with the latter is possible. > Prevent such collisions by allocating user instantiated devices with > higher IDs, currently set to 1024. Can you please elaborate? How exactly is this possible? Aggregators instantiated through sysfs are named "gpio-aggregator.<n>", and are IDR-based. Aggregators instantiated from DT are named "<unit-address>.<node-name>". How can this conflict? When instantiated from ACPI? What am I missing? > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- a/drivers/gpio/gpio-aggregator.c > +++ b/drivers/gpio/gpio-aggregator.c > @@ -26,6 +26,7 @@ > #include <linux/gpio/driver.h> > #include <linux/gpio/machine.h> > > +#define AGGREGATOR_MIN_DEVID 1024 > #define AGGREGATOR_MAX_GPIOS 512 > > /* > @@ -135,7 +136,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf, > } > > mutex_lock(&gpio_aggregator_lock); > - id = idr_alloc(&gpio_aggregator_idr, aggr, 0, 0, GFP_KERNEL); > + id = idr_alloc(&gpio_aggregator_idr, aggr, AGGREGATOR_MIN_DEVID, 0, GFP_KERNEL); Iff this would solve an issue, it would be only temporarily, until someone instantiates 1024 aggregators through some other means ;-) > mutex_unlock(&gpio_aggregator_lock); > > if (id < 0) { Gr{oetje,eeting}s, Geert
On Thu, Jun 15, 2023 at 04:54:14PM +0200, Geert Uytterhoeven wrote: > On Thu, Jun 15, 2023 at 3:51 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > In case we have a device instantiated via DT or other means than > > via new_device sysfs node, the collision with the latter is possible. > > Prevent such collisions by allocating user instantiated devices with > > higher IDs, currently set to 1024. > > Can you please elaborate? How exactly is this possible? > > Aggregators instantiated through sysfs are named "gpio-aggregator.<n>", > and are IDR-based. > Aggregators instantiated from DT are named "<unit-address>.<node-name>". > How can this conflict? When instantiated from ACPI? > What am I missing? Nothing. It's me who misunderstood how OF platform device naming schema works. So this patch can be discarded as we never will have gpio-delay available for removal via delete_device sysfs node. Bart, tell me if you need a new version w/o this patch (but note that b4 can handle this case with b4 -slt -P1,2,4,5 ... ).
diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c index 4a470dd8b75d..8892cb37ad79 100644 --- a/drivers/gpio/gpio-aggregator.c +++ b/drivers/gpio/gpio-aggregator.c @@ -26,6 +26,7 @@ #include <linux/gpio/driver.h> #include <linux/gpio/machine.h> +#define AGGREGATOR_MIN_DEVID 1024 #define AGGREGATOR_MAX_GPIOS 512 /* @@ -135,7 +136,7 @@ static ssize_t new_device_store(struct device_driver *driver, const char *buf, } mutex_lock(&gpio_aggregator_lock); - id = idr_alloc(&gpio_aggregator_idr, aggr, 0, 0, GFP_KERNEL); + id = idr_alloc(&gpio_aggregator_idr, aggr, AGGREGATOR_MIN_DEVID, 0, GFP_KERNEL); mutex_unlock(&gpio_aggregator_lock); if (id < 0) {
In case we have a device instantiated via DT or other means than via new_device sysfs node, the collision with the latter is possible. Prevent such collisions by allocating user instantiated devices with higher IDs, currently set to 1024. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/gpio/gpio-aggregator.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)