[PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Jingoo Han]

These functios allows the driver core to automatically clean up
any allocation made by rtc drivers. Thus, it simplifies the error
paths.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
Changes since v2:
- Removed unnecessary WARN_ON() of devm_rtc_device_match()

Changes since v1:
- Simplified 'if' statements using WARN_ON()
- Added a description of the return value of devm_rtc_device_register()

 drivers/rtc/class.c |   70 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/rtc.h |    6 ++++
 2 files changed, 76 insertions(+), 0 deletions(-)

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 9b742d3..b72b40b 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -259,6 +259,76 @@ void rtc_device_unregister(struct rtc_device *rtc)
 }
 EXPORT_SYMBOL_GPL(rtc_device_unregister);
 
+static void devm_rtc_device_release(struct device *dev, void *res)
+{
+	struct rtc_device *rtc = *(struct rtc_device **)res;
+
+	rtc_device_unregister(rtc);
+}
+
+static int devm_rtc_device_match(struct device *dev, void *res, void *data)
+{
+	struct rtc **r = res;
+
+	return *r == data;
+}
+
+/**
+ * devm_rtc_device_register - resource managed rtc_device_register()
+ * @name: the name of the device
+ * @dev: the device to register
+ * @ops: the rtc operations structure
+ * @owner: the module owner
+ *
+ * @return a struct rtc on success, or an ERR_PTR on error
+ *
+ * Managed rtc_device_register(). The rtc_device returned from this function
+ * are automatically freed on driver detach. See rtc_device_register()
+ * for more information.
+ */
+
+struct rtc_device *devm_rtc_device_register(const char *name,
+					struct device *dev,
+					const struct rtc_class_ops *ops,
+					struct module *owner)
+{
+	struct rtc_device **ptr, *rtc;
+
+	ptr = devres_alloc(devm_rtc_device_release, sizeof(*ptr), GFP_KERNEL);
+	if (!ptr)
+		return ERR_PTR(-ENOMEM);
+
+	rtc = rtc_device_register(name, dev, ops, owner);
+	if (!IS_ERR(rtc)) {
+		*ptr = rtc;
+		devres_add(dev, ptr);
+	} else {
+		devres_free(ptr);
+	}
+
+	return rtc;
+}
+EXPORT_SYMBOL_GPL(devm_rtc_device_register);
+
+/**
+ * devm_rtc_device_unregister - resource managed devm_rtc_device_unregister()
+ * @dev: the device to unregister
+ * @rtc: the RTC class device to unregister
+ *
+ * Deallocated a rtc allocated with devm_rtc_device_register(). Normally this
+ * function will not need to be called and the resource management code will
+ * ensure that the resource is freed.
+ */
+void devm_rtc_device_unregister(struct device *dev, struct rtc_device *rtc)
+{
+	int rc;
+
+	rc = devres_release(dev, devm_rtc_device_release,
+				devm_rtc_device_match, rtc);
+	WARN_ON(rc);
+}
+EXPORT_SYMBOL_GPL(devm_rtc_device_unregister);
+
 static int __init rtc_init(void)
 {
 	rtc_class = class_create(THIS_MODULE, "rtc");
diff --git a/include/linux/rtc.h b/include/linux/rtc.h
index 580b24c..d955768 100644
--- a/include/linux/rtc.h
+++ b/include/linux/rtc.h
@@ -133,7 +133,13 @@ extern struct rtc_device *rtc_device_register(const char *name,
 					struct device *dev,
 					const struct rtc_class_ops *ops,
 					struct module *owner);
+extern struct rtc_device *devm_rtc_device_register(const char *name,
+					struct device *dev,
+					const struct rtc_class_ops *ops,
+					struct module *owner);
 extern void rtc_device_unregister(struct rtc_device *rtc);
+extern void devm_rtc_device_unregister(struct device *dev,
+					struct rtc_device *rtc);
 
 extern int rtc_read_time(struct rtc_device *rtc, struct rtc_time *tm);
 extern int rtc_set_time(struct rtc_device *rtc, struct rtc_time *tm);
-- 
1.7.2.5

Re: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

['Tejun Heo']

On Tue, Feb 26, 2013 at 10:49:57AM +0900, Jingoo Han wrote:
> These functios allows the driver core to automatically clean up
> any allocation made by rtc drivers. Thus, it simplifies the error
> paths.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Acked-by: Tejun Heo <tj@kernel.org>

Thanks.

-- 
tejun

RE: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Venu Byravarasu]

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> owner@vger.kernel.org] On Behalf Of Jingoo Han
> Sent: Tuesday, February 26, 2013 7:20 AM
> To: 'Andrew Morton'
> Cc: linux-kernel@vger.kernel.org; 'Tejun Heo'; 'Greg KH'; 'Alessandro
> Zummo'; rtc-linux@googlegroups.com; 'Jingoo Han'
> Subject: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()
> 
> These functios allows the driver core to automatically clean up
> any allocation made by rtc drivers. Thus, it simplifies the error
> paths.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
> Changes since v2:
> - Removed unnecessary WARN_ON() of devm_rtc_device_match()
> 
> Changes since v1:
> - Simplified 'if' statements using WARN_ON()
> - Added a description of the return value of devm_rtc_device_register()
> 
>  drivers/rtc/class.c |   70
> +++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/rtc.h |    6 ++++
>  2 files changed, 76 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> index 9b742d3..b72b40b 100644
> --- a/drivers/rtc/class.c
> +++ b/drivers/rtc/class.c

