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

Laxman Dewangan
2016-02-03T13:24:45Z

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.