| Message ID | 20250213195621.3133406-1-andriy.shevchenko@linux.intel.com |
|---|---|
| Headers | show |
| Series | gpio: regmap: Make use of 'ngpios' property | expand |
On Thu Feb 13, 2025 at 8:48 PM CET, Andy Shevchenko wrote: > It appears that regmap GPIO doesn't take into account 'ngpios' property > and requires hard coded values or duplication of the parsing the same > outside of GPIO library. This miniseries addresses that. > > For the record, I have checked all bgpio_init() users and haven't seen > the suspicious code that this series might break, e.g., an equivalent of > something like this: > > static int foo_probe(struct device *dev) > { > struct gpio_chip *gc = devm_kzalloc(...); > struct fwnode_handle *fwnode = ...; // NOT dev_fwnode(dev)! > > ... > gc->parent = dev; > gc->fwnode = fwnode; > > ret = bgpio_init(gc, dev, ...); > ... > } > > Reported-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> > > Andy Shevchenko (5): > gpiolib: Extract gpiochip_choose_fwnode() for wider use > gpiolib: Use fwnode instead of device in gpiochip_get_ngpios() > gpio: regmap: Group optional assignments together for better > understanding > gpio: regmap: Move optional assignments down in the code > gpio: regmap: Allow ngpio to be read from the property > > drivers/gpio/gpio-regmap.c | 41 +++++++++++++++++++++---------------- > drivers/gpio/gpiolib.c | 27 ++++++++++++++++-------- > include/linux/gpio/regmap.h | 4 ++-- > 3 files changed, 43 insertions(+), 29 deletions(-) Hi Andy, Thanks, I confirm I tested this series and it does fix my case: I can leave the ngpio field uninitialized and its value will be correctly retrieved from the "ngpios" property. Also the whole series looks good to me.
On Thu, Feb 13, 2025 at 8:56 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > It appears that regmap GPIO doesn't take into account 'ngpios' property > and requires hard coded values or duplication of the parsing the same > outside of GPIO library. This miniseries addresses that. > > For the record, I have checked all bgpio_init() users and haven't seen > the suspicious code that this series might break, e.g., an equivalent of > something like this: > > static int foo_probe(struct device *dev) > { > struct gpio_chip *gc = devm_kzalloc(...); > struct fwnode_handle *fwnode = ...; // NOT dev_fwnode(dev)! > > ... > gc->parent = dev; > gc->fwnode = fwnode; > > ret = bgpio_init(gc, dev, ...); > ... > } > > Reported-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> Thanks for fixing this Andy! Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Fri Feb 14, 2025 at 2:49 PM CET, Andy Shevchenko wrote: > On Fri, Feb 14, 2025 at 10:18:29AM +0100, Mathieu Dubois-Briand wrote: > > On Thu Feb 13, 2025 at 8:48 PM CET, Andy Shevchenko wrote: > > > It appears that regmap GPIO doesn't take into account 'ngpios' property > > > and requires hard coded values or duplication of the parsing the same > > > outside of GPIO library. This miniseries addresses that. > > > > > > For the record, I have checked all bgpio_init() users and haven't seen > > > the suspicious code that this series might break, e.g., an equivalent of > > > something like this: > > > > > > static int foo_probe(struct device *dev) > > > { > > > struct gpio_chip *gc = devm_kzalloc(...); > > > struct fwnode_handle *fwnode = ...; // NOT dev_fwnode(dev)! > > > > > > ... > > > gc->parent = dev; > > > gc->fwnode = fwnode; > > > > > > ret = bgpio_init(gc, dev, ...); > > > ... > > > } > > > > > > Reported-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> > > ... > > > Thanks, I confirm I tested this series and it does fix my case: I can > > leave the ngpio field uninitialized and its value will be correctly > > retrieved from the "ngpios" property. > > > > Also the whole series looks good to me. > > Thank you! Can you give a formal tag(s)? Sure! Tested-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> Reviewed-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: > On Thu, Feb 13, 2025 at 8:56 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > > It appears that regmap GPIO doesn't take into account 'ngpios' property > > and requires hard coded values or duplication of the parsing the same > > outside of GPIO library. This miniseries addresses that. > > > > For the record, I have checked all bgpio_init() users and haven't seen > > the suspicious code that this series might break, e.g., an equivalent of > > something like this: > > > > static int foo_probe(struct device *dev) > > { > > struct gpio_chip *gc = devm_kzalloc(...); > > struct fwnode_handle *fwnode = ...; // NOT dev_fwnode(dev)! > > > > ... > > gc->parent = dev; > > gc->fwnode = fwnode; > > > > ret = bgpio_init(gc, dev, ...); > > ... > > } > > > > Reported-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> > > Thanks for fixing this Andy! > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Thank you for the review! Bart, do you think it can be applied?
On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: > > On Thu, Feb 13, 2025 at 8:56 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > It appears that regmap GPIO doesn't take into account 'ngpios' property > > > and requires hard coded values or duplication of the parsing the same > > > outside of GPIO library. This miniseries addresses that. > > > > > > For the record, I have checked all bgpio_init() users and haven't seen > > > the suspicious code that this series might break, e.g., an equivalent of > > > something like this: > > > > > > static int foo_probe(struct device *dev) > > > { > > > struct gpio_chip *gc = devm_kzalloc(...); > > > struct fwnode_handle *fwnode = ...; // NOT dev_fwnode(dev)! > > > > > > ... > > > gc->parent = dev; > > > gc->fwnode = fwnode; > > > > > > ret = bgpio_init(gc, dev, ...); > > > ... > > > } > > > > > > Reported-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> > > > > Thanks for fixing this Andy! > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Thank you for the review! > > Bart, do you think it can be applied? > Andy, I really rarely lose track of patches. It's been just under a week since this was posted. Please don't ping me to pick things up unless I'm not reacting for at least two weeks. I typically leave patches on the list for some time to give bots some time to react. Bartosz
On Thu, Feb 20, 2025 at 02:22:29PM +0100, Bartosz Golaszewski wrote: > On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: ... > > Bart, do you think it can be applied? > > Andy, > > I really rarely lose track of patches. It's been just under a week > since this was posted. Please don't ping me to pick things up unless > I'm not reacting for at least two weeks. I typically leave patches on > the list for some time to give bots some time to react. I see, I thought your cadence is one week, that's why I have pinged you. Will try to keep this in mind for the future and sorry to interrupt!
On Thu, Feb 20, 2025 at 03:40:15PM +0200, Andy Shevchenko wrote: > On Thu, Feb 20, 2025 at 02:22:29PM +0100, Bartosz Golaszewski wrote: > > On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: ... > > > Bart, do you think it can be applied? > > > > Andy, > > > > I really rarely lose track of patches. It's been just under a week > > since this was posted. Please don't ping me to pick things up unless > > I'm not reacting for at least two weeks. I typically leave patches on > > the list for some time to give bots some time to react. > > I see, I thought your cadence is one week, that's why I have pinged you. > Will try to keep this in mind for the future and sorry to interrupt! Btw, if it's easier to you, I can just combine this to my usual PR to you.
On Thu, Feb 20, 2025 at 2:41 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Feb 20, 2025 at 03:40:15PM +0200, Andy Shevchenko wrote: > > On Thu, Feb 20, 2025 at 02:22:29PM +0100, Bartosz Golaszewski wrote: > > > On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: > > ... > > > > > Bart, do you think it can be applied? > > > > > > Andy, > > > > > > I really rarely lose track of patches. It's been just under a week > > > since this was posted. Please don't ping me to pick things up unless > > > I'm not reacting for at least two weeks. I typically leave patches on > > > the list for some time to give bots some time to react. > > > > I see, I thought your cadence is one week, that's why I have pinged you. > > Will try to keep this in mind for the future and sorry to interrupt! > > Btw, if it's easier to you, I can just combine this to my usual PR to you. > No, that's fine, let's stick to ACPI-only PRs. Bart
On Thu, Feb 20, 2025 at 02:42:26PM +0100, Bartosz Golaszewski wrote: > On Thu, Feb 20, 2025 at 2:41 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Feb 20, 2025 at 03:40:15PM +0200, Andy Shevchenko wrote: > > > On Thu, Feb 20, 2025 at 02:22:29PM +0100, Bartosz Golaszewski wrote: > > > > On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: ... > > > > > Bart, do you think it can be applied? > > > > > > > > Andy, > > > > > > > > I really rarely lose track of patches. It's been just under a week > > > > since this was posted. Please don't ping me to pick things up unless > > > > I'm not reacting for at least two weeks. I typically leave patches on > > > > the list for some time to give bots some time to react. > > > > > > I see, I thought your cadence is one week, that's why I have pinged you. > > > Will try to keep this in mind for the future and sorry to interrupt! > > > > Btw, if it's easier to you, I can just combine this to my usual PR to you. > > No, that's fine, let's stick to ACPI-only PRs. Hmm... Is the Intel GPIO stuff should go directly to your tree? Seems I missed some changes in the flow...
On Thu, Feb 20, 2025 at 3:07 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Feb 20, 2025 at 02:42:26PM +0100, Bartosz Golaszewski wrote: > > On Thu, Feb 20, 2025 at 2:41 PM Andy Shevchenko > > <andriy.shevchenko@linux.intel.com> wrote: > > > On Thu, Feb 20, 2025 at 03:40:15PM +0200, Andy Shevchenko wrote: > > > > On Thu, Feb 20, 2025 at 02:22:29PM +0100, Bartosz Golaszewski wrote: > > > > > On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko > > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: > > ... > > > > > > > Bart, do you think it can be applied? > > > > > > > > > > Andy, > > > > > > > > > > I really rarely lose track of patches. It's been just under a week > > > > > since this was posted. Please don't ping me to pick things up unless > > > > > I'm not reacting for at least two weeks. I typically leave patches on > > > > > the list for some time to give bots some time to react. > > > > > > > > I see, I thought your cadence is one week, that's why I have pinged you. > > > > Will try to keep this in mind for the future and sorry to interrupt! > > > > > > Btw, if it's easier to you, I can just combine this to my usual PR to you. > > > > No, that's fine, let's stick to ACPI-only PRs. > > Hmm... Is the Intel GPIO stuff should go directly to your tree? Seems I missed > some changes in the flow... > Ah, no, sure, intel and ACPI and whatever you did up until this point. Bart
On Thu, Feb 20, 2025 at 03:11:04PM +0100, Bartosz Golaszewski wrote: > On Thu, Feb 20, 2025 at 3:07 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Feb 20, 2025 at 02:42:26PM +0100, Bartosz Golaszewski wrote: > > > On Thu, Feb 20, 2025 at 2:41 PM Andy Shevchenko > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > On Thu, Feb 20, 2025 at 03:40:15PM +0200, Andy Shevchenko wrote: > > > > > On Thu, Feb 20, 2025 at 02:22:29PM +0100, Bartosz Golaszewski wrote: > > > > > > On Thu, Feb 20, 2025 at 2:18 PM Andy Shevchenko > > > > > > <andriy.shevchenko@linux.intel.com> wrote: > > > > > > > On Fri, Feb 14, 2025 at 11:50:53AM +0100, Linus Walleij wrote: ... > > > > > > > Bart, do you think it can be applied? > > > > > > > > > > > > Andy, > > > > > > > > > > > > I really rarely lose track of patches. It's been just under a week > > > > > > since this was posted. Please don't ping me to pick things up unless > > > > > > I'm not reacting for at least two weeks. I typically leave patches on > > > > > > the list for some time to give bots some time to react. > > > > > > > > > > I see, I thought your cadence is one week, that's why I have pinged you. > > > > > Will try to keep this in mind for the future and sorry to interrupt! > > > > > > > > Btw, if it's easier to you, I can just combine this to my usual PR to you. > > > > > > No, that's fine, let's stick to ACPI-only PRs. > > > > Hmm... Is the Intel GPIO stuff should go directly to your tree? Seems I missed > > some changes in the flow... > > Ah, no, sure, intel and ACPI and whatever you did up until this point. Got it, thanks!
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> On Thu, 13 Feb 2025 21:48:45 +0200, Andy Shevchenko wrote: > It appears that regmap GPIO doesn't take into account 'ngpios' property > and requires hard coded values or duplication of the parsing the same > outside of GPIO library. This miniseries addresses that. > > For the record, I have checked all bgpio_init() users and haven't seen > the suspicious code that this series might break, e.g., an equivalent of > something like this: > > [...] Applied, thanks! [1/5] gpiolib: Extract gpiochip_choose_fwnode() for wider use commit: 375790f18396b2ba706e031b150c58cd37b45a11 [2/5] gpiolib: Use fwnode instead of device in gpiochip_get_ngpios() commit: 6f077e575893214136f9739f993bd9fedf61731a [3/5] gpio: regmap: Group optional assignments together for better understanding commit: 97673ea38a77e42eaafcf5181c84f6c8d40b97e7 [4/5] gpio: regmap: Move optional assignments down in the code commit: a630d3960b6ac3c37cb0789605056e8845ffbf16 [5/5] gpio: regmap: Allow ngpio to be read from the property commit: db305161880a024a43f4b1cbafa7a294793d7a9e Best regards,
It appears that regmap GPIO doesn't take into account 'ngpios' property and requires hard coded values or duplication of the parsing the same outside of GPIO library. This miniseries addresses that. For the record, I have checked all bgpio_init() users and haven't seen the suspicious code that this series might break, e.g., an equivalent of something like this: static int foo_probe(struct device *dev) { struct gpio_chip *gc = devm_kzalloc(...); struct fwnode_handle *fwnode = ...; // NOT dev_fwnode(dev)! ... gc->parent = dev; gc->fwnode = fwnode; ret = bgpio_init(gc, dev, ...); ... } Reported-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> Andy Shevchenko (5): gpiolib: Extract gpiochip_choose_fwnode() for wider use gpiolib: Use fwnode instead of device in gpiochip_get_ngpios() gpio: regmap: Group optional assignments together for better understanding gpio: regmap: Move optional assignments down in the code gpio: regmap: Allow ngpio to be read from the property drivers/gpio/gpio-regmap.c | 41 +++++++++++++++++++++---------------- drivers/gpio/gpiolib.c | 27 ++++++++++++++++-------- include/linux/gpio/regmap.h | 4 ++-- 3 files changed, 43 insertions(+), 29 deletions(-)