mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] watchdog: take all OF aliases into account when assigning id
@ 2026-06-15 14:57 Rasmus Villemoes
  2026-07-08 14:53 ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Rasmus Villemoes @ 2026-06-15 14:57 UTC (permalink / raw)
  To: Wim Van Sebroeck, Guenter Roeck
  Cc: linux-watchdog, linux-kernel, Rasmus Villemoes

If some, but not all, watchdog devices have device tree aliases, those
without aliases might (depending on probe order) be assigned an id
which would otherwise be assigned to one of those with an alias.

This is problematic when for example watchdog0 is an alias for an
always-running gpio watchdog that userspace must handle, but the SOC's
watchdog device(s) get probed first and thus one of those become
/dev/watchdog0.

Ensure that ids for devices without a device tree alias are allocated
from above the highest numbered alias, if any.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---

This is similar to how the mmc, i2c, i3c and spi subsystems handle
device tree aliases and avoid using an id that might be assigned to a
device/bus that is probed later.

 drivers/watchdog/watchdog_core.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 6152dba4b52c..80675f160e11 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -239,7 +239,7 @@ EXPORT_SYMBOL_GPL(watchdog_set_restart_priority);
 
 static int ___watchdog_register_device(struct watchdog_device *wdd)
 {
-	int ret, id = -1;
+	int ret, min_id, id = -1;
 
 	if (wdd == NULL || wdd->info == NULL || wdd->ops == NULL)
 		return -EINVAL;
@@ -264,8 +264,15 @@ static int ___watchdog_register_device(struct watchdog_device *wdd)
 					     GFP_KERNEL);
 	}
 
-	if (id < 0)
-		id = ida_alloc_max(&watchdog_ida, MAX_DOGS - 1, GFP_KERNEL);
+	if (id < 0) {
+		ret = of_alias_get_highest_id("watchdog");
+		if (ret >= 0)
+			min_id = ret + 1;
+		else
+			min_id = 0;
+
+		id = ida_alloc_range(&watchdog_ida, min_id, MAX_DOGS - 1, GFP_KERNEL);
+	}
 
 	if (id < 0)
 		return id;
-- 
2.54.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog: take all OF aliases into account when assigning id
  2026-06-15 14:57 [PATCH] watchdog: take all OF aliases into account when assigning id Rasmus Villemoes
@ 2026-07-08 14:53 ` Guenter Roeck
  2026-07-13 14:35   ` Rasmus Villemoes
  0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2026-07-08 14:53 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel

On Mon, Jun 15, 2026 at 04:57:59PM +0200, Rasmus Villemoes wrote:
> If some, but not all, watchdog devices have device tree aliases, those
> without aliases might (depending on probe order) be assigned an id
> which would otherwise be assigned to one of those with an alias.
> 
> This is problematic when for example watchdog0 is an alias for an
> always-running gpio watchdog that userspace must handle, but the SOC's
> watchdog device(s) get probed first and thus one of those become
> /dev/watchdog0.
> 
> Ensure that ids for devices without a device tree alias are allocated
> from above the highest numbered alias, if any.
> 
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> 
> This is similar to how the mmc, i2c, i3c and spi subsystems handle
> device tree aliases and avoid using an id that might be assigned to a
> device/bus that is probed later.

The patch makes sense. Unfortunately, there are systems with aliased
watchdogs which do not enable "watchdog0" (e.g., several Nuvoton based
boards). On such systems, if they do have an unaliased / auto-generated
watchdog, /dev/watchdog0 and with it /dev/watchdog would no longer be
created. This would result in a ABI break.

On top of that, the patch only affects systems with both aliased and
un-aliased watchdogs, which makes me even more concerned.

To apply this or a similar patch, we would have to ensure that there
is no enabled watchdog with ID == 0.

