Hi,
On 19-10-15 01:17, Simon Glass wrote:
Hi Hans,
On 12 September 2015 at 09:15, Hans de Goede <hdego...@redhat.com> wrote:
Hi,
On 08-09-15 19:15, Simon Glass wrote:
Switch USB keyboards over to use driver model instead of scanning with the
horrible usb_get_dev_index() function. This involves creating a new uclass
for keyboards, although so far there is no API.
Signed-off-by: Simon Glass <s...@chromium.org>
In general I like this patch, so ack for the principle, but the
implementation has issues.
You're now allowing the registration of multiple usb keyb stdio
input devices, but you are still relying on usb_kbd_deregister()
to remove them from the stdio devices list, and when multiple
or used that one will only remove the first one.
This can be fixed by switching to stdio_register_dev, and
store the returned struct stdio_dev pointer for the new dev,
and add a dm remove callback which deregisters that specific
stdio_dev that will also remove the ugliness of looking up
the device by its name to unregister it.
The name which in itself is another, harder to fix issue,
when using iomux, and the stdin string contains usbkbd we
really want to get all usbkbd-s to work, but iomux will
only take the first one.
This can be fixed in 2 ways:
1) in the usbkbd driver by registering a shared stdio_dev
for all usbkbd's and deregistering that only when the
last usbkbd is removed (in the case of dm), this will
require a global list of usbkbd devices, and stdio
callbacks to walk over this list, I believe that this
is likely the best approach
2) Fix this in iomux, and make it look for multiple
devs with the same name and mux them all.
This seems cleaner at a conceptual level, but likely
somewhat hard to implement.
I've had another look at this and here are my comments so far:
1. The existing driver does not support multiple keyboards. It
implements this limitation in multiple ways which would be a real pain
to fix while keeping the old code. I think it makes much more sense to
remove this limitation when we have either a) moved all uses of USB
keyboard to driver model, or perhaps b) moved stdio to driver model.
For now the driver model approach provides the same functionality as
before so I think it is fine.
I think that supporting multiple keyboards the way I've outlined
above as "1)" should not be that hard. But I do not plan to make time
for this anytime soon, and as such I can hardly ask you to do this.
So I reluctantly agree to keep this as is (I was hoping the move
to dm would fix this).
2. The point about out-of-order devices in the 'usb tree'
display....well if you disable unbinding that is what you get. I'm
sure we could fix it by sorting the devices before displaying them,
but it does not seem that important to me. It is more likely that the
unbind support will be enabled in U-Boot proper, and perhaps disabled
in SPL, which doesn't have commands anyway.
I'm fine with "usb tree" showing things the wrong way on builds where
unbinding is disabled. But if I remember the patch-set this thread is
about correctly, it completely removed unbinding from the usb code.
If you do a new version where unbinding is only skipped when compiled
out then that is fine with me.
Regards,
Hans
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
