On Thu, 07 Apr 2016, Laxman Dewangan wrote:
> Hi Lee,
> Thanks for review.
> I will send another patch with incorporating your comments.
>
>
> On Thursday 07 April 2016 04:14 PM, Lee Jones wrote:
> >On Tue, 05 Apr 2016, Laxman Dewangan wrote:
> >
> >+ if (!ret) {
> >+ *ptr = dev;
> >+ devres_add(dev, ptr);
> >+ } else {
> >+ devres_free(ptr);
> >+ }
> >Switch these round. If you encounter a problem, free and return. If
> >not, skip the error handling and add the device outside of the if().
>
> Like below?
>
> if (ret) {
> devres_free(ptr);
> return ret;
> }
>
> *ptr = dev;
> devres_add(dev, ptr);
>
> return ret;
This is more in line with what I expect, yes.
> >>+ * Remove all mfd devices added on the device.
> >s/mfd/MFD/
> >
> >'D' already means devices, so here you are saying "devices devices".
> >Please re-word. Besides, you need to be more specific as to which
> >"devices on the devices" you are detailing, since this sentence
> >doesn't really make a great deal of sense.
> Wanted to say
> Remove all devices added by mfd_add_devices() from parent device.
Remove all chlid-devices?
> >>+ * Normally this function will not need to be called and the resource
> >>+ * management code will ensure that the resource is freed.
> >Then what is the purpose of providing it? Do you have a user?
>
> To have pair of release. I have not seen the usage of most of
> devm_*_release() function other than devm_kfree().
Unless you have a need or a user, I would omit this for now.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
