2015-08-28 21:58:53

by Justin Chen

Subject: [PATCH] watchdog_dev: Use device tree alias for naming watchdogs

Currently there is no way to easily differentiate multiple
watchdog devices. The watchdogs are named by the order they
are probed.
1st probed watchdog: /dev/watchdog0
2nd probed watchdog: /dev/watchdog1
...

This change uses the alias of the watchdog device node for
the name of the watchdog.
aliases {
watchdog0 = "/...../...."
watchdog3 = "/..../....."
watchdog2 = "/..../....."
...
}

This will translate to...
/dev/watchdog0
/dev/watchdog3
/dev/watchdog2

Signed-off-by: Justin Chen <[email protected]>
---
drivers/watchdog/watchdog_dev.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 6aaefba..52b1f0b 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -41,6 +41,7 @@
#include <linux/miscdevice.h> /* For handling misc devices */
#include <linux/init.h> /* For __init/__exit/... */
#include <linux/uaccess.h> /* For copy_to_user/put_user/... */
+#include <linux/of.h>

#include "watchdog_core.h"

@@ -522,7 +523,13 @@ static struct miscdevice watchdog_miscdev = {

int watchdog_dev_register(struct watchdog_device *watchdog)
{
- int err, devno;
+ int err, devno, ret;
+
+ if (watchdog->parent) {
+ ret = of_alias_get_id(watchdog->parent->of_node, "watchdog");
+ if (ret >= 0)
+ watchdog->id = ret;
+ }

if (watchdog->id == 0) {
old_wdd = watchdog;
--
2.1.0

2015-08-29 09:53:36

by Guenter Roeck

Subject: Re: [PATCH] watchdog_dev: Use device tree alias for naming watchdogs

On 08/28/2015 02:58 PM, Justin Chen wrote:
> Currently there is no way to easily differentiate multiple
> watchdog devices. The watchdogs are named by the order they
> are probed.
> 1st probed watchdog: /dev/watchdog0
> 2nd probed watchdog: /dev/watchdog1
> ...
>
> This change uses the alias of the watchdog device node for
> the name of the watchdog.
> aliases {
> watchdog0 = "/...../...."
> watchdog3 = "/..../....."
> watchdog2 = "/..../....."
> ...
> }
>
> This will translate to...
> /dev/watchdog0
> /dev/watchdog3
> /dev/watchdog2
>
> Signed-off-by: Justin Chen <[email protected]>

Interesting idea. Checking through other subsystems, many others do the same,
so it makes sense to use that mechanism.

However, the id assignment should be in the calling code, in __watchdog_register_device,
to avoid that another id, possibly conflicting, is assigned through the ida mechanism.

This is a bit more complicated than it looks like to ensure correct id assignment.
Have a look into the i2c code to see how it is handled. Essentially we must pass
the requested number to ida_simple_get().

Thanks,
Guenter

> ---
> drivers/watchdog/watchdog_dev.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 6aaefba..52b1f0b 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -41,6 +41,7 @@
> #include <linux/miscdevice.h> /* For handling misc devices */
> #include <linux/init.h> /* For __init/__exit/... */
> #include <linux/uaccess.h> /* For copy_to_user/put_user/... */
> +#include <linux/of.h>
>
> #include "watchdog_core.h"
>
> @@ -522,7 +523,13 @@ static struct miscdevice watchdog_miscdev = {
>
> int watchdog_dev_register(struct watchdog_device *watchdog)
> {
> - int err, devno;
> + int err, devno, ret;
> +
> + if (watchdog->parent) {
> + ret = of_alias_get_id(watchdog->parent->of_node, "watchdog");
> + if (ret >= 0)
> + watchdog->id = ret;
> + }
>
> if (watchdog->id == 0) {
> old_wdd = watchdog;
>