Reset core uses the reset_gpio driver for a fallback mechanism. So,
include it always once its dependencies are met to enable the fallback
mechanism whenever possible. This avoids regressions when drivers remove
open coded solutions in favor of this fallback.
Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
Closes: https://lore.kernel.org/r/87a51um1y1.wl-kuninori.morimoto.gx@renesas.com
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since RFC v1:
* new patch
drivers/reset/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 78b7078478d4..7319bcd251dc 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -5,6 +5,7 @@ config ARCH_HAS_RESET_CONTROLLER
menuconfig RESET_CONTROLLER
bool "Reset Controller Support"
default y if ARCH_HAS_RESET_CONTROLLER
+ select RESET_GPIO if GPIOLIB
help
Generic Reset Controller support.
--
2.47.2On Mi, 2025-10-15 at 22:59 +0200, Wolfram Sang wrote: > Reset core uses the reset_gpio driver for a fallback mechanism. So, > include it always once its dependencies are met to enable the fallback > mechanism whenever possible. This avoids regressions when drivers remove > open coded solutions in favor of this fallback. > > Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > Closes: https://lore.kernel.org/r/87a51um1y1.wl-kuninori.morimoto.gx@renesas.com > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > > Changes since RFC v1: > * new patch > > drivers/reset/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig > index 78b7078478d4..7319bcd251dc 100644 > --- a/drivers/reset/Kconfig > +++ b/drivers/reset/Kconfig > @@ -5,6 +5,7 @@ config ARCH_HAS_RESET_CONTROLLER > menuconfig RESET_CONTROLLER > bool "Reset Controller Support" > default y if ARCH_HAS_RESET_CONTROLLER > + select RESET_GPIO if GPIOLIB Thank you, what was the reason to go with this instead of default y if GPIOLIB ? regards Philipp
> > + select RESET_GPIO if GPIOLIB > > Thank you, what was the reason to go with this instead of > > default y if GPIOLIB The above will "fix" existing configs automatically. If a config has already RESET_GPIO=n, it will not be updated with the latter, so it still requires user interaction. Because RESET_GPIO is a leaf object, I think it is OK to select it.
Hi Wolfram,
On Thu, 16 Oct 2025 at 14:16, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Reset core uses the reset_gpio driver for a fallback mechanism. So,
> include it always once its dependencies are met to enable the fallback
> mechanism whenever possible. This avoids regressions when drivers remove
> open coded solutions in favor of this fallback.
>
> Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> Closes: https://lore.kernel.org/r/87a51um1y1.wl-kuninori.morimoto.gx@renesas.com
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Thanks for your patch!
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -5,6 +5,7 @@ config ARCH_HAS_RESET_CONTROLLER
> menuconfig RESET_CONTROLLER
> bool "Reset Controller Support"
> default y if ARCH_HAS_RESET_CONTROLLER
> + select RESET_GPIO if GPIOLIB
> help
> Generic Reset Controller support.
>
Makes sense, so
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
This does mean RESET_GPIO will never be modular anymore, while it could
still work as a module (the reset core creates the platform device,
which can be probed later), albeit in a non-intuitive way.
BTW, could we run into a circular dependency?
config RESET_TI_TPS380X
tristate "TI TPS380x Reset Driver"
select GPIOLIB
I guess this should be changed from select to depends on?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Do, 2025-10-16 at 15:02 +0200, Geert Uytterhoeven wrote: > Hi Wolfram, > > On Thu, 16 Oct 2025 at 14:16, Wolfram Sang > <wsa+renesas@sang-engineering.com> wrote: > > Reset core uses the reset_gpio driver for a fallback mechanism. So, > > include it always once its dependencies are met to enable the fallback > > mechanism whenever possible. This avoids regressions when drivers remove > > open coded solutions in favor of this fallback. > > > > Reported-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Closes: https://lore.kernel.org/r/87a51um1y1.wl-kuninori.morimoto.gx@renesas.com > > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > > Thanks for your patch! > > > --- a/drivers/reset/Kconfig > > +++ b/drivers/reset/Kconfig > > @@ -5,6 +5,7 @@ config ARCH_HAS_RESET_CONTROLLER > > menuconfig RESET_CONTROLLER > > bool "Reset Controller Support" > > default y if ARCH_HAS_RESET_CONTROLLER > > + select RESET_GPIO if GPIOLIB > > help > > Generic Reset Controller support. > > > > Makes sense, so > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > This does mean RESET_GPIO will never be modular anymore, while it could > still work as a module (the reset core creates the platform device, > which can be probed later), albeit in a non-intuitive way. Btw, Bartosz (added to Cc:) is reworking reset-gpio into an auxiliary device driver. [1] https://lore.kernel.org/all/20251006-reset-gpios-swnodes-v1-0-6d3325b9af42@linaro.org/ > BTW, could we run into a circular dependency? > > config RESET_TI_TPS380X > tristate "TI TPS380x Reset Driver" > select GPIOLIB > > I guess this should be changed from select to depends on? The drivers referencing GPIOLIB seem to be split in the middle between select and depends... regards Philipp
> > This does mean RESET_GPIO will never be modular anymore, while it could > > still work as a module (the reset core creates the platform device, > > which can be probed later), albeit in a non-intuitive way. > > Btw, Bartosz (added to Cc:) is reworking reset-gpio into an auxiliary > device driver. Thanks for the pointer, but this is too much side-tracking for me ;) > > config RESET_TI_TPS380X > > tristate "TI TPS380x Reset Driver" > > select GPIOLIB > > > > I guess this should be changed from select to depends on? > > The drivers referencing GPIOLIB seem to be split in the middle between > select and depends... :(
Hi Geert, > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thank you! > This does mean RESET_GPIO will never be modular anymore, while it could > still work as a module (the reset core creates the platform device, > which can be probed later), albeit in a non-intuitive way. Interesting topic. In fact, I think we should make RESET_GPIO bool. I think the fallback mechanism of the core should work without any module loading infrastructure. It should be there whenever possible. > BTW, could we run into a circular dependency? > > config RESET_TI_TPS380X > tristate "TI TPS380x Reset Driver" > select GPIOLIB > > I guess this should be changed from select to depends on? I agree. This is not "select" material. Happy hacking, Wolfram
On Thu, Oct 16, 2025 at 4:28 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > Hi Geert, > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Thank you! > > > This does mean RESET_GPIO will never be modular anymore, while it could > > still work as a module (the reset core creates the platform device, > > which can be probed later), albeit in a non-intuitive way. > > Interesting topic. In fact, I think we should make RESET_GPIO bool. I > think the fallback mechanism of the core should work without any module > loading infrastructure. It should be there whenever possible. > You have not said *why*. How is this different from any other device whose driver is only loaded when actually needed? Bartosz
> > Interesting topic. In fact, I think we should make RESET_GPIO bool. I > > think the fallback mechanism of the core should work without any module > > loading infrastructure. It should be there whenever possible. > > > > You have not said *why*. How is this different from any other device > whose driver is only loaded when actually needed? ? I just did that, but let me repeat: I think the fallback mechanism of the core should work without any module loading infrastructure. It should be there whenever possible. I might add that module loading infrastructure might be broken in userspace. Been there. Also, some drivers might need their reset early? Looking more into it, I can't find any 'request_module'. Am I overlooking something? The fallback feature is only present if the user loads the driver manually? If that is true, it would make it rather useless IMHO because consumer drivers cannot rely on it. I must be missing something...
On Fri, Oct 17, 2025 at 12:48 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > > > > Interesting topic. In fact, I think we should make RESET_GPIO bool. I > > > think the fallback mechanism of the core should work without any module > > > loading infrastructure. It should be there whenever possible. > > > > > > > You have not said *why*. How is this different from any other device > > whose driver is only loaded when actually needed? > > ? I just did that, but let me repeat: > > I think the fallback mechanism of the core should work without any > module loading infrastructure. It should be there whenever possible. > It's not really a fallback, is it? This is the path we'll always take if the driver requests a reset control on a firmware node which has a reset-gpios property. If the driver goes with the gpiod API, it will get a regular descriptor. It's deterministic enough to not warrant the term "fallback". > I might add that module loading infrastructure might be broken in > userspace. Been there. Also, some drivers might need their reset early? > Then I believe the platform's config should make sure the driver is built-in. I don't think it makes sense to just cram it into the kernel image for the few users it currently has. > Looking more into it, I can't find any 'request_module'. Am I > overlooking something? The fallback feature is only present if the user > loads the driver manually? If that is true, it would make it rather > useless IMHO because consumer drivers cannot rely on it. I must be > missing something... > The reset-gpio driver has a MODULE_DEVICE_TABLE(). Bart
> > I think the fallback mechanism of the core should work without any > > module loading infrastructure. It should be there whenever possible. > > > > It's not really a fallback, is it? This is the path we'll always take > if the driver requests a reset control on a firmware node which has a > reset-gpios property. If the driver goes with the gpiod API, it will > get a regular descriptor. It's deterministic enough to not warrant the > term "fallback". I dunno for how many drivers this is really applicable, but I really liked the cleanup of the pca954x driver. Don't handle GPIOs internally, just get a reset, and it might be a GPIO. I think it is very useful and I would like to see it wherever possible. We could now make these drivers depend on RESET_GPIO. This would make sense in a way but is uncomfortable for the user who has not RESET_GPIO enabled before. The driver would just disappear because of unmet dependencies. Yes, this can happen all the time because we always find new dependencies and describe them. I just hoped it could be avoided in this case. > Then I believe the platform's config should make sure the driver is > built-in. I don't think it makes sense to just cram it into the kernel > image for the few users it currently has. For Morimoto-san, the PCA954x update resulted in a regression. It is worth thinking how to avoid that. The driver is so small, I wouldn't mind the extra space if it saves users from disappearing devices. But mileages vary...
On Fri, Oct 17, 2025 at 1:25 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > > > > I think the fallback mechanism of the core should work without any > > > module loading infrastructure. It should be there whenever possible. > > > > > > > It's not really a fallback, is it? This is the path we'll always take > > if the driver requests a reset control on a firmware node which has a > > reset-gpios property. If the driver goes with the gpiod API, it will > > get a regular descriptor. It's deterministic enough to not warrant the > > term "fallback". > > I dunno for how many drivers this is really applicable, but I really > liked the cleanup of the pca954x driver. Don't handle GPIOs internally, > just get a reset, and it might be a GPIO. I think it is very useful and > I would like to see it wherever possible. > > We could now make these drivers depend on RESET_GPIO. This would make > sense in a way but is uncomfortable for the user who has not RESET_GPIO > enabled before. The driver would just disappear because of unmet > dependencies. Yes, this can happen all the time because we always find > new dependencies and describe them. I just hoped it could be avoided in > this case. > > > Then I believe the platform's config should make sure the driver is > > built-in. I don't think it makes sense to just cram it into the kernel > > image for the few users it currently has. > > For Morimoto-san, the PCA954x update resulted in a regression. It is > worth thinking how to avoid that. The driver is so small, I wouldn't > mind the extra space if it saves users from disappearing devices. But > mileages vary... > It's up to Philipp but I'd personally go with "default m if GPIOLIB". Bartosz
On Fr, 2025-10-17 at 19:02 +0200, Bartosz Golaszewski wrote: > On Fri, Oct 17, 2025 at 1:25 PM Wolfram Sang > <wsa+renesas@sang-engineering.com> wrote: > > > > > > > > I think the fallback mechanism of the core should work without any > > > > module loading infrastructure. It should be there whenever possible. > > > > > > > > > > It's not really a fallback, is it? This is the path we'll always take > > > if the driver requests a reset control on a firmware node which has a > > > reset-gpios property. If the driver goes with the gpiod API, it will > > > get a regular descriptor. It's deterministic enough to not warrant the > > > term "fallback". > > > > I dunno for how many drivers this is really applicable, but I really > > liked the cleanup of the pca954x driver. That cleanup might have been a little premature, given that the reset- gpio driver currently only works on OF-based platforms, and even there only with gpio controllers with #gpio-cells = <2>. > > Don't handle GPIOs internally, > > just get a reset, and it might be a GPIO. I think it is very useful and > > I would like to see it wherever possible. > > > > We could now make these drivers depend on RESET_GPIO. This would make > > sense in a way but is uncomfortable for the user who has not RESET_GPIO > > enabled before. The driver would just disappear because of unmet > > dependencies. Yes, this can happen all the time because we always find > > new dependencies and describe them. I just hoped it could be avoided in > > this case. How about selecting RESET_GPIO from I2C_MUX_PCA954x? It already depends on GPIOLIB. Although I don't like the idea of drivers being converted en masse, all selecting RESET_GPIO ... > > > > > Then I believe the platform's config should make sure the driver is > > > built-in. I don't think it makes sense to just cram it into the kernel > > > image for the few users it currently has. > > > > For Morimoto-san, the PCA954x update resulted in a regression. It is > > worth thinking how to avoid that. The driver is so small, I wouldn't > > mind the extra space if it saves users from disappearing devices. But > > mileages vary... > > > > It's up to Philipp but I'd personally go with "default m if GPIOLIB". To be honest, I don't like either very much. Yes, the reset-gpio driver is only about three pages in size, but force-enabling it for nearly everyone, just because some hardware designs like to share resets a little too much, feels wrong to me, especially in its current state. And just default-enabling it doesn't solve the regression problem when updating preexisting configs. regards Philipp
Hi Philipp, > > > I dunno for how many drivers this is really applicable, but I really > > > liked the cleanup of the pca954x driver. > > That cleanup might have been a little premature, given that the reset- > gpio driver currently only works on OF-based platforms, and even there > only with gpio controllers with #gpio-cells = <2>. I see. That kind of spoils my assumption that it is a fallback supported by the core. Darn, I would still like to have it, but it seems more complicated than I have time for it :( > How about selecting RESET_GPIO from I2C_MUX_PCA954x? It already depends > on GPIOLIB. Although I don't like the idea of drivers being converted > en masse, all selecting RESET_GPIO ... Well, on top of that, reset is optional with this driver, so selecting it doesn't feel proper. > To be honest, I don't like either very much. > > Yes, the reset-gpio driver is only about three pages in size, but > force-enabling it for nearly everyone, just because some hardware > designs like to share resets a little too much, feels wrong to me, > especially in its current state. I understand the argument of 'too limited in the current state'. I don't get the 'share too much' argument? The fallback would remove open-coded "reset-gpios" handling and sync it with generic reset handling? > And just default-enabling it doesn't solve the regression problem when > updating preexisting configs. Yes. Well, at least patch 1 seems okay, so users at least get notified of the problem... All the best, Wolfram
On Thu, Oct 23, 2025 at 11:22 AM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > Hi Philipp, > > > > > I dunno for how many drivers this is really applicable, but I really > > > > liked the cleanup of the pca954x driver. > > > > That cleanup might have been a little premature, given that the reset- > > gpio driver currently only works on OF-based platforms, and even there > > only with gpio controllers with #gpio-cells = <2>. > > I see. That kind of spoils my assumption that it is a fallback supported > by the core. Darn, I would still like to have it, but it seems more > complicated than I have time for it :( > As soon as my two other reset series land in next, I will finish my work on converting the reset core to fwnode which should help. Bart
> > I see. That kind of spoils my assumption that it is a fallback supported > > by the core. Darn, I would still like to have it, but it seems more > > complicated than I have time for it :( > > > > As soon as my two other reset series land in next, I will finish my > work on converting the reset core to fwnode which should help. Cool! Then you bring back my argument, that it should be always compiled in because it is a core feature ;)
On Thu, Oct 23, 2025 at 1:18 PM Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > > > > I see. That kind of spoils my assumption that it is a fallback supported > > > by the core. Darn, I would still like to have it, but it seems more > > > complicated than I have time for it :( > > > > > > > As soon as my two other reset series land in next, I will finish my > > work on converting the reset core to fwnode which should help. > > Cool! Then you bring back my argument, that it should be always compiled > in because it is a core feature ;) > No, I still think it should be a module by default with an option to build it in if the platform demands it. Just like 95% of the drivers out there. Bartosz
> > Cool! Then you bring back my argument, that it should be always compiled > > in because it is a core feature ;) > > > > No, I still think it should be a module by default with an option to > build it in if the platform demands it. Just like 95% of the drivers > out there. I know how you think, that's why I put the smiley there.
© 2016 - 2025 Red Hat, Inc.