[U-Boot] USB and unbinding

[Simon Glass]

Hi Hans,

On 21 July 2015 at 13:52, Hans de Goede <hdegoede at 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 at 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