Re: [PATCH] platform: Fix platform device resource linking

[Pantelis Antoniou]

On Jan 17, 2013, at 6:31 PM, Greg Kroah-Hartman wrote:

> On Fri, Jan 04, 2013 at 12:43:46AM +0200, Pantelis Antoniou wrote:
>> Hi Greg,
>> 
>> On Jan 4, 2013, at 12:40 AM, Greg Kroah-Hartman wrote:
>> 
>>> On Fri, Jan 04, 2013 at 12:31:10AM +0200, Pantelis Antoniou wrote:
>>>> Platform device removal uncovered a number of problems with
>>>> the way resources are handled in the core platform code.
>>>> 
>>>> Resources now form child/parent linkages and this requires
>>>> proper linking of the resources. On top of that the OF core
>>>> directly creates it's own platform devices. Simplify things
>>>> by providing helper functions that manage the linking properly.
>>>> 
>>>> Two functions are provided:
>>>> 
>>>> platform_device_link_resources(), which links all the
>>>> linkable resources (if not already linked).
>>>> 
>>>> and platform_device_unlink_resources(), which unlinks all the
>>>> resources.
>>> 
>>> Who would call these functions, and why?
>>> 
>>> And why have we never seen problems with removing platform devices
>>> previously?
>>> 
>> 
>> Have you tried removing devices that were created via DT and 
>> not using platform data?
> 
> Don't you think that answering two questions with another question as
> something that isn't very helpful?  :)
> 
> Dropped from my queue, please resend when you can provide the needed
> information.
> 
> thanks,
> 

That's not very nice, but anyway...

In a nutshell, we have to exercise the platform device subsystem, in ways
that never happened before, so all sorts of weird bugs that no-one has seen
before.

In that case, the code path for creating platform devices from DT is
not the same as the one that is used when creating platform device from
a board file.

Take a look at platform_device_add() in drivers/base/platform.c and
drivers/of/device.c

platform_device_add() properly links the resources by using insert_resource(),
while of_device_add() doesn't bother with it. Now when we try to unregister
the device everything will is fine in the platform device case, since the resources
are linked properly. In the DT case we will crash spectacularly in 
__release_resource at the first line (p = &old->parent->child), since no-one bothered
to fill in the parent pointer.

That's what the patches do; first the code in platform_device_add() that perform the
resource linking is factored as a separate function (platform_device_link_resources).

The platform_device_unlink_resources() function, just makes things more clearer. 

> greg k-h

Regards

-- Pantelis

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/