Thanks,
Guenter

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog: take all OF aliases into account when assigning id
  2026-07-08 14:53 ` Guenter Roeck
@ 2026-07-13 14:35   ` Rasmus Villemoes
  2026-07-13 15:48     ` Guenter Roeck
  0 siblings, 1 reply; 5+ messages in thread
From: Rasmus Villemoes @ 2026-07-13 14:35 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel

On Wed, Jul 08 2026, "Guenter Roeck" <linux@roeck-us.net> wrote:

> On Mon, Jun 15, 2026 at 04:57:59PM +0200, Rasmus Villemoes wrote:
>> If some, but not all, watchdog devices have device tree aliases, those
>> without aliases might (depending on probe order) be assigned an id
>> which would otherwise be assigned to one of those with an alias.
>>
>> This is problematic when for example watchdog0 is an alias for an
>> always-running gpio watchdog that userspace must handle, but the SOC's
>> watchdog device(s) get probed first and thus one of those become
>> /dev/watchdog0.
>>
>> Ensure that ids for devices without a device tree alias are allocated
>> from above the highest numbered alias, if any.
>>
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> ---
>>
>> This is similar to how the mmc, i2c, i3c and spi subsystems handle
>> device tree aliases and avoid using an id that might be assigned to a
>> device/bus that is probed later.
>
> The patch makes sense. Unfortunately, there are systems with aliased
> watchdogs which do not enable "watchdog0" (e.g., several Nuvoton based
> boards). On such systems, if they do have an unaliased / auto-generated
> watchdog, /dev/watchdog0 and with it /dev/watchdog would no longer be
> created. This would result in a ABI break.
>
> On top of that, the patch only affects systems with both aliased and
> un-aliased watchdogs, which makes me even more concerned.

Well, yes, the problem only occurs on exactly such systems.

- If all enabled watchdog devices have DT aliases, they all get their
assigned id.

- If no wathcdog device has a DT alias, they'll just get sequentially
assigned ids in probe order, and none of them will "accidentally" get an
id that should be assigned to a device with a DT alias.

> To apply this or a similar patch, we would have to ensure that there
> is no enabled watchdog with ID == 0.

I'm not sure I completely understand your concern(s), but I can see that
if there is any watchdog DT alias, we'll never use id 0 except if there
is a watchdog0 DT alias (and that device is actually enabled).

What if instead of assigning dynamic ids from above the highest existing
alias, we assign a dynamic id as usual, but skip existing aliases? So if
there's a watchdog1 alias, but no watchdog0 alias, the first unaliased
watchdog device being probed would become /dev/watchdog0 and hence
/dev/watchdog. Would that work?

Rasmus

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog: take all OF aliases into account when assigning id
  2026-07-13 14:35   ` Rasmus Villemoes
@ 2026-07-13 15:48     ` Guenter Roeck
  2026-07-14 11:08       ` Rasmus Villemoes
  0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2026-07-13 15:48 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel

On 7/13/26 07:35, Rasmus Villemoes wrote:
> On Wed, Jul 08 2026, "Guenter Roeck" <linux@roeck-us.net> wrote:
> 
>> On Mon, Jun 15, 2026 at 04:57:59PM +0200, Rasmus Villemoes wrote:
>>> If some, but not all, watchdog devices have device tree aliases, those
>>> without aliases might (depending on probe order) be assigned an id
>>> which would otherwise be assigned to one of those with an alias.
>>>
>>> This is problematic when for example watchdog0 is an alias for an
>>> always-running gpio watchdog that userspace must handle, but the SOC's
>>> watchdog device(s) get probed first and thus one of those become
>>> /dev/watchdog0.
>>>
>>> Ensure that ids for devices without a device tree alias are allocated
>>> from above the highest numbered alias, if any.
>>>
>>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>> ---
>>>
>>> This is similar to how the mmc, i2c, i3c and spi subsystems handle
>>> device tree aliases and avoid using an id that might be assigned to a
>>> device/bus that is probed later.
>>
>> The patch makes sense. Unfortunately, there are systems with aliased
>> watchdogs which do not enable "watchdog0" (e.g., several Nuvoton based
>> boards). On such systems, if they do have an unaliased / auto-generated
>> watchdog, /dev/watchdog0 and with it /dev/watchdog would no longer be
>> created. This would result in a ABI break.
>>
>> On top of that, the patch only affects systems with both aliased and
>> un-aliased watchdogs, which makes me even more concerned.
> 
> Well, yes, the problem only occurs on exactly such systems.
> 
> - If all enabled watchdog devices have DT aliases, they all get their
> assigned id.
> 
> - If no wathcdog device has a DT alias, they'll just get sequentially
> assigned ids in probe order, and none of them will "accidentally" get an
> id that should be assigned to a device with a DT alias.
> 
>> To apply this or a similar patch, we would have to ensure that there
>> is no enabled watchdog with ID == 0.
> 
> I'm not sure I completely understand your concern(s), but I can see that
> if there is any watchdog DT alias, we'll never use id 0 except if there
> is a watchdog0 DT alias (and that device is actually enabled).
> 
... and if ID 0 is not used in that situaton, /dev/watchdog will not be
created since it is tied to /dev/watchdog0.

> What if instead of assigning dynamic ids from above the highest existing
> alias, we assign a dynamic id as usual, but skip existing aliases? So if
> there's a watchdog1 alias, but no watchdog0 alias, the first unaliased
> watchdog device being probed would become /dev/watchdog0 and hence
> /dev/watchdog. Would that work?
> 

Yes, since that would not change existing behavior.

Is this an problem that is actually observed on some system, or a theoretic
one ?

Thanks,
Guenter


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] watchdog: take all OF aliases into account when assigning id
  2026-07-13 15:48     ` Guenter Roeck
@ 2026-07-14 11:08       ` Rasmus Villemoes
  0 siblings, 0 replies; 5+ messages in thread
