Commit 11a78b794496 ("ARM: OMAP: MPUIO wake updates") registers the
omap_mpuio_driver from omap_mpuio_init(), which is called from
omap_gpio_probe().

However, it neither makes sense to register drivers from probe()
callbacks of other drivers, nor does the driver core allow registering
drivers with a device lock already being held.

The latter was revealed by commit dc23806a7c47 ("driver core: enforce
device_lock for driver_match_device()") leading to a potential deadlock
condition described in [1].

Additionally, the omap_mpuio_driver is never unregistered from the
driver core, even if the module is unloaded.

Hence, register the omap_mpuio_driver from the module initcall and
unregister it in module_exit().

Link: https://lore.kernel.org/lkml/DFU7CEPUSG9A.1KKGVW4HIPMSH@kernel.org/ [1]
Fixes: dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
Fixes: 11a78b794496 ("ARM: OMAP: MPUIO wake updates")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
In addition to this fix, the omap_mpuio_device (struct platform_device) should
not be declared as global static, especially when their backing memory can
become invalue due to the module being unloaded. Devices are reference counted
and should be allocated dynamically. This needs a separate fix.

Besides that, the whole construct seems a bit questionable and I'm not exactly
sure what should be achieved by registering the *same* static device everytime
probe() is called for the omap_gpio_driver.

However, for the purpose of avoiding the described potential deadlock in
combination with commit dc23806a7c47 ("driver core: enforce device_lock for
driver_match_device()"), this patch only addresses the driver registration
issue.
---
 drivers/gpio/gpio-omap.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index e136e81794df..8db71a2db9ff 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -800,9 +800,7 @@ static struct platform_device omap_mpuio_device = {
 static inline void omap_mpuio_init(struct gpio_bank *bank)
 {
 	platform_set_drvdata(&omap_mpuio_device, bank);
-
-	if (platform_driver_register(&omap_mpuio_driver) == 0)
-		(void) platform_device_register(&omap_mpuio_device);
+	(void)platform_device_register(&omap_mpuio_device);
 }
 
 /*---------------------------------------------------------------------*/
@@ -1575,13 +1573,24 @@ static struct platform_driver omap_gpio_driver = {
  */
 static int __init omap_gpio_drv_reg(void)
 {
-	return platform_driver_register(&omap_gpio_driver);
+	int ret;
+
+	ret = platform_driver_register(&omap_mpuio_driver);
+	if (ret)
+		return ret;
+
+	ret = platform_driver_register(&omap_gpio_driver);
+	if (ret)
+		platform_driver_unregister(&omap_mpuio_driver);
+
+	return ret;
 }
 postcore_initcall(omap_gpio_drv_reg);
 
 static void __exit omap_gpio_exit(void)
 {
 	platform_driver_unregister(&omap_gpio_driver);
+	platform_driver_unregister(&omap_mpuio_driver);
 }
 module_exit(omap_gpio_exit);
 

base-commit: ed1ac3c977dd6b119405fa36dd41f7151bd5b4de
-- 
2.52.0

On Fri, 23 Jan 2026 14:31:56 +0100, Danilo Krummrich wrote:
> Commit 11a78b794496 ("ARM: OMAP: MPUIO wake updates") registers the
> omap_mpuio_driver from omap_mpuio_init(), which is called from
> omap_gpio_probe().
> 
> However, it neither makes sense to register drivers from probe()
> callbacks of other drivers, nor does the driver core allow registering
> drivers with a device lock already being held.
> 
> [...]

Applied, thanks!

[1/1] gpio: omap: do not register driver in probe()
      commit: 3cb53b083fa665ec14c52962f50b9c1df48cf87b

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

On Tue Jan 27, 2026 at 10:09 AM CET, Bartosz Golaszewski wrote:
> Applied, thanks!
>
> [1/1] gpio: omap: do not register driver in probe()
>       commit: 3cb53b083fa665ec14c52962f50b9c1df48cf87b

I think you missed to pick up the diff in [1]. Please let me know if you want a
fixup patch for this.

(One unrelated note, just be because I noticed by accident. Your applied patches
seem to use https://lore.kernel.org for Link: tags, while it has been agreed to
use https://patch.msgid.link instead [2].

I just mention it since it might be unintentional, i.e. it might be that your
tooling does not consider it.

Personally, I use b4 and have the following in my .gitconfig:

	[b4]
		linkmask = https://patch.msgid.link/%s

Not sure this is still necessary in the latest version though.)

[1] https://lore.kernel.org/all/DFW0SC4QG4W8.C7BRHX02W3IK@kernel.org/
[2] https://lore.kernel.org/all/CAHk-=wj5MATvT-FR8qNpXuuBGiJdjY1kRfhtzuyBSpTKR+=Vtw@mail.gmail.com/

On Tue, Jan 27, 2026 at 2:38 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Jan 27, 2026 at 10:09 AM CET, Bartosz Golaszewski wrote:
> > Applied, thanks!
> >
> > [1/1] gpio: omap: do not register driver in probe()
> >       commit: 3cb53b083fa665ec14c52962f50b9c1df48cf87b
>
> I think you missed to pick up the diff in [1]. Please let me know if you want a
> fixup patch for this.
>

Yes, can you send a v2 with that included? I'm dropping this patch
from my queue for now.

> (One unrelated note, just be because I noticed by accident. Your applied patches
> seem to use https://lore.kernel.org for Link: tags, while it has been agreed to
> use https://patch.msgid.link instead [2].
>
> I just mention it since it might be unintentional, i.e. it might be that your
> tooling does not consider it.
>
> Personally, I use b4 and have the following in my .gitconfig:
>
>         [b4]
>                 linkmask = https://patch.msgid.link/%s
>

Yeah, I do use b4. I changed the linkmask now, thanks.

Bartosz

> Not sure this is still necessary in the latest version though.)
>
> [1] https://lore.kernel.org/all/DFW0SC4QG4W8.C7BRHX02W3IK@kernel.org/
> [2] https://lore.kernel.org/all/CAHk-=wj5MATvT-FR8qNpXuuBGiJdjY1kRfhtzuyBSpTKR+=Vtw@mail.gmail.com/

On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index e136e81794df..8db71a2db9ff 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -800,9 +800,7 @@ static struct platform_device omap_mpuio_device = {
>  static inline void omap_mpuio_init(struct gpio_bank *bank)
>  {
>  	platform_set_drvdata(&omap_mpuio_device, bank);
> -
> -	if (platform_driver_register(&omap_mpuio_driver) == 0)
> -		(void) platform_device_register(&omap_mpuio_device);
> +	(void)platform_device_register(&omap_mpuio_device);
>  }

On a second look, it recognize that this did abuse the fact that
platform_driver_register() fails when attempting to register a driver multiple
times to avoid registering the same static device multiple times.

So, I guess this has to be changed to:

diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
index 8db71a2db9ff..3e1ac34994fb 100644
--- a/drivers/gpio/gpio-omap.c
+++ b/drivers/gpio/gpio-omap.c
@@ -799,8 +799,13 @@ static struct platform_device omap_mpuio_device = {

 static inline void omap_mpuio_init(struct gpio_bank *bank)
 {
+       static bool registered = false;
+
        platform_set_drvdata(&omap_mpuio_device, bank);
-       (void)platform_device_register(&omap_mpuio_device);
+       if (!registered) {
+               (void)platform_device_register(&omap_mpuio_device);
+               registered = true;
+       }
 }

 /*---------------------------------------------------------------------*/

On Fri, Jan 23, 2026 at 02:57:45PM +0100, Danilo Krummrich wrote:
> On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> > index e136e81794df..8db71a2db9ff 100644
> > --- a/drivers/gpio/gpio-omap.c
> > +++ b/drivers/gpio/gpio-omap.c
> > @@ -800,9 +800,7 @@ static struct platform_device omap_mpuio_device = {
> >  static inline void omap_mpuio_init(struct gpio_bank *bank)
> >  {
> >  	platform_set_drvdata(&omap_mpuio_device, bank);
> > -
> > -	if (platform_driver_register(&omap_mpuio_driver) == 0)
> > -		(void) platform_device_register(&omap_mpuio_device);
> > +	(void)platform_device_register(&omap_mpuio_device);
> >  }
> 
> On a second look, it recognize that this did abuse the fact that
> platform_driver_register() fails when attempting to register a driver multiple
> times to avoid registering the same static device multiple times.
> 
> So, I guess this has to be changed to:
> 
> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> index 8db71a2db9ff..3e1ac34994fb 100644
> --- a/drivers/gpio/gpio-omap.c
> +++ b/drivers/gpio/gpio-omap.c
> @@ -799,8 +799,13 @@ static struct platform_device omap_mpuio_device = {
> 
>  static inline void omap_mpuio_init(struct gpio_bank *bank)
>  {
> +       static bool registered = false;
> +
>         platform_set_drvdata(&omap_mpuio_device, bank);
> -       (void)platform_device_register(&omap_mpuio_device);
> +       if (!registered) {
> +               (void)platform_device_register(&omap_mpuio_device);
> +               registered = true;
> +       }
>  }

But there are no platform resources for this at all, shouldn't this be a
faux device instead?

That being said, ignoring the return value of platform_device_register()
is probably not something we want to keep around.

thanks,

greg k-h

On Fri Jan 23, 2026 at 3:19 PM CET, Greg KH wrote:
> On Fri, Jan 23, 2026 at 02:57:45PM +0100, Danilo Krummrich wrote:
>> On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
>> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> > index e136e81794df..8db71a2db9ff 100644
>> > --- a/drivers/gpio/gpio-omap.c
>> > +++ b/drivers/gpio/gpio-omap.c
>> > @@ -800,9 +800,7 @@ static struct platform_device omap_mpuio_device = {
>> >  static inline void omap_mpuio_init(struct gpio_bank *bank)
>> >  {
>> >  	platform_set_drvdata(&omap_mpuio_device, bank);
>> > -
>> > -	if (platform_driver_register(&omap_mpuio_driver) == 0)
>> > -		(void) platform_device_register(&omap_mpuio_device);
>> > +	(void)platform_device_register(&omap_mpuio_device);
>> >  }
>> 
>> On a second look, it recognize that this did abuse the fact that
>> platform_driver_register() fails when attempting to register a driver multiple
>> times to avoid registering the same static device multiple times.
>> 
>> So, I guess this has to be changed to:
>> 
>> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
>> index 8db71a2db9ff..3e1ac34994fb 100644
>> --- a/drivers/gpio/gpio-omap.c
>> +++ b/drivers/gpio/gpio-omap.c
>> @@ -799,8 +799,13 @@ static struct platform_device omap_mpuio_device = {
>> 
>>  static inline void omap_mpuio_init(struct gpio_bank *bank)
>>  {
>> +       static bool registered = false;
>> +
>>         platform_set_drvdata(&omap_mpuio_device, bank);
>> -       (void)platform_device_register(&omap_mpuio_device);
>> +       if (!registered) {
>> +               (void)platform_device_register(&omap_mpuio_device);
>> +               registered = true;
>> +       }
>>  }
>
> But there are no platform resources for this at all, shouldn't this be a
> faux device instead?

Probably, but that's for another patch, since this one may potentially be
backported beyond the existence of the faux bus.

> That being said, ignoring the return value of platform_device_register()
> is probably not something we want to keep around.

Yes, as mentioned below the commit message, there are a couple of things that
need to be followed up on here.

With this patch I only intend to fix the deadlock condition and otherwise keep
all the existing semantics as it is.

I.e. maybe it is intentional and this driver should not abort probing if this
can't be registered for some reason.

On Fri, Jan 23, 2026 at 03:25:43PM +0100, Danilo Krummrich wrote:
> On Fri Jan 23, 2026 at 3:19 PM CET, Greg KH wrote:
> > On Fri, Jan 23, 2026 at 02:57:45PM +0100, Danilo Krummrich wrote:
> >> On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
> >> > diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> >> > index e136e81794df..8db71a2db9ff 100644
> >> > --- a/drivers/gpio/gpio-omap.c
> >> > +++ b/drivers/gpio/gpio-omap.c
> >> > @@ -800,9 +800,7 @@ static struct platform_device omap_mpuio_device = {
> >> >  static inline void omap_mpuio_init(struct gpio_bank *bank)
> >> >  {
> >> >  	platform_set_drvdata(&omap_mpuio_device, bank);
> >> > -
> >> > -	if (platform_driver_register(&omap_mpuio_driver) == 0)
> >> > -		(void) platform_device_register(&omap_mpuio_device);
> >> > +	(void)platform_device_register(&omap_mpuio_device);
> >> >  }
> >> 
> >> On a second look, it recognize that this did abuse the fact that
> >> platform_driver_register() fails when attempting to register a driver multiple
> >> times to avoid registering the same static device multiple times.
> >> 
> >> So, I guess this has to be changed to:
> >> 
> >> diff --git a/drivers/gpio/gpio-omap.c b/drivers/gpio/gpio-omap.c
> >> index 8db71a2db9ff..3e1ac34994fb 100644
> >> --- a/drivers/gpio/gpio-omap.c
> >> +++ b/drivers/gpio/gpio-omap.c
> >> @@ -799,8 +799,13 @@ static struct platform_device omap_mpuio_device = {
> >> 
> >>  static inline void omap_mpuio_init(struct gpio_bank *bank)
> >>  {
> >> +       static bool registered = false;
> >> +
> >>         platform_set_drvdata(&omap_mpuio_device, bank);
> >> -       (void)platform_device_register(&omap_mpuio_device);
> >> +       if (!registered) {
> >> +               (void)platform_device_register(&omap_mpuio_device);
> >> +               registered = true;
> >> +       }
> >>  }
> >
> > But there are no platform resources for this at all, shouldn't this be a
> > faux device instead?
> 
> Probably, but that's for another patch, since this one may potentially be
> backported beyond the existence of the faux bus.
> 
> > That being said, ignoring the return value of platform_device_register()
> > is probably not something we want to keep around.
> 
> Yes, as mentioned below the commit message, there are a couple of things that
> need to be followed up on here.
> 
> With this patch I only intend to fix the deadlock condition and otherwise keep
> all the existing semantics as it is.
> 
> I.e. maybe it is intentional and this driver should not abort probing if this
> can't be registered for some reason.

Ok, fair enough, fixing this immediate bug is good enough for me.

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

On Fri Jan 23, 2026 at 4:23 PM CET, Greg KH wrote:
> On Fri, Jan 23, 2026 at 03:25:43PM +0100, Danilo Krummrich wrote:
>> On Fri Jan 23, 2026 at 3:19 PM CET, Greg KH wrote:
>> > But there are no platform resources for this at all, shouldn't this be a
>> > faux device instead?
>> 
>> Probably, but that's for another patch, since this one may potentially be
>> backported beyond the existence of the faux bus.
>> 
>> > That being said, ignoring the return value of platform_device_register()
>> > is probably not something we want to keep around.
>> 
>> Yes, as mentioned below the commit message, there are a couple of things that
>> need to be followed up on here.
>> 
>> With this patch I only intend to fix the deadlock condition and otherwise keep
>> all the existing semantics as it is.
>> 
>> I.e. maybe it is intentional and this driver should not abort probing if this
>> can't be registered for some reason.
>
> Ok, fair enough, fixing this immediate bug is good enough for me.
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Just for the record, the things that should still be addressed:

  (1) Potential UAF on module unload: The device is declared as global static
      (should use a dynamic allocation instead); device is never unregistered.

  (2) Handle return value of *_device_register().

  (3) Use faux bus; maybe not directly suitable as omap_mpuio_driver only uses
      struct dev_pm_ops callbacks. Though, we could extend struct
      faux_device_ops accordingly.

- Danilo

On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
> However, for the purpose of avoiding the described potential deadlock in
> combination with commit dc23806a7c47 ("driver core: enforce device_lock for
> driver_match_device()"), this patch only addresses the driver registration
> issue.

I.e. unless there are any concerns, I'd like to take this one through the
driver-core tree.

On Fri, Jan 23, 2026 at 2:44 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
> > However, for the purpose of avoiding the described potential deadlock in
> > combination with commit dc23806a7c47 ("driver core: enforce device_lock for
> > driver_match_device()"), this patch only addresses the driver registration
> > issue.
>
> I.e. unless there are any concerns, I'd like to take this one through the
> driver-core tree.

It looks good to me, but it's a GPIO driver, this can go together with
GPIO fixes for v6.19-rc8 as usual.

Bart

On Mon Jan 26, 2026 at 10:06 AM CET, Bartosz Golaszewski wrote:
> On Fri, Jan 23, 2026 at 2:44 PM Danilo Krummrich <dakr@kernel.org> wrote:
>>
>> On Fri Jan 23, 2026 at 2:31 PM CET, Danilo Krummrich wrote:
>> > However, for the purpose of avoiding the described potential deadlock in
>> > combination with commit dc23806a7c47 ("driver core: enforce device_lock for
>> > driver_match_device()"), this patch only addresses the driver registration
>> > issue.
>>
>> I.e. unless there are any concerns, I'd like to take this one through the
>> driver-core tree.
>
> It looks good to me, but it's a GPIO driver, this can go together with
> GPIO fixes for v6.19-rc8 as usual.

Commit [1] reveals the issue and was originally scheduled for a driver-core
fixes PR, so it made sense for this patch to go together with [1]. However, I
told Linus [2] that I will hold back [1] for now and send it with the -rc1 PR to
give it some more time in linux-next.

So, if you can take this patch for -rc8, that'd be great.

Thanks,
Danilo

[1] dc23806a7c47 ("driver core: enforce device_lock for driver_match_device()")
[2] https://lore.kernel.org/driver-core/DFWVL46MM928.V9LOBRWI8BLZ@kernel.org/