Messages in this thread |  | | Date | Thu, 17 Jan 2013 09:07:19 -0800 | From | Greg Kroah-Hartman <> | Subject | Re: [PATCH] platform: Fix platform device resource linking |
| |
On Thu, Jan 17, 2013 at 06:50:19PM +0200, Pantelis Antoniou wrote: > > 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...
What would you have me do if you were in my shoes?
> 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.
Why do you have to do this? What are you doing that is so different from everyone else? What drivers are you using that trigger this type of thing?
> 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.
Why not?
> 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.
So, isn't that a bug in the DT case? A device always has to have a parent, as you have found out. Hm, maybe not "root" devices, but you don't have many of those, right?
> 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.
But you added a new function that no one calls, which is what I am objecting to.
thanks,
greg k-h
|  |