> +/**
> + * devm_rtc_device_register - resource managed rtc_device_register()
> + * @name: the name of the device
> + * @dev: the device to register
> + * @ops: the rtc operations structure
> + * @owner: the module owner
> + *
> + * @return a struct rtc on success, or an ERR_PTR on error
> + *
> + * Managed rtc_device_register(). The rtc_device returned from this
> function
> + * are automatically freed on driver detach. See rtc_device_register()
> + * for more information.
> + */
> +
> +struct rtc_device *devm_rtc_device_register(const char *name,
> +					struct device *dev,

As most of devm_* functions use  " struct device *dev" as their first param,
why not this function also modified to be in sync with them?

> +					const struct rtc_class_ops *ops,
> +					struct module *owner)
> +{
> +	struct rtc_device **ptr, *rtc;

Re: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Stephen Boyd]

On 03/05/13 22:03, Venu Byravarasu wrote:
>
>> +/**
>> + * devm_rtc_device_register - resource managed rtc_device_register()
>> + * @name: the name of the device
>> + * @dev: the device to register
>> + * @ops: the rtc operations structure
>> + * @owner: the module owner
>> + *
>> + * @return a struct rtc on success, or an ERR_PTR on error
>> + *
>> + * Managed rtc_device_register(). The rtc_device returned from this
>> function
>> + * are automatically freed on driver detach. See rtc_device_register()
>> + * for more information.
>> + */
>> +
>> +struct rtc_device *devm_rtc_device_register(const char *name,
>> +					struct device *dev,
> As most of devm_* functions use  " struct device *dev" as their first param,
> why not this function also modified to be in sync with them?

I suspect it's because the signature matches rtc_device_register(). This
way you can do a simple search and replace and avoid having to reorder
arguments.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Jingoo Han]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 1558 bytes --]

