On Mon, 25 Feb 2013 16:35:53 +0900
Jingoo Han <[email protected]> wrote:
> These functios allows the driver core to automatically clean up
> any allocation made by rtc drivers. Thus, it simplifies the error
> paths.
>
> ...
>
> @@ -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 (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
Use
if (WARN_ON(!r || !*r))
return 0;
However, if anyone ever reports this warning you will be sad because
you won't know which condition triggered it. So a more useful approach
is
if (WARN_ON(!r))
return 0;
if (WARN_ON(!*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
> + *
> + * Managed rtc_device_register(). The rtc_device returned from this function
> + * are automatically freed on driver detach. See rtc_device_register()
> + * for more information.
> + */
I think the name is inappropriate. devm functions start with "devm"
and rtc functions start with "rtc" and this code is a part of rtc core,
not a part of devm core. Hence it should be something like
"rtc_devm_device_register".
> +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);
So this function will return an ERR_PTR on error. Please add a
description of the return value into the kerneldoc.
> +/**
> + * 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);
> + if (rc != 0)
> + WARN_ON(rc);
WARN_ON(!rc);
> +}
> +EXPORT_SYMBOL_GPL(devm_rtc_device_unregister);
> +
>
> ...
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
