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

Groups > linux.kernel > #1293835 > unrolled thread

[PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

Started byPratyush Anand <panand@redhat.com>
First post2015-12-17 13:30 +0100
Last post2015-12-21 17:20 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer Pratyush Anand <panand@redhat.com> - 2015-12-17 13:30 +0100
    Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class  in stead of pointer Guenter Roeck <linux@roeck-us.net> - 2015-12-17 16:00 +0100
      Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class  in stead of pointer Pratyush Anand <panand@redhat.com> - 2015-12-19 05:30 +0100
        Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class  in stead of pointer Guenter Roeck <linux@roeck-us.net> - 2015-12-19 17:30 +0100
        Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class  in stead of pointer Guenter Roeck <linux@roeck-us.net> - 2015-12-21 17:00 +0100
          Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class  in stead of pointer Pratyush Anand <panand@redhat.com> - 2015-12-21 17:20 +0100

#1293835 — [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

FromPratyush Anand <panand@redhat.com>
Date2015-12-17 13:30 +0100
Subject[PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer
Message-ID<qGFR0-7NE-1@gated-at.bofh.it>
We need few sysfs attributes to know different status of a watchdog device.
To do that, we need to associate .dev_groups with watchdog_class. So
convert it from pointer to static.
Putting this static struct in watchdog_dev.c, so that static device
attributes defined in that file can be attached to it.

Signed-off-by: Pratyush Anand <panand@redhat.com>
Suggested-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/watchdog/watchdog_core.c | 15 ++-------------
 drivers/watchdog/watchdog_core.h |  2 +-
 drivers/watchdog/watchdog_dev.c  | 26 ++++++++++++++++++++++----
 3 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 873f13972cf4..a55e846eec79 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -285,19 +285,9 @@ static int __init watchdog_deferred_registration(void)
 
 static int __init watchdog_init(void)
 {
-	int err;
-
-	watchdog_class = class_create(THIS_MODULE, "watchdog");
-	if (IS_ERR(watchdog_class)) {
-		pr_err("couldn't create class\n");
+	watchdog_class = watchdog_dev_init();
+	if (IS_ERR(watchdog_class))
 		return PTR_ERR(watchdog_class);
-	}
-
-	err = watchdog_dev_init();
-	if (err < 0) {
-		class_destroy(watchdog_class);
-		return err;
-	}
 
 	watchdog_deferred_registration();
 	return 0;
@@ -306,7 +296,6 @@ static int __init watchdog_init(void)
 static void __exit watchdog_exit(void)
 {
 	watchdog_dev_exit();
-	class_destroy(watchdog_class);
 	ida_destroy(&watchdog_ida);
 }
 
diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
index 6c951418fca7..1c8d6b0e68c7 100644
--- a/drivers/watchdog/watchdog_core.h
+++ b/drivers/watchdog/watchdog_core.h
@@ -33,5 +33,5 @@
  */
 extern int watchdog_dev_register(struct watchdog_device *);
 extern int watchdog_dev_unregister(struct watchdog_device *);
-extern int __init watchdog_dev_init(void);
+extern struct class * __init watchdog_dev_init(void);
 extern void __exit watchdog_dev_exit(void);
diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index 56a649e66eb2..055a4e1b4c13 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -581,18 +581,35 @@ int watchdog_dev_unregister(struct watchdog_device *wdd)
 	return 0;
 }
 
+static struct class watchdog_class = {
+	.name =		"watchdog",
+	.owner =	THIS_MODULE,
+};
+
 /*
  *	watchdog_dev_init: init dev part of watchdog core
  *
  *	Allocate a range of chardev nodes to use for watchdog devices
  */
 
-int __init watchdog_dev_init(void)
+struct class * __init watchdog_dev_init(void)
 {
-	int err = alloc_chrdev_region(&watchdog_devt, 0, MAX_DOGS, "watchdog");
-	if (err < 0)
+	int err;
+
+	err = class_register(&watchdog_class);
+	if (err < 0) {
+		pr_err("couldn't register class\n");
+		return ERR_PTR(err);
+	}
+
+	err = alloc_chrdev_region(&watchdog_devt, 0, MAX_DOGS, "watchdog");
+	if (err < 0) {
 		pr_err("watchdog: unable to allocate char dev region\n");
-	return err;
+		class_unregister(&watchdog_class);
+		return ERR_PTR(err);
+	}
+
+	return &watchdog_class;
 }
 
 /*
@@ -604,4 +621,5 @@ int __init watchdog_dev_init(void)
 void __exit watchdog_dev_exit(void)
 {
 	unregister_chrdev_region(watchdog_devt, MAX_DOGS);
+	class_unregister(&watchdog_class);
 }
-- 
2.5.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]

#1293950 — Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

FromGuenter Roeck <linux@roeck-us.net>
Date2015-12-17 16:00 +0100
SubjectRe: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer
Message-ID<qGIca-Ma-13@gated-at.bofh.it>
In reply to#1293835
On 12/17/2015 04:23 AM, Pratyush Anand wrote:
> We need few sysfs attributes to know different status of a watchdog device.
> To do that, we need to associate .dev_groups with watchdog_class. So
> convert it from pointer to static.
> Putting this static struct in watchdog_dev.c, so that static device
> attributes defined in that file can be attached to it.
>
> Signed-off-by: Pratyush Anand <panand@redhat.com>
> Suggested-by: Guenter Roeck <linux@roeck-us.net>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

As things evolve, I'd by now prefer to move the call to device_create()
into watchdog_dev.c, but that can wait for a follow-up patch if Wim
is ok with this series.

Thanks,
Guenter

> ---
>   drivers/watchdog/watchdog_core.c | 15 ++-------------
>   drivers/watchdog/watchdog_core.h |  2 +-
>   drivers/watchdog/watchdog_dev.c  | 26 ++++++++++++++++++++++----
>   3 files changed, 25 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 873f13972cf4..a55e846eec79 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -285,19 +285,9 @@ static int __init watchdog_deferred_registration(void)
>
>   static int __init watchdog_init(void)
>   {
> -	int err;
> -
> -	watchdog_class = class_create(THIS_MODULE, "watchdog");
> -	if (IS_ERR(watchdog_class)) {
> -		pr_err("couldn't create class\n");
> +	watchdog_class = watchdog_dev_init();
> +	if (IS_ERR(watchdog_class))
>   		return PTR_ERR(watchdog_class);
> -	}
> -
> -	err = watchdog_dev_init();
> -	if (err < 0) {
> -		class_destroy(watchdog_class);
> -		return err;
> -	}
>
>   	watchdog_deferred_registration();
>   	return 0;
> @@ -306,7 +296,6 @@ static int __init watchdog_init(void)
>   static void __exit watchdog_exit(void)
>   {
>   	watchdog_dev_exit();
> -	class_destroy(watchdog_class);
>   	ida_destroy(&watchdog_ida);
>   }
>
> diff --git a/drivers/watchdog/watchdog_core.h b/drivers/watchdog/watchdog_core.h
> index 6c951418fca7..1c8d6b0e68c7 100644
> --- a/drivers/watchdog/watchdog_core.h
> +++ b/drivers/watchdog/watchdog_core.h
> @@ -33,5 +33,5 @@
>    */
>   extern int watchdog_dev_register(struct watchdog_device *);
>   extern int watchdog_dev_unregister(struct watchdog_device *);
> -extern int __init watchdog_dev_init(void);
> +extern struct class * __init watchdog_dev_init(void);
>   extern void __exit watchdog_dev_exit(void);
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index 56a649e66eb2..055a4e1b4c13 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -581,18 +581,35 @@ int watchdog_dev_unregister(struct watchdog_device *wdd)
>   	return 0;
>   }
>
> +static struct class watchdog_class = {
> +	.name =		"watchdog",
> +	.owner =	THIS_MODULE,
> +};
> +
>   /*
>    *	watchdog_dev_init: init dev part of watchdog core
>    *
>    *	Allocate a range of chardev nodes to use for watchdog devices
>    */
>
> -int __init watchdog_dev_init(void)
> +struct class * __init watchdog_dev_init(void)
>   {
> -	int err = alloc_chrdev_region(&watchdog_devt, 0, MAX_DOGS, "watchdog");
> -	if (err < 0)
> +	int err;
> +
> +	err = class_register(&watchdog_class);
> +	if (err < 0) {
> +		pr_err("couldn't register class\n");
> +		return ERR_PTR(err);
> +	}
> +
> +	err = alloc_chrdev_region(&watchdog_devt, 0, MAX_DOGS, "watchdog");
> +	if (err < 0) {
>   		pr_err("watchdog: unable to allocate char dev region\n");
> -	return err;
> +		class_unregister(&watchdog_class);
> +		return ERR_PTR(err);
> +	}
> +
> +	return &watchdog_class;
>   }
>
>   /*
> @@ -604,4 +621,5 @@ int __init watchdog_dev_init(void)
>   void __exit watchdog_dev_exit(void)
>   {
>   	unregister_chrdev_region(watchdog_devt, MAX_DOGS);
> +	class_unregister(&watchdog_class);
>   }
>

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

#1295299 — Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

FromPratyush Anand <panand@redhat.com>
Date2015-12-19 05:30 +0100
SubjectRe: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer
Message-ID<qHhjA-6Mt-3@gated-at.bofh.it>
In reply to#1293950
On 17/12/2015:06:56:27 AM, Guenter Roeck wrote:
> On 12/17/2015 04:23 AM, Pratyush Anand wrote:
> >We need few sysfs attributes to know different status of a watchdog device.
> >To do that, we need to associate .dev_groups with watchdog_class. So
> >convert it from pointer to static.
> >Putting this static struct in watchdog_dev.c, so that static device
> >attributes defined in that file can be attached to it.
> >
> >Signed-off-by: Pratyush Anand <panand@redhat.com>
> >Suggested-by: Guenter Roeck <linux@roeck-us.net>
> >Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> 
> As things evolve, I'd by now prefer to move the call to device_create()
> into watchdog_dev.c, but that can wait for a follow-up patch if Wim
> is ok with this series.

Thanks for your quick review.

OK. I will wait for Wim's comment and then may be I will send another
version with your comment for patch 1/1 incorporated.

~Pratyush
--
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]

#1295400 — Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

FromGuenter Roeck <linux@roeck-us.net>
Date2015-12-19 17:30 +0100
SubjectRe: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer
Message-ID<qHsym-5vX-5@gated-at.bofh.it>
In reply to#1295299
On 12/18/2015 08:22 PM, Pratyush Anand wrote:
> On 17/12/2015:06:56:27 AM, Guenter Roeck wrote:
>> On 12/17/2015 04:23 AM, Pratyush Anand wrote:
>>> We need few sysfs attributes to know different status of a watchdog device.
>>> To do that, we need to associate .dev_groups with watchdog_class. So
>>> convert it from pointer to static.
>>> Putting this static struct in watchdog_dev.c, so that static device
>>> attributes defined in that file can be attached to it.
>>>
>>> Signed-off-by: Pratyush Anand <panand@redhat.com>
>>> Suggested-by: Guenter Roeck <linux@roeck-us.net>
>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>> As things evolve, I'd by now prefer to move the call to device_create()
>> into watchdog_dev.c, but that can wait for a follow-up patch if Wim
>> is ok with this series.
>
> Thanks for your quick review.
>
> OK. I will wait for Wim's comment and then may be I will send another
> version with your comment for patch 1/1 incorporated.
>

I reworked my patch to call device_create() from watchdog_dev.c on top of
your patches and on top of Wim's watchdog-next branch. Hopefully we can get
that into v4.5.

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] | [next] | [standalone]

#1296044 — Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

FromGuenter Roeck <linux@roeck-us.net>
Date2015-12-21 17:00 +0100
SubjectRe: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer
Message-ID<qIb2q-8oD-3@gated-at.bofh.it>
In reply to#1295299
On 12/18/2015 08:22 PM, Pratyush Anand wrote:
> On 17/12/2015:06:56:27 AM, Guenter Roeck wrote:
>> On 12/17/2015 04:23 AM, Pratyush Anand wrote:
>>> We need few sysfs attributes to know different status of a watchdog device.
>>> To do that, we need to associate .dev_groups with watchdog_class. So
>>> convert it from pointer to static.
>>> Putting this static struct in watchdog_dev.c, so that static device
>>> attributes defined in that file can be attached to it.
>>>
>>> Signed-off-by: Pratyush Anand <panand@redhat.com>
>>> Suggested-by: Guenter Roeck <linux@roeck-us.net>
>>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>> As things evolve, I'd by now prefer to move the call to device_create()
>> into watchdog_dev.c, but that can wait for a follow-up patch if Wim
>> is ok with this series.
>
> Thanks for your quick review.
>
> OK. I will wait for Wim's comment and then may be I will send another
> version with your comment for patch 1/1 incorporated.
>

Thinking about it, my comment is really minor and should not hold up the
series from going forward. So let's just skip another version unless Wim
or someone else has any comments.

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] | [next] | [standalone]

#1296056 — Re: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer

FromPratyush Anand <panand@redhat.com>
Date2015-12-21 17:20 +0100
SubjectRe: [PATCH V5 1/2] watchdog: Use static struct class watchdog_class in stead of pointer
Message-ID<qIblN-jv-15@gated-at.bofh.it>
In reply to#1296044
On 21/12/2015:07:52:59 AM, Guenter Roeck wrote:
> On 12/18/2015 08:22 PM, Pratyush Anand wrote:
> >On 17/12/2015:06:56:27 AM, Guenter Roeck wrote:
> >>On 12/17/2015 04:23 AM, Pratyush Anand wrote:
> >>>We need few sysfs attributes to know different status of a watchdog device.
> >>>To do that, we need to associate .dev_groups with watchdog_class. So
> >>>convert it from pointer to static.
> >>>Putting this static struct in watchdog_dev.c, so that static device
> >>>attributes defined in that file can be attached to it.
> >>>
> >>>Signed-off-by: Pratyush Anand <panand@redhat.com>
> >>>Suggested-by: Guenter Roeck <linux@roeck-us.net>
> >>>Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> >>
> >>As things evolve, I'd by now prefer to move the call to device_create()
> >>into watchdog_dev.c, but that can wait for a follow-up patch if Wim
> >>is ok with this series.
> >
> >Thanks for your quick review.
> >
> >OK. I will wait for Wim's comment and then may be I will send another
> >version with your comment for patch 1/1 incorporated.
> >
> 
> Thinking about it, my comment is really minor and should not hold up the
> series from going forward. So let's just skip another version unless Wim
> or someone else has any comments.

Thanks a lot :-)

~Pratyush
--
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