On Wednesday, March 06, 2013 3:03 PM, Venu Byravarasu wrote:
> 
> > --- a/drivers/rtc/class.c
> > +++ b/drivers/rtc/class.c
> 
> > +/**
> > + * devm_rtc_device_register - resource managed rtc_device_register()
> > + * @name: the name of the device
> > + * @dev: the device to register
> > + * @ops: the rtc operations structure
> > + * @owner: the module owner
> > + *
> > + * @return a struct rtc on success, or an ERR_PTR on error
> > + *
> > + * Managed rtc_device_register(). The rtc_device returned from this
> > function
> > + * are automatically freed on driver detach. See rtc_device_register()
> > + * for more information.
> > + */
> > +
> > +struct rtc_device *devm_rtc_device_register(const char *name,
> > +					struct device *dev,
> 
> As most of devm_* functions use  " struct device *dev" as their first param,
> why not this function also modified to be in sync with them?

Yes, but, I want to sync with the form of rtc_device_register().
This function already uses 'struct device *dev' as second argument as below.

 struct rtc_device *rtc_device_register(const char *name, struct device *dev,
                                         const struct rtc_class_ops *ops,
                                         struct module *owner)

Best regards,
Jingoo Han

> 
> > +					const struct rtc_class_ops *ops,
> > +					struct module *owner)
> > +{
> > +	struct rtc_device **ptr, *rtc;
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Venu Byravarasu]

> -----Original Message-----
> From: Jingoo Han [mailto:jg1.han@samsung.com]
> Sent: Wednesday, March 06, 2013 11:43 AM
> To: Venu Byravarasu
> Cc: 'Andrew Morton'; linux-kernel@vger.kernel.org; 'Tejun Heo'; 'Greg KH';
> 'Alessandro Zummo'; rtc-linux@googlegroups.com; Jingoo Han
> Subject: Re: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()
> 
> On Wednesday, March 06, 2013 3:03 PM, Venu Byravarasu wrote:
> >
> > > --- a/drivers/rtc/class.c
> > > +++ b/drivers/rtc/class.c
> >
> > > +/**
> > > + * devm_rtc_device_register - resource managed rtc_device_register()
> > > + * @name: the name of the device
> > > + * @dev: the device to register
> > > + * @ops: the rtc operations structure
> > > + * @owner: the module owner
> > > + *
> > > + * @return a struct rtc on success, or an ERR_PTR on error
> > > + *
> > > + * Managed rtc_device_register(). The rtc_device returned from this
> > > function
> > > + * are automatically freed on driver detach. See rtc_device_register()
> > > + * for more information.
> > > + */
> > > +
> > > +struct rtc_device *devm_rtc_device_register(const char *name,
> > > +					struct device *dev,
> >
> > As most of devm_* functions use  " struct device *dev" as their first param,
> > why not this function also modified to be in sync with them?
> 
> Yes, but, I want to sync with the form of rtc_device_register().
> This function already uses 'struct device *dev' as second argument as below.
> 

IMO any kernel driver developer using devm_* API, expects struct device* as first argument.
Breaking this policy for one module (RTC here) & making this API special, might not be a good idea.
Anyhow, maintainers may add their comments on this.

Thanks,
Venu

>  struct rtc_device *rtc_device_register(const char *name, struct device *dev,
>                                          const struct rtc_class_ops *ops,
>                                          struct module *owner)
> 
> Best regards,
> Jingoo Han
> 
> >
> > > +					const struct rtc_class_ops *ops,
> > > +					struct module *owner)
> > > +{
> > > +	struct rtc_device **ptr, *rtc;

Re: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Jingoo Han]

