On Mon, Jul 28, 2014 at 10:37:18PM +0530, Pramod Gurav wrote:

Thanks Dmitry for reviewing.

On Mon, Jul 28, 2014 at 10:19 PM, Dmitry Torokhov
[off-list ref] wrote:

quoted

Hi Pramod,

On Mon, Jul 28, 2014 at 02:16:55PM +0530, pramod.gurav.etc@gmail.com wrote:

quoted

From: Pramod Gurav <redacted>

This switches memory allocations from kzalloc to devm_kzalloc.
This also changes the way return checks were done on failure cases
of three allocations. The checks were clubbed together and hence
must be done seperately to avoid calling kfree on unallocated memory.

Moreover in case of input_allocate_device failure, we were calling
input_free_device which is not needed.

But not hurting either - like kfree() and many other "cleanup" functions
in kernel they are happily accept NULL input (so that cleanup code is
less branchy).

quoted

Great. Good to learn.

quoted

quoted

input device must be released(input_free_device) when ads7846_probe_dt
fails hence adds a fix there as well.

That is the real bug, could you send me a patch that fixes just that (by
jumping to err_free_mem)?

Sure I will.

quoted

The rest I'd like to leave as is unless you can convert _all_ resources
to be managed ones: mixing up 2 styles (manual and automatic release) is
bound to have issues (like we had with ads7846_probe_dt which assumed
that since it was releasing its resources automatically the rest would
be released automatically as well.

I thought I would use a managed input_device allocation in a separate patch.
I will convert all allocations to managed then.

This driver also registers a irq. You suggest to convert it to managed as well?
I was planning to do that?

Yes. And regulators, gpios, sysfs (you might need to have custom action
for that) and chip cleanup. OTOH I do not see much value in these
conversions unless they make the code much more clean.

Thanks,

-- 
Dmitry