Hi Greg,
On Aug 6, 2013, at 12:33 PM, Greg Kroah-Hartman wrote:
> On Tue, Aug 06, 2013 at 10:53:44AM +0300, Pantelis Antoniou wrote:
>> Removing any omap device always resulted in a crash; turns out
>> BUS_NOTIFY_DEL_DEVICE is not the last notifier event sent in the
>> course of removing the device, the correct event is
>> BUS_NOTIFY_UNBOUND_DRIVER, which still is not the right place to
>> perform the cleanup. A device callback handles that properly, as
>> well as making sure the hwmods of the device are shutdown.
>>
>> Signed-off-by: Pantelis Antoniou <panto@xxxxxxxxxxxxxxxxxxxxxxx>
>> ---
>> arch/arm/mach-omap2/omap_device.c | 34 ++++++++++++++++++++++++++++++++--
>> 1 file changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c
>> index f33b40c..6dec521 100644
>> --- a/arch/arm/mach-omap2/omap_device.c
>> +++ b/arch/arm/mach-omap2/omap_device.c
>> @@ -178,6 +178,32 @@ odbfd_exit:
>> return ret;
>> }
>>
>> +static void _omap_device_cleanup(struct device *dev)
>> +{
>> + struct platform_device *pdev = to_platform_device(dev);
>> + struct omap_device *od;
>> + struct omap_hwmod *oh;
>> + int i;
>> +
>> + od = pdev->archdata.od;
>> + if (!od)
>> + return;
>> +
>> + for (i = 0; i < od->hwmods_cnt; i++) {
>> +
>> + oh = od->hwmods[i];
>> +
>> + /* shutdown hwmods */
>> + omap_hwmod_shutdown(oh);
>> +
>> + /* we don't remove clocks cause there's no API to do so */
>> + /* no harm done, since they will not be created next time */
>> + }
>> +
>> + /* cleanup the structure now */
>> + omap_device_delete(od);
>> +}
>> +
>> static int _omap_device_notifier_call(struct notifier_block *nb,
>> unsigned long event, void *dev)
>> {
>> @@ -185,9 +211,13 @@ static int _omap_device_notifier_call(struct notifier_block *nb,
>> struct omap_device *od;
>>
>> switch (event) {
>> - case BUS_NOTIFY_DEL_DEVICE:
>> + case BUS_NOTIFY_UNBOUND_DRIVER:
>> + /* NOTIFY_DEL_DEVICE is not the right call...
>> + * we use a callback here, to make sure no-one is going to
>> + * try to use the omap_device data after they're deleted
>> + */
>> if (pdev->archdata.od)
>> - omap_device_delete(pdev->archdata.od);
>> + device_schedule_callback(dev, _omap_device_cleanup);
>
> Really? This is one sign that you are totally using the driver core
> incorrectly. You shouldn't have to rely on notifier callbacks to handle
> device removals, your bus code should do that for you directly.
>
> I don't like this at all, sorry.
>
Don't shoot the messenger please...
This is all about fixing a crash without messing too many things.
> And I was waiting for the day when people started to finally remove
> platform devices from the system, I always thought it would never work
> properly. Good luck with this, I think you have a lot of work ahead of
> yourself...
>
I know.
Platform device removal is the wild-wild west...
If I had to make a wish, would be to kill platform_device completely and
integrate all it's functionality in the the vanilla device.
What exactly is a platform device anyway?
> greg k-h
Regards
-- Pantelis
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
