Re: [PATCH v2 4/4] ARM: DTS: AM43x: Add DSS node

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- Attachment #1: Type: text/plain, Size: 2209 bytes --]

On 14/03/14 18:04, Felipe Balbi wrote:
> On Fri, Mar 14, 2014 at 02:07:45PM +0000, Mark Rutland wrote:
>> On Fri, Mar 14, 2014 at 11:07:05AM +0000, Tomi Valkeinen wrote:
>>> On 14/03/14 12:19, Tomi Valkeinen wrote:
>>>> On 14/03/14 12:14, Mark Rutland wrote:
>>>>
>>>>> I can't see anything obviously wrong in platform_device_del. Do you have
>>>>> a backtrace?
>>>>
>>>> Yes, below.
>>>>
>>>> I can see at least drivers/usb/dwc3/dwc3-exynos.c doing the exact same thing
>>>> I do, so maybe I've got something wrong with the omapdss driver.
>>>
>>> Looks to me that the devices created by of_platform_populate() are not
>>> unregisterable in all cases. The address resource created via
>>> of_platform_populate() had NULL res->parent, which causes
>>> release_resource to crash.
>>
>> Hmm. I can't see that unregistering such devices ever works as you say,
>> given that __release_resource expects a non-NULL parent pointer. Either
>> we should be setting the parent pointer when initialising devices from
>> dt or we should teach __release_resource to not care. I'll have a go at
>> fixing that.
>>
>> It looks like drivers/usb/dwc3/dwc3-exynos.c only unregisters the
>> top-level device, not children. This top-level device has no
>> IORESOURCE_{IO,MEM} resources judging by
>> arch/arm/boot/dts/exynos5250.dtsi, which would explain why that driver
>> isn't exploding: __release_resource will never get called.
>>
>> Anton, Felipe: 
>>
>> Does unregistering the parent ensure the children get cleaned up, or
>> does it leave them dangling in the dwc3-exynos driver?
> 
> you should platform_device_unregister() for each children and
> dwc3-exynos does that just fine:

Yes, that's what I do also, and it crashes. What Mark said above about
unregistering never working for such devices is correct, but I don't
know why he said dwc3-exynos only unregisters the top level device.

"such devices" above meaning devices with a 'reg' defined in the DT
data, if I'm not mistaken.

So at the moment, I think of_platform_populate() and
platform_device_unregister() combination in a driver is a bit risky.
Work fine for certain cases, not for some other.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]