All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()
@ 2013-02-26  1:21 Jingoo Han
  2013-02-26  1:23 ` 'Tejun Heo'
  2013-02-26 22:33 ` Dmitry Torokhov
  0 siblings, 2 replies; 6+ messages in thread
From: Jingoo Han @ 2013-02-26  1:21 UTC (permalink / raw)
  To: 'Andrew Morton'
  Cc: linux-kernel, 'Tejun Heo', 'Greg KH',
	'Alessandro Zummo', rtc-linux, '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 v1:
- Simplified 'if' statements using WARN_ON()
- Added a description of the return value of devm_rtc_device_register()

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

diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
index 9b742d3..31496bc 100644
--- a/drivers/rtc/class.c
+++ b/drivers/rtc/class.c
@@ -259,6 +259,78 @@ 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;
+	if (WARN_ON(!r || !*r))
+		return 0;
+
+	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



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()
  2013-02-26  1:21 [PATCH V2] rtc: add devm_rtc_device_{register,unregister}() Jingoo Han
@ 2013-02-26  1:23 ` 'Tejun Heo'
  2013-02-26  1:30   ` 'Tejun Heo'
  2013-02-26 22:33 ` Dmitry Torokhov
  1 sibling, 1 reply; 6+ messages in thread
From: 'Tejun Heo' @ 2013-02-26  1:23 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Andrew Morton', linux-kernel, 'Greg KH',
	'Alessandro Zummo',
	rtc-linux

Hello,

On Tue, Feb 26, 2013 at 10:21:06AM +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>

Looks okay to me but

> +static int devm_rtc_device_match(struct device *dev, void *res, void *data)
> +{
> +	struct rtc **r = res;
> +	if (WARN_ON(!r || !*r))
> +		return 0;
> +
> +	return *r == data;
> +}

again, why do we need the WARN_ON() at all given that other
devm_*_match() don't do that and the only way to get NULL there would
be bug in devm_rtc*() code itself rather than its users?

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()
  2013-02-26  1:23 ` 'Tejun Heo'
@ 2013-02-26  1:30   ` 'Tejun Heo'
  2013-02-26  1:42     ` Jingoo Han
  0 siblings, 1 reply; 6+ messages in thread
From: 'Tejun Heo' @ 2013-02-26  1:30 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Andrew Morton', linux-kernel, 'Greg KH',
	'Alessandro Zummo',
	rtc-linux

On Mon, Feb 25, 2013 at 05:23:17PM -0800, 'Tejun Heo' wrote:
> again, why do we need the WARN_ON() at all given that other
> devm_*_match() don't do that and the only way to get NULL there would
> be bug in devm_rtc*() code itself rather than its users?

Ughh... I see that you probably copied the chunk from clk / regulator
implementation.  Let's please not copy those.  Input validation via
WARN_ON()s is nice when the interface is taking input from outside or
the code in question is complex / fragile.  Here, the code involved is
like 20 lines and self-contained.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()
  2013-02-26  1:30   ` 'Tejun Heo'
@ 2013-02-26  1:42     ` Jingoo Han
  0 siblings, 0 replies; 6+ messages in thread
From: Jingoo Han @ 2013-02-26  1:42 UTC (permalink / raw)
  To: 'Tejun Heo'
  Cc: 'Andrew Morton', linux-kernel, 'Greg KH',
	'Alessandro Zummo', rtc-linux, 'Jingoo Han'

On Tuesday, February 26, 2013 10:30 AM, Tejun Heo wrote:
> 
> On Mon, Feb 25, 2013 at 05:23:17PM -0800, 'Tejun Heo' wrote:
> > again, why do we need the WARN_ON() at all given that other
> > devm_*_match() don't do that and the only way to get NULL there would
> > be bug in devm_rtc*() code itself rather than its users?
> 
> Ughh... I see that you probably copied the chunk from clk / regulator
> implementation.  Let's please not copy those.  Input validation via
> WARN_ON()s is nice when the interface is taking input from outside or
> the code in question is complex / fragile.  Here, the code involved is
> like 20 lines and self-contained.

OK, I see.
Thank you for your comment.
I will send v3 patch soon :)

Best regards,
Jingoo Han

> 
> Thanks.
> 
> --
> tejun


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()
  2013-02-26  1:21 [PATCH V2] rtc: add devm_rtc_device_{register,unregister}() Jingoo Han
  2013-02-26  1:23 ` 'Tejun Heo'
@ 2013-02-26 22:33 ` Dmitry Torokhov
  2013-02-27  2:12   ` Jingoo Han
  1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Torokhov @ 2013-02-26 22:33 UTC (permalink / raw)
  To: Jingoo Han
  Cc: 'Andrew Morton', linux-kernel, 'Tejun Heo',
	'Greg KH', 'Alessandro Zummo',
	rtc-linux

On Tue, Feb 26, 2013 at 10:21:06AM +0900, Jingoo Han wrote:
> +/**
> + * 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)

Why do you need a separate function? You can add a flag to struct rtc_device
so it knows whether it is devm-managed or not and behave accordingly.
And then you can do

#define devm_rtc_device_unregister rtc_device_unregister

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] rtc: add devm_rtc_device_{register,unregister}()
  2013-02-26 22:33 ` Dmitry Torokhov
@ 2013-02-27  2:12   ` Jingoo Han
  0 siblings, 0 replies; 6+ messages in thread
From: Jingoo Han @ 2013-02-27  2:12 UTC (permalink / raw)
  To: 'Dmitry Torokhov'
  Cc: 'Andrew Morton', linux-kernel, 'Tejun Heo',
	'Greg KH', 'Alessandro Zummo',
	rtc-linux, 'Jingoo Han'

On Wednesday, February 27, 2013 7:33 AM, Dmitry Torokhov wrote:
> 
> On Tue, Feb 26, 2013 at 10:21:06AM +0900, Jingoo Han wrote:
> > +/**
> > + * 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)
> 
> Why do you need a separate function? You can add a flag to struct rtc_device
> so it knows whether it is devm-managed or not and behave accordingly.
> And then you can do
> 
> #define devm_rtc_device_unregister rtc_device_unregister

Um, I don't prefer that way using additional a flag.

Also, most of other devm_* functions are using a separate functions
such as devm_pwm_put(), devm_regulator_put(),devm_usb_put_phy(), etc.


> 
> Thanks.
> 
> --
> Dmitry


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-02-27  2:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-26  1:21 [PATCH V2] rtc: add devm_rtc_device_{register,unregister}() Jingoo Han
2013-02-26  1:23 ` 'Tejun Heo'
2013-02-26  1:30   ` 'Tejun Heo'
2013-02-26  1:42     ` Jingoo Han
2013-02-26 22:33 ` Dmitry Torokhov
2013-02-27  2:12   ` Jingoo Han

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.