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

[Kevin Hilman <khilman@linaro.org>]

Pantelis Antoniou <panto@antoniou-consulting.com> writes:

> 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 That branch contains multiple other branches (as you stated yourself
above), many of which look to be out of tree:

$ git log --oneline --merges  panto/master..panto/merge-20130806
031297e Merge branch 'lcdc-fixes' into merge-20130806
44cf018 Merge branch 'usb-fixes' into merge-20130806
3cc739b Merge branch 'tscadc' into merge-20130806
f0ec35d Merge branch 'capemgr' into merge-20130806
4d87712 Merge branch 'general-fixes' into merge-20130806
04535fb Merge branch 'pinctrl-fixes' into merge-20130806
5115b55 Merge branch 'i2c-fixes' into merge-20130806
e96edf5 Merge branch 'capes' into merge-20130806
1eb1779 Merge branch 'dts-fixes' into merge-20130806
ca12149 Merge branch 'mmc-fixes' into merge-20130806
41ed7a0 Merge branch 'pdev-fixes' into merge-20130806
2ba9995 Merge branch 'of-fixes' into merge-20130806
38b5cb4 Merge branch 'dtc-overlays' into merge-20130806
167648d Merge branch 'dtc-fixes' into merge-20130806
2f7de02 Merge branch 'reset' into merge-20130806

$ git log --oneline --no-merges panto/master..panto/merge-20130806 |wc -l
85

> 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. 

I agree something does not seem right, and never said otherwise.  I'm
simply looking for an easy way to reproduce it with mainline.  

Since it's so "damn obvious", I'll accept the insult and agree to being
a moron.  Please educate me by making it even more obvious with a way to
reproduce it against mainline.

> 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?

Settle down, I said nothing of the sort.

On the contrary, I assumed you were being a generous citizen of the
community and contributing back your fixes, and as a developer of this
part of the kernel, I'm trying to help.  If I didn't care, I wouldn't
have responded in the first place.

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

I was, but your accusatory tone is reducing my motivation to help.

Kevin