On Wednesday, March 06, 2013 3:19 PM, Venu Byravarasu wrote:
> On Wednesday, March 06, 2013 11:43 AM, Jingoo Han wrote:
> >
> > On Wednesday, March 06, 2013 3:03 PM, Venu Byravarasu wrote:
> > >
> > > > --- a/drivers/rtc/class.c
> > > > +++ b/drivers/rtc/class.c
> > >
> > > > +/**
> > > > + * devm_rtc_device_register - resource managed rtc_device_register()
> > > > + * @name: the name of the device
> > > > + * @dev: the device to register
> > > > + * @ops: the rtc operations structure
> > > > + * @owner: the module owner
> > > > + *
> > > > + * @return a struct rtc on success, or an ERR_PTR on error
> > > > + *
> > > > + * Managed rtc_device_register(). The rtc_device returned from this
> > > > function
> > > > + * are automatically freed on driver detach. See rtc_device_register()
> > > > + * for more information.
> > > > + */
> > > > +
> > > > +struct rtc_device *devm_rtc_device_register(const char *name,
> > > > +					struct device *dev,
> > >
> > > As most of devm_* functions use  " struct device *dev" as their first param,
> > > why not this function also modified to be in sync with them?
> >
> > Yes, but, I want to sync with the form of rtc_device_register().
> > This function already uses 'struct device *dev' as second argument as below.
> >
> 
> IMO any kernel driver developer using devm_* API, expects struct device* as first argument.
> Breaking this policy for one module (RTC here) & making this API special, might not be a good idea.
> Anyhow, maintainers may add their comments on this.

OK, I see.

I just look at other devm_* API. All devm_* APIs use 'struct device*'
as first argument.

To prevent the confusion, I will send the patch that uses
'struct device*' as first argument as below.

 struct rtc_device *devm_rtc_device_register(struct device *dev,
                                         const char *name,

Thanks for your comment

Best regards,
Jingoo Han

> 
> Thanks,
> Venu
> 
> >  struct rtc_device *rtc_device_register(const char *name, struct device *dev,
> >                                          const struct rtc_class_ops *ops,
> >                                          struct module *owner)
> >
> > Best regards,
> > Jingoo Han
> >
> > >
> > > > +					const struct rtc_class_ops *ops,
> > > > +					struct module *owner)
> > > > +{
> > > > +	struct rtc_device **ptr, *rtc;

Re: [PATCH v3] rtc: add devm_rtc_device_{register,unregister}()

[Jingoo Han]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 2497 bytes --]

On Wednesday, March 06, 2013 3:19 PM, Venu Byravarasu wrote:
> On Wednesday, March 06, 2013 11:43 AM, Jingoo Han wrote:
> >
> > On Wednesday, March 06, 2013 3:03 PM, Venu Byravarasu wrote:
> > >
> > > > --- a/drivers/rtc/class.c
> > > > +++ b/drivers/rtc/class.c
> > >
> > > > +/**
> > > > + * devm_rtc_device_register - resource managed rtc_device_register()
> > > > + * @name: the name of the device
> > > > + * @dev: the device to register
> > > > + * @ops: the rtc operations structure
> > > > + * @owner: the module owner
> > > > + *
> > > > + * @return a struct rtc on success, or an ERR_PTR on error
> > > > + *
> > > > + * Managed rtc_device_register(). The rtc_device returned from this
> > > > function
> > > > + * are automatically freed on driver detach. See rtc_device_register()
> > > > + * for more information.
> > > > + */
> > > > +
> > > > +struct rtc_device *devm_rtc_device_register(const char *name,
> > > > +					struct device *dev,
> > >
> > > As most of devm_* functions use  " struct device *dev" as their first param,
> > > why not this function also modified to be in sync with them?
> >
> > Yes, but, I want to sync with the form of rtc_device_register().
> > This function already uses 'struct device *dev' as second argument as below.
> >
> 
> IMO any kernel driver developer using devm_* API, expects struct device* as first argument.
> Breaking this policy for one module (RTC here) & making this API special, might not be a good idea.
> Anyhow, maintainers may add their comments on this.

OK, I see.

I just look at other devm_* API. All devm_* APIs use 'struct device*'
as first argument.

To prevent the confusion, I will send the patch that uses
'struct device*' as first argument as below.

 struct rtc_device *devm_rtc_device_register(struct device *dev,
                                         const char *name,

Thanks for your comment

Best regards,
Jingoo Han

> 
> Thanks,
> Venu
> 
> >  struct rtc_device *rtc_device_register(const char *name, struct device *dev,
> >                                          const struct rtc_class_ops *ops,
> >                                          struct module *owner)
> >
> > Best regards,
> > Jingoo Han
> >
> > >
> > > > +					const struct rtc_class_ops *ops,
> > > > +					struct module *owner)
> > > > +{
> > > > +	struct rtc_device **ptr, *rtc;
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