Hi,
On 06/30/2015 06:07 PM, Simon Glass wrote:
<snip>
Instead, I wonder if we can remove the children when we probe the bus?
That should work, but I do not really see any advantage in that,
removing the children is not that expensive and it feels like a
kludge.
That's how it currently works, from what I can see in the code. But
since there is a 'usb_started' boolean this is irrelevant.
Also, what happens to children that are in the device tree - i.e.
static USB devices like WiFi? The device tree might have parameters
for them. Still, that might not matter as I'm not sure that case is
handled correctly today.
AFAIK there is no such thing as usb devices in devicetree, which
makes sense as usb is a fully discoverable bus.
Sort-of. But as with PCI it is useful to be able to add settings for
the devices in some cases. You can match them using vendor/device or
interface IDs. Then the driver can access its settings.
AFAIK there is not a single example of having settings in devicetree
for an usb device, since usb-devices are always 100% self describing
since usb is a bus designed for hot(un)plug from the outset.
That's why I'm suggesting we unbind the devices that are no-longer present.
You're asking to make the code more complicated here using a what if
reasoning with a "what if" which is likely to never happen.
The result of this commit is best seen in the output of "dm tree" after
plugging out an usb hub with 2 devices plugges in and plugging in a
keyb.
instead, before this commit the output would be:
usb [ + ] `-- sunxi-musb
usb_hub [ ] |-- usb_hub
usb_mass_st [ ] | |-- usb_mass_storage
usb_dev_gen [ ] | `-- generic_bus_0_dev_3
usb_dev_gen [ + ] `-- generic_bus_0_dev_1
Notice the non active usb_hub child and its 2 non active children. The
first child being non-active as in this example also causes
usb_get_dev_index
to return NULL when probing the first child, which results in the usb
kbd
code not binding to the keyboard.
Although I suspect that could be fixed.
Right, but just removing the children is a much cleaner solution, and
also
makes the output of "dm tree" properly reflect reality.
True, although you also have 'usb tree' for that. Another option would
be to mark devices that were found and remove the others after the
scan.
That seems like needless complexity. I believe that simply removing +
unbinding
the children on usb_stop is the right thing to do, and it also is the KISS
solution.
I'm good with the remove(), but less sure about the unbind().
The unbind is necessary for usb_get_dev_index() to work properly,
which is necessary for proper output of "usb tree" and for driver
binding to work properly, without the unbind usb-keyboards will e.g.
not work in certain circumstances.
The
state of 'dm tree' does not bother me,
The state if dm tree is what usb_get_dev_index() works from, so if
it is not in a good state, then usb_get_dev_index() will not work.
and I worry that we then limit
our ability to use usb_find_child() to locate a device's parameters
(i.e. support for more complex devices which need settings might be
harder).
Again this is a what if reasoning for a hypothetical future problem
which will likely never happen, where as the broken state of the
dm tree after a "usb reset" is causing real problems.
For now, can we just leave this alone? I really don't want to re-visit
this later.
Nope we cannot leave this alone, without unbinding usb devices which
no longer exist, the dm tree will be broken and with it
usb_get_dev_index() and through usb_get_dev_index() the keyboard
driver.
Regards,
Hans
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