From: Rasmus Villemoes @ 2026-07-14 11:08 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: Wim Van Sebroeck, linux-watchdog, linux-kernel

On Mon, Jul 13 2026, "Guenter Roeck" <linux@roeck-us.net> wrote:

> On 7/13/26 07:35, Rasmus Villemoes wrote:
>> On Wed, Jul 08 2026, "Guenter Roeck" <linux@roeck-us.net> wrote:
>>
>>> On Mon, Jun 15, 2026 at 04:57:59PM +0200, Rasmus Villemoes wrote:
>>>> If some, but not all, watchdog devices have device tree aliases, those
>>>> without aliases might (depending on probe order) be assigned an id
>>>> which would otherwise be assigned to one of those with an alias.
>>>>
>>>> This is problematic when for example watchdog0 is an alias for an
>>>> always-running gpio watchdog that userspace must handle, but the SOC's
>>>> watchdog device(s) get probed first and thus one of those become
>>>> /dev/watchdog0.
>>>>
>>>> Ensure that ids for devices without a device tree alias are allocated
>>>> from above the highest numbered alias, if any.
>>>>
>>>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>>> ---
>>>>
>>>> This is similar to how the mmc, i2c, i3c and spi subsystems handle
>>>> device tree aliases and avoid using an id that might be assigned to a
>>>> device/bus that is probed later.
>>>
>>> The patch makes sense. Unfortunately, there are systems with aliased
>>> watchdogs which do not enable "watchdog0" (e.g., several Nuvoton based
>>> boards). On such systems, if they do have an unaliased / auto-generated
>>> watchdog, /dev/watchdog0 and with it /dev/watchdog would no longer be
>>> created. This would result in a ABI break.
>>>
>>> On top of that, the patch only affects systems with both aliased and
>>> un-aliased watchdogs, which makes me even more concerned.
>>
>> Well, yes, the problem only occurs on exactly such systems.
>>
>> - If all enabled watchdog devices have DT aliases, they all get their
>> assigned id.
>>
>> - If no wathcdog device has a DT alias, they'll just get sequentially
>> assigned ids in probe order, and none of them will "accidentally" get an
>> id that should be assigned to a device with a DT alias.
>>
>>> To apply this or a similar patch, we would have to ensure that there
>>> is no enabled watchdog with ID == 0.
>>
>> I'm not sure I completely understand your concern(s), but I can see that
>> if there is any watchdog DT alias, we'll never use id 0 except if there
>> is a watchdog0 DT alias (and that device is actually enabled).
>>
> ... and if ID 0 is not used in that situaton, /dev/watchdog will not be
> created since it is tied to /dev/watchdog0.
>
>> What if instead of assigning dynamic ids from above the highest existing
>> alias, we assign a dynamic id as usual, but skip existing aliases? So if
>> there's a watchdog1 alias, but no watchdog0 alias, the first unaliased
>> watchdog device being probed would become /dev/watchdog0 and hence
>> /dev/watchdog. Would that work?
>>
>
> Yes, since that would not change existing behavior.
>
> Is this an problem that is actually observed on some system, or a theoretic
> one ?

Well, a little of both. I did observe it on a board I'm working on, but
as I'm authoring the .dts, I can/could just ensure that all watchdog
devices get aliases, and then they'll get exactly the ids I expect, and
userspace will know which watchdog device(s) it must handle.

It just caught me a little by surprise that when I only had watchdog0 =
&gpio_watchdog and watchdog1 = &soc_watchdog0 aliases (because those
were the only two I initially cared about, but the SOC has 5 instances
of its watchdog IP), I ended up with

  /dev/watchdog0 -> soc watchdog1
  /dev/watchdog1 -> soc watchdog0
  /dev/watchdog2 -> soc watchdog2
  /dev/watchdog3 -> soc watchdog3
  /dev/watchdog4 -> soc watchdog4
  /dev/watchdog5 -> the gpio watchdog

And that was only after I enabled the driver for the soc watchdogs,
before that I only had the gpio watchdog, which of course became
watchdog0 as it should.

So since I knew the mmc and i2c subsystems had this mechanism to avoid
using ids that exist as aliases, I went to look at the watchdog code and
see how it did the "use alias id if possible", and saw that there was no
handling of the "some with, some without" alias case. So while I don't
really need this for the board I'm doing bringup on right now (I've
added aliases for all), it's something that could

I've sent a v2 where I use any available and not "reserved by alias" id.

Thanks,
Rasmus

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-07-14 11:08 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-06-15 14:57 [PATCH] watchdog: take all OF aliases into account when assigning id Rasmus Villemoes
2026-07-08 14:53 ` Guenter Roeck
2026-07-13 14:35   ` Rasmus Villemoes
2026-07-13 15:48     ` Guenter Roeck
2026-07-14 11:08       ` Rasmus Villemoes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome