Add device addition/removal notifier
in [Kernel]
|
From: Randy Dunlap on 19 Oct 2006 12:00 On Thu, 19 Oct 2006 17:56:31 +1000 Benjamin Herrenschmidt wrote: > Index: linux-cell/include/linux/device.h > =================================================================== > --- linux-cell.orig/include/linux/device.h 2006-10-19 17:43:58.000000000 +1000 > +++ linux-cell/include/linux/device.h 2006-10-19 17:44:24.000000000 +1000 > @@ -427,6 +427,22 @@ extern int (*platform_notify)(struct dev > > extern int (*platform_notify_remove)(struct device * dev); > > +/** > + * Device notifiers. Get notified of addition/removal of devices > + * and possibly other events in the future. Replacement for the > + * platform "fixup" functions. This is a low level hook provided > + * for the platform to initialize private parts of struct device, > + * like firmware related links. Add is called before the device is > + * added to a bus (and thus the driver probed) and Remove is called > + * afterward. > + */ That's not kernel-doc, so please don't begin it with "/**". --- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 19 Oct 2006 22:00 > That's not kernel-doc, so please don't begin it with "/**". Oops, sorry, new patch on the way Ben. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 19 Oct 2006 23:30 On Fri, Oct 20, 2006 at 11:55:50AM +1000, Benjamin Herrenschmidt wrote: > @@ -608,12 +615,14 @@ void device_del(struct device * dev) > device_remove_groups(dev); > device_remove_attrs(dev); > > + bus_remove_device(dev); > /* Notify the platform of the removal, in case they > * need to do anything... > */ > if (platform_notify_remove) > platform_notify_remove(dev); > - bus_remove_device(dev); > + blocking_notifier_call_chain(&device_notifier, DEVICE_NOTIFY_DEL_DEV, > + dev); Why did you move the call to bus_remove_device() to be before the platform_notify_remove() and notifier is called? And I don't think this is really going to work well. You have created a notifier for all devices in the system, right? How do you know what type of struct device is being passed to your notifier callback? At least with the platform callback, you knew it was a platform device :) > device_pm_remove(dev); > kobject_uevent(&dev->kobj, KOBJ_REMOVE); > kobject_del(&dev->kobj); > @@ -836,3 +845,15 @@ int device_rename(struct device *dev, ch > > return error; > } > + > +int register_device_notifier(struct notifier_block *nb) > +{ > + return blocking_notifier_chain_register(&device_notifier, nb); > +} > +EXPORT_SYMBOL(register_device_notifier); > + > +int unregister_device_notifier(struct notifier_block *nb) > +{ > + return blocking_notifier_chain_unregister(&device_notifier, nb); > +} > +EXPORT_SYMBOL(unregister_device_notifier); All driver core exports should be _GPL() please. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 20 Oct 2006 00:40 On Thu, 2006-10-19 at 20:26 -0700, Greg KH wrote: > On Fri, Oct 20, 2006 at 11:55:50AM +1000, Benjamin Herrenschmidt wrote: > > @@ -608,12 +615,14 @@ void device_del(struct device * dev) > > device_remove_groups(dev); > > device_remove_attrs(dev); > > > > + bus_remove_device(dev); > > /* Notify the platform of the removal, in case they > > * need to do anything... > > */ > > if (platform_notify_remove) > > platform_notify_remove(dev); > > - bus_remove_device(dev); > > + blocking_notifier_call_chain(&device_notifier, DEVICE_NOTIFY_DEL_DEV, > > + dev); > > Why did you move the call to bus_remove_device() to be before the > platform_notify_remove() and notifier is called? I sent a mail about that a couple of days ago. The current behaviour is bogus imho. (mail subjet was [PATCH/RFC] Call platform_notify_remove later, and Len Brown who uses that hook in ACPI agreed). Basically, we notify the platform of insertion before we bind devices to busses & drivers (so the platform can do fixups, allocate auxilliary data structures, whatever else...) and thus we should notify the platform of removal -after- we unbind from the bus and driver where it will then destroy those data structures). In my case, I need to hookup the DMA ops pointers. It would be very bogus to destroy those before the driver remove() has been called. > And I don't think this is really going to work well. You have created a > notifier for all devices in the system, right? How do you know what > type of struct device is being passed to your notifier callback? At > least with the platform callback, you knew it was a platform device :) No I didn't. The platform_notify callback was called for any device as is my notifier :) All I did was to add a notifier chain in addition to the old function pointer with the intend of deprecating the function pointer :) You can know what type of device you are called for by comparing the device->bus to the address of the bus type data structure. That's the only way to do so but it works pretty fine. (There are actually some severe issues with the device model and with PCI around that area that I'm hitting trying to clean up some of the powerpc mess but let's handle one problem at a time). On PowerPC, for example, I'll use that notifier in at least 2 different places: For PCI, I need to use the "delete" callback to destroy the auxilliary data structure containing the DMA ops (it's currently created by some PCI fixup code, but I might also migrate that to the "add" callback. The other place is for platforms like Cell that are now growing DMA capable non-PCI devices, that I'm catching in the cell specific iommu code via that notifier, to hook them up to the right DMA ops. Ben. > > > > device_pm_remove(dev); > > kobject_uevent(&dev->kobj, KOBJ_REMOVE); > > kobject_del(&dev->kobj); > > @@ -836,3 +845,15 @@ int device_rename(struct device *dev, ch > > > > return error; > > } > > + > > +int register_device_notifier(struct notifier_block *nb) > > +{ > > + return blocking_notifier_chain_register(&device_notifier, nb); > > +} > > +EXPORT_SYMBOL(register_device_notifier); > > + > > +int unregister_device_notifier(struct notifier_block *nb) > > +{ > > + return blocking_notifier_chain_unregister(&device_notifier, nb); > > +} > > +EXPORT_SYMBOL(unregister_device_notifier); > > All driver core exports should be _GPL() please. > > thanks, > > greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 20 Oct 2006 01:20
On Fri, Oct 20, 2006 at 02:29:24PM +1000, Benjamin Herrenschmidt wrote: > On Thu, 2006-10-19 at 20:26 -0700, Greg KH wrote: > > On Fri, Oct 20, 2006 at 11:55:50AM +1000, Benjamin Herrenschmidt wrote: > > > @@ -608,12 +615,14 @@ void device_del(struct device * dev) > > > device_remove_groups(dev); > > > device_remove_attrs(dev); > > > > > > + bus_remove_device(dev); > > > /* Notify the platform of the removal, in case they > > > * need to do anything... > > > */ > > > if (platform_notify_remove) > > > platform_notify_remove(dev); > > > - bus_remove_device(dev); > > > + blocking_notifier_call_chain(&device_notifier, DEVICE_NOTIFY_DEL_DEV, > > > + dev); > > > > Why did you move the call to bus_remove_device() to be before the > > platform_notify_remove() and notifier is called? > > I sent a mail about that a couple of days ago. The current behaviour is > bogus imho. (mail subjet was [PATCH/RFC] Call platform_notify_remove > later, and Len Brown who uses that hook in ACPI agreed). > > Basically, we notify the platform of insertion before we bind devices to > busses & drivers (so the platform can do fixups, allocate auxilliary > data structures, whatever else...) and thus we should notify the > platform of removal -after- we unbind from the bus and driver where it > will then destroy those data structures). > > In my case, I need to hookup the DMA ops pointers. It would be very > bogus to destroy those before the driver remove() has been called. Ok, as long as you all agree that this does change the behavior, it's fine with me :) > > And I don't think this is really going to work well. You have created a > > notifier for all devices in the system, right? How do you know what > > type of struct device is being passed to your notifier callback? At > > least with the platform callback, you knew it was a platform device :) > > No I didn't. The platform_notify callback was called for any device as > is my notifier :) > > All I did was to add a notifier chain in addition to the old function > pointer with the intend of deprecating the function pointer :) > > You can know what type of device you are called for by comparing the > device->bus to the address of the bus type data structure. That's the > only way to do so but it works pretty fine. (There are actually some > severe issues with the device model and with PCI around that area that > I'm hitting trying to clean up some of the powerpc mess but let's handle > one problem at a time). > > On PowerPC, for example, I'll use that notifier in at least 2 different > places: For PCI, I need to use the "delete" callback to destroy the > auxilliary data structure containing the DMA ops (it's currently created > by some PCI fixup code, but I might also migrate that to the "add" > callback. The other place is for platforms like Cell that are now > growing DMA capable non-PCI devices, that I'm catching in the cell > specific iommu code via that notifier, to hook them up to the right DMA > ops. Ok, then perhaps you just want a bus specific callback for the devices on that bus? That would be much simpler and keep you from having to do that mess with the different tests of bus type. Actually, that's the only thing that really makes sense here, now that I think about it, the platform_notify doesn't really make any sense... thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |