Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1215582 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH] watchdog_dev: Use device tree alias for naming watchdogs Justin Chen <justinpopo6@gmail.com> - 2015-08-29 00:00 +0200
    Re: [PATCH] watchdog_dev: Use device tree alias for naming watchdogs Guenter Roeck <linux@roeck-us.net> - 2015-08-29 12:00 +0200
      Re: [PATCH] watchdog_dev: Use device tree alias for naming watchdogs Guenter Roeck <linux@roeck-us.net> - 2015-09-01 20:00 +0200

#1215582 — [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 <justinpopo6@gmail.com>
---
 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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1215745

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 <justinpopo6@gmail.com>

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;
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1217002

On Tue, Sep 01, 2015 at 10:20:36AM -0700, Justin Chen wrote:
> On Sat, Aug 29, 2015 at 2:53 AM, Guenter Roeck <linux@roeck-us.net> wrote:
> > 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 <justinpopo6@gmail.com>
> >
> >
> > 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
> 
> Ok that makes sense. I will put a wrapper function around ida_simple_get()
> that will request a specific number, if that fails then do a normal request.
> Something like this...
> 
> ret = of_alias_get_id(....)
> 
> id = ida_simple_get(&watchdog_ida, ret, ret, GFP_KERNEL);
> 
				ret, ret + 1

but something like
	ret =  of_alias_get_id(...);
	if (ret >= 0)
		id = ida_simple_get(&watchdog_ida, ret, ret + 1, GFP_KERNEL);
	else
		id = ida_simple_get(&watchdog_ida, 0, MAX_DOGS, GFP_KERNEL);
	if (id < 0)
		return id;

would be better. Passing 'ret' directly would not work, because
ida_simple_get() expects an unsigned range as parameters, and
of_alias_get_id() can return a negative error code.

Thanks,
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web