Hi Hans,
I've been thinking about the USB unbinding code. I know that I agreed to go with it, but in retrospect I think that was a mistake.
I believe we should separate out the unbinding and make it an option, so that it is not required in order to use USB. In effect this makes one of driver model's design goals (the option to drop unused code) useless since USB is a common interface.
If I recall the only problem the lack of unbinding caused was that the keyboard driver broke. I suspect it broke in a way that can be fixed. In fact I recently converted usb_ether to driver model and I'm willing to do the keyboard side also.
I'd like the USB code to function with or without the unbinding (i.e. it uses it if there). What do you think?
Regards, Simon
Hi,
On 20-07-15 04:23, Simon Glass wrote:
Hi Hans,
I've been thinking about the USB unbinding code. I know that I agreed to go with it, but in retrospect I think that was a mistake.
I believe we should separate out the unbinding and make it an option, so that it is not required in order to use USB. In effect this makes one of driver model's design goals (the option to drop unused code) useless since USB is a common interface.
If I recall the only problem the lack of unbinding caused was that the keyboard driver broke. I suspect it broke in a way that can be fixed. In fact I recently converted usb_ether to driver model and I'm willing to do the keyboard side also.
I'd like the USB code to function with or without the unbinding (i.e. it uses it if there). What do you think?
I strongly believe that unbinding is the proper thing todo for usb since it is a hotplug bus.
IMHO the way the usb_find_emul_child() function was used before to re-use udevice-s after e.g. a "usb reset" was an ugly hack which just happened to work, but it in no way reflects reality.
More importantly we need unbind support to properly stop usb controllers when booting the OS, so that they are not DMA-ing to/from their scratch-ram area in DRAM when the main OS boots, so not having unbind support combined with USB really is a no no.
This is why I suggested to simply select the unbind Kconfig when USB is selected in Kconfig.
The actual unbind core code is not that big, so I believe that the best solution is to always build the core if either DM_DEVICE_REMOVE *or* DM_USB is selected, and non USB drivers can leave out their unbind code if DM_DEVICE_REMOVE is not set, that should still give us most of the size savings without needing to do ugly hacks for USB.
Regards,
Hans
Hi Hans,
On 20 July 2015 at 09:31, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 20-07-15 04:23, Simon Glass wrote:
Hi Hans,
I've been thinking about the USB unbinding code. I know that I agreed to go with it, but in retrospect I think that was a mistake.
I believe we should separate out the unbinding and make it an option, so that it is not required in order to use USB. In effect this makes one of driver model's design goals (the option to drop unused code) useless since USB is a common interface.
If I recall the only problem the lack of unbinding caused was that the keyboard driver broke. I suspect it broke in a way that can be fixed. In fact I recently converted usb_ether to driver model and I'm willing to do the keyboard side also.
I'd like the USB code to function with or without the unbinding (i.e. it uses it if there). What do you think?
I strongly believe that unbinding is the proper thing todo for usb since it is a hotplug bus.
IMHO the way the usb_find_emul_child() function was used before to re-use udevice-s after e.g. a "usb reset" was an ugly hack which just happened to work, but it in no way reflects reality.
More importantly we need unbind support to properly stop usb controllers when booting the OS, so that they are not DMA-ing to/from their scratch-ram area in DRAM when the main OS boots, so not having unbind support combined with USB really is a no no.
This is why I suggested to simply select the unbind Kconfig when USB is selected in Kconfig.
I think you are referring to remove(), not unbind(). Although we might consider spiting them so we have a DM_DEVICE_REMOVE and a separate DM_DEVICE_UNBIND.
The actual unbind core code is not that big, so I believe that the best solution is to always build the core if either DM_DEVICE_REMOVE *or* DM_USB is selected, and non USB drivers can leave out their unbind code if DM_DEVICE_REMOVE is not set, that should still give us most of the size savings without needing to do ugly hacks for USB.
My main objection is that we tie USB such that it *will not work* unless we support unbinding. I'm fine with it being recommended, but core driver model features should be independent of subsystems. This also seems quite unnecessary. Re your common about the 'ugly hack that just happened to work', in principle we can just keep on creating new devices and ignore the old ones. That's the idea behind not supporting unbinding. There should be no problem with this approach.
So I'd like to adjust the USB code so that it still works without unbinding, even if it is not optimal. I think that is the right thing to do in this case.
Regards, Simon
Hi,
On 07/20/2015 05:49 PM, Simon Glass wrote:
Hi Hans,
On 20 July 2015 at 09:31, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 20-07-15 04:23, Simon Glass wrote:
Hi Hans,
I've been thinking about the USB unbinding code. I know that I agreed to go with it, but in retrospect I think that was a mistake.
I believe we should separate out the unbinding and make it an option, so that it is not required in order to use USB. In effect this makes one of driver model's design goals (the option to drop unused code) useless since USB is a common interface.
If I recall the only problem the lack of unbinding caused was that the keyboard driver broke. I suspect it broke in a way that can be fixed. In fact I recently converted usb_ether to driver model and I'm willing to do the keyboard side also.
I'd like the USB code to function with or without the unbinding (i.e. it uses it if there). What do you think?
I strongly believe that unbinding is the proper thing todo for usb since it is a hotplug bus.
IMHO the way the usb_find_emul_child() function was used before to re-use udevice-s after e.g. a "usb reset" was an ugly hack which just happened to work, but it in no way reflects reality.
More importantly we need unbind support to properly stop usb controllers when booting the OS, so that they are not DMA-ing to/from their scratch-ram area in DRAM when the main OS boots, so not having unbind support combined with USB really is a no no.
This is why I suggested to simply select the unbind Kconfig when USB is selected in Kconfig.
I think you are referring to remove(), not unbind().
Right I mean that the remove callback *must* be called on usb_stop to avoid the usb controller dma-ing over random DRAM when the OS starts.
Although we might consider spiting them so we have a DM_DEVICE_REMOVE and a separate DM_DEVICE_UNBIND.
The actual unbind core code is not that big, so I believe that the best solution is to always build the core if either DM_DEVICE_REMOVE *or* DM_USB is selected, and non USB drivers can leave out their unbind code if DM_DEVICE_REMOVE is not set, that should still give us most of the size savings without needing to do ugly hacks for USB.
My main objection is that we tie USB such that it *will not work* unless we support unbinding. I'm fine with it being recommended, but core driver model features should be independent of subsystems. This also seems quite unnecessary. Re your common about the 'ugly hack that just happened to work', in principle we can just keep on creating new devices and ignore the old ones.
That will still cause problems with code addressing the usb-devices by index, as the old devices will still be there.
That's the idea behind not supporting unbinding. There should be no problem with this approach.
This approach will only work if find_child_devnum is fixed to search backwards through the childs list, so that it will check the newly added nodes first.
So I'd like to adjust the USB code so that it still works without unbinding, even if it is not optimal. I think that is the right thing to do in this case.
As said, the remove callback of usb-host drivers *must* always be called, other then that if you can make things work without unbind that is fine with me.
Regards,
Hans
Hi Hans,
On 21 July 2015 at 13:52, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 07/20/2015 05:49 PM, Simon Glass wrote:
Hi Hans,
On 20 July 2015 at 09:31, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 20-07-15 04:23, Simon Glass wrote:
Hi Hans,
I've been thinking about the USB unbinding code. I know that I agreed to go with it, but in retrospect I think that was a mistake.
I believe we should separate out the unbinding and make it an option, so that it is not required in order to use USB. In effect this makes one of driver model's design goals (the option to drop unused code) useless since USB is a common interface.
If I recall the only problem the lack of unbinding caused was that the keyboard driver broke. I suspect it broke in a way that can be fixed. In fact I recently converted usb_ether to driver model and I'm willing to do the keyboard side also.
I'd like the USB code to function with or without the unbinding (i.e. it uses it if there). What do you think?
I strongly believe that unbinding is the proper thing todo for usb since it is a hotplug bus.
IMHO the way the usb_find_emul_child() function was used before to re-use udevice-s after e.g. a "usb reset" was an ugly hack which just happened to work, but it in no way reflects reality.
More importantly we need unbind support to properly stop usb controllers when booting the OS, so that they are not DMA-ing to/from their scratch-ram area in DRAM when the main OS boots, so not having unbind support combined with USB really is a no no.
This is why I suggested to simply select the unbind Kconfig when USB is selected in Kconfig.
I think you are referring to remove(), not unbind().
Right I mean that the remove callback *must* be called on usb_stop to avoid the usb controller dma-ing over random DRAM when the OS starts.
OK.
Although we might consider spiting them so we have a DM_DEVICE_REMOVE and a separate DM_DEVICE_UNBIND.
The actual unbind core code is not that big, so I believe that the best solution is to always build the core if either DM_DEVICE_REMOVE *or* DM_USB is selected, and non USB drivers can leave out their unbind code if DM_DEVICE_REMOVE is not set, that should still give us most of the size savings without needing to do ugly hacks for USB.
My main objection is that we tie USB such that it *will not work* unless we support unbinding. I'm fine with it being recommended, but core driver model features should be independent of subsystems. This also seems quite unnecessary. Re your common about the 'ugly hack that just happened to work', in principle we can just keep on creating new devices and ignore the old ones.
That will still cause problems with code addressing the usb-devices by index, as the old devices will still be there.
That's the idea behind not supporting unbinding. There should be no problem with this approach.
This approach will only work if find_child_devnum is fixed to search backwards through the childs list, so that it will check the newly added nodes first.
Or that it just ignores the nodes that aren't active. Anyway that function is a hang-over from the old code. It makes no sense to enumerate the devices when you can just look up the data and find them. I think it can be made to work for now, but perhaps we should port the keyboard drivers to DM?
So I'd like to adjust the USB code so that it still works without unbinding, even if it is not optimal. I think that is the right thing to do in this case.
As said, the remove callback of usb-host drivers *must* always be called, other then that if you can make things work without unbind that is fine with me.
OK thanks, will give it a crack.
Regards, Simon
Hi,
On 22-07-15 05:48, Simon Glass wrote:
Hi Hans,
On 21 July 2015 at 13:52, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 07/20/2015 05:49 PM, Simon Glass wrote:
Hi Hans,
On 20 July 2015 at 09:31, Hans de Goede hdegoede@redhat.com wrote:
Hi,
On 20-07-15 04:23, Simon Glass wrote:
Hi Hans,
I've been thinking about the USB unbinding code. I know that I agreed to go with it, but in retrospect I think that was a mistake.
I believe we should separate out the unbinding and make it an option, so that it is not required in order to use USB. In effect this makes one of driver model's design goals (the option to drop unused code) useless since USB is a common interface.
If I recall the only problem the lack of unbinding caused was that the keyboard driver broke. I suspect it broke in a way that can be fixed. In fact I recently converted usb_ether to driver model and I'm willing to do the keyboard side also.
I'd like the USB code to function with or without the unbinding (i.e. it uses it if there). What do you think?
I strongly believe that unbinding is the proper thing todo for usb since it is a hotplug bus.
IMHO the way the usb_find_emul_child() function was used before to re-use udevice-s after e.g. a "usb reset" was an ugly hack which just happened to work, but it in no way reflects reality.
More importantly we need unbind support to properly stop usb controllers when booting the OS, so that they are not DMA-ing to/from their scratch-ram area in DRAM when the main OS boots, so not having unbind support combined with USB really is a no no.
This is why I suggested to simply select the unbind Kconfig when USB is selected in Kconfig.
I think you are referring to remove(), not unbind().
Right I mean that the remove callback *must* be called on usb_stop to avoid the usb controller dma-ing over random DRAM when the OS starts.
OK.
Although we might consider spiting them so we have a DM_DEVICE_REMOVE and a separate DM_DEVICE_UNBIND.
The actual unbind core code is not that big, so I believe that the best solution is to always build the core if either DM_DEVICE_REMOVE *or* DM_USB is selected, and non USB drivers can leave out their unbind code if DM_DEVICE_REMOVE is not set, that should still give us most of the size savings without needing to do ugly hacks for USB.
My main objection is that we tie USB such that it *will not work* unless we support unbinding. I'm fine with it being recommended, but core driver model features should be independent of subsystems. This also seems quite unnecessary. Re your common about the 'ugly hack that just happened to work', in principle we can just keep on creating new devices and ignore the old ones.
That will still cause problems with code addressing the usb-devices by index, as the old devices will still be there.
That's the idea behind not supporting unbinding. There should be no problem with this approach.
This approach will only work if find_child_devnum is fixed to search backwards through the childs list, so that it will check the newly added nodes first.
Or that it just ignores the nodes that aren't active. Anyway that function is a hang-over from the old code. It makes no sense to enumerate the devices when you can just look up the data and find them.
Right, walking over the tree is still necessary for the "usb tree" command though.
I think it can be made to work for now, but perhaps we should port the keyboard drivers to DM?
I agree that atleast the usb-keyb. driver should be ported to use DM style binding like the usb-storage driver.
I've been thinking a bit about this, currently the driver will only bind to the first available usb hid intf with a boot - keyb subclass. If we move to DM binding we should support multiple keyboards, but the current stdio.c code deals poorly with this, so I think that it would be best to keep a list of usb-keyb-devices in common/usb_kbd.c and register a stdio device for this when the first usb-keyb-device gets registered (so the list is empty) and unregister it when the last one gets removed.
All usb-keyboards would then feed keypresses into a single FIFO (usb_kbd_buffer + usb_[in|out]_pointer in the current code), from which the single stdio device would feed.
Note that the unregistering bit requires DM_DEVICE_REMOVE, we can do without this though and just keep the stdio-device around with an always empty keyevent fifo.
So I'd like to adjust the USB code so that it still works without unbinding, even if it is not optimal. I think that is the right thing to do in this case.
As said, the remove callback of usb-host drivers *must* always be called, other then that if you can make things work without unbind that is fine with me.
OK thanks, will give it a crack.
Regards,
Hans
participants (2)
-
Hans de Goede -
Simon Glass