From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Wed, 22 Jul 2015 11:09:50 +0200 Subject: [U-Boot] USB and unbinding In-Reply-To: References: <55AD1457.3040703@redhat.com> <55AEA30C.7020104@redhat.com> Message-ID: <55AF5DDE.8010409@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 22-07-15 05:48, Simon Glass wrote: > Hi Hans, > > On 21 July 2015 at 13:52, Hans de Goede wrote: >> Hi, >> >> On 07/20/2015 05:49 PM, Simon Glass wrote: >>> >>> Hi Hans, >>> >>> >>> On 20 July 2015 at 09:31, Hans de Goede 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