Re: [PATCH 3/5] omap: Properly handle resources for omap_devices

Pantelis Antoniou
2013-08-09T15:32:50Z

Hi
On Aug 9, 2013, at 6:16 PM, Kevin Hilman wrote:

> Pantelis Antoniou <panto@antoniou-consulting.com> writes:
>
>> Hi Kevin,
>>
>> On Aug 7, 2013, at 9:45 PM, Kevin Hilman wrote:
>>
>>> [fixing address for Benoit]
>>>
>>> Pantelis Antoniou <panto@antoniou-consulting.com> writes:
>>>
>>>> omap_device relies on the platform notifier callbacks managing resources
>>>> behind the scenes. The resources were not properly linked causing crashes
>>>> when removing the device.
>>>>
>>>> Rework the resource modification code so that linking is performed properly,
>>>> and make sure that no resources that have no parent (which can happen for DMA
>>>> & IRQ resources) are ever left for cleanup by the core resource layer.
>>>>
>>>> Signed-off-by: Pantelis Antoniou <panto@antoniou-consulting.com>
>>>
>>> This one failed my "took more than 15 minutes to understand" test. The
>>> changelog is rather vague (especially about what "properly" means), and
>>> the combination of moving code and changing it makes the patch rather
>>> clunky to read, so I remain a bit confused about what the actual problem
>>> is. Please elaborate.
>>>
>>> Also, could you share a crash dump as well as details about how to
>>> reproduce this problem?
>>>
>>> Thanks,
>>>
>>> Kevin
>>
>> It's the full patchset that fixes the problem:
>>
>> Let me illustrate:
>>
>> The kernel I use is located at:
>>
>> git@github.com:pantoniou/linux-beagle-track-mainline.git
>> branch: merge-20130806 (there are topic branches for other stuff too)
>
> Sorry, I don't have the time to go through a bunch of out of tree
> branches to figure this out. Can you create a simpler test case to
> reproduce this? e.g. Does this happen when building the serial driver
> as a module and then removing it? If not, why not?

What 'bunch of out of tree branches'? There is a single merge branch against
current mainline. And no you don't trigger this by removing a module.

platform_driver_unregister() != platform_device_register().

I'll try to figure out something simpler, but it's pretty damn obvious that
something is not right there. Do you think fixing problems in that part
of platform device removal was something I did because I didn't have other
things to do?

It is broken and I'm trying to get it fixed. Are you?

>
> Kevin

-- Pantelis