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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <56B1FCF3.2040307@nvidia.com>
Date:	Wed, 3 Feb 2016 18:43:23 +0530
From:	Laxman Dewangan <ldewangan@...dia.com>
To:	Krzysztof Kozlowski <k.kozlowski@...sung.com>,
	<lee.jones@...aro.org>, <alexandre.belloni@...e-electrons.com>,
	<javier@....samsung.com>
CC:	<cw00.choi@...sung.com>, <linux-kernel@...r.kernel.org>,
	<rtc-linux@...glegroups.com>
Subject: Re: [PATCH V2 5/5] rtc: max77686: move initialisation of rtc regmap,
 irq chip locally


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.