Re: [PATCH V2 5/5] rtc: max77686: move initialisation of rtc regmap, irq chip locally

[Laxman Dewangan]

On Wednesday 03 February 2016 04:52 PM, Krzysztof Kozlowski wrote:
> On 03.02.2016 19:45, Krzysztof Kozlowski wrote:
> > On 03.02.2016 18:30, Laxman Dewangan wrote:
> > >   
> > >   err_rtc:
> > > +	if (info->rtc)
> > > +		i2c_unregister_device(info->rtc);
> > > +	regmap_del_irq_chip(info->rtc_irq, info->rtc_irq_data);
> > > +
> > You should clean up in reverse order of allocation, so first
> > regmap_del_irq_chip then i2c_unregister_device. This
> > is a common pattern of cleaning up. Sometimes such order
> > is even necessary because of dependencies between
> > components... which is not a case here but still the
> > natural way is reversing the allocation code.

I made the change in other place (remove callback) but not this place. 
It is just missed by me. my bad..
Will do in next patch.

> > [   88.296055] [<c00eaa6c>] (do_filp_open) from [<c00dc29c>] (do_sys_open+0x114/0x1c0)
> > [   88.303699] [<c00dc29c>] (do_sys_open) from [<c000f8c0>] (ret_fast_syscall+0x0/0x3c)
> > [   88.311419] Code: e7905003 e3550000 0a00001f e5943014 (e7950003)
> > [   88.317573] ---[ end trace 9502799e3ea05a80 ]---
> However removal of "remove" callback helps... which could be expected...
> maybe it is not an error of the driver itself?


OK, looked it and found that we are registering the chip_irq as normal 
and interrupt as devm_*.
So when we remove, we delete the regmap_irq_chip first and then free 
irq. That is creating issue.

It seems I can not use the devm_requested_irq_thread() and need to use 
requested_irq_thread()  for proper sequence of freeing it.

Otherwise we need to add the devm_regmap_add_irq_chip() first and then 
use it.

I am spinning the series to use the requested_thread_irq() only to avoid 
unbind issue.