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 <[email protected]>
Suggested-by: Guenter Roeck <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
---
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
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 <[email protected]>
> Suggested-by: Guenter Roeck <[email protected]>
> Reviewed-by: Guenter Roeck <[email protected]>
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);
> }
>
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 <[email protected]>
> >Suggested-by: Guenter Roeck <[email protected]>
> >Reviewed-by: Guenter Roeck <[email protected]>
>
> 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
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 <[email protected]>
>>> Suggested-by: Guenter Roeck <[email protected]>
>>> Reviewed-by: Guenter Roeck <[email protected]>
>>
>> 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
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 <[email protected]>
>>> Suggested-by: Guenter Roeck <[email protected]>
>>> Reviewed-by: Guenter Roeck <[email protected]>
>>
>> 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
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 <[email protected]>
> >>>Suggested-by: Guenter Roeck <[email protected]>
> >>>Reviewed-by: Guenter Roeck <[email protected]>
> >>
> >>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