grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [grub-devel] loongson-2f mini-pc (fuloong) elf image generation.


From: Aleš Nesrsta
Subject: Re: [grub-devel] loongson-2f mini-pc (fuloong) elf image generation.
Date: Tue, 16 Jul 2013 21:25:38 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130625 Thunderbird/17.0.7

Hi Vladimir,

I have some additional information - results of some today's experiments.
The main important thing related to USB keyboard:

USB keyboard doesn't work if it is connected as first device to the same non-root hub port, where was connected USBMS device previously! If the keyboard is disconnected and connected again to the same port, it works. this behavior is systematical, it can be repeated at any time (at least on my PC).

What is very interesting, USB keyboard is listed by command "usb".
And it doesn't matter if previously connected USBMS device was HIGH of FULL device.

I.e., according these test results, even if the USB keybord is not working, it is connected to port and addressed properly and its control pipe is working, but device itself is not working - maybe there is something wrong in "high level" driver (usb_keyboard) - ?


There is also another bad thing - after some number of connecting/disconnecting of the keyboard (at least about 20 and more) the GRUB crashes - more properly, it reboots PC, i.e. there is probably also some memory leak...

BR,
Ales


1.
Some of my USBMS devices have problems to work properly. It seems to be
some regression because they worked well on some older revisions... :-(
I did not make any investigation it this direction yet, as this problem
is probably not related to latest changes (fix of root ports) - so I
ignore them for now.

2.
Sometimes some devices are not recognized (not working) in the case when
they are connected before USB "starting" time (before the moment when
USB modules/drivers are loaded) or when hub with this device is connected.
Additionally, it seems to happen only if device is connected via hub(s),
not directly into root port - at least it was behavior during my tests
(but I did only few tests on root ports, I focused my tests to USB
keyboard connected via hub(s) etc.).
It looks like, in some rare cases, usbhub.c maybe miss some non-root hub
port change(s). Unfortunately I had no enough time to try debug these
situations.

So I went through Your changes in usbhub.c and other USB related files.
Unfortunately, I did not found reason of the problem mentioned above in
point 2. yet.
But I found some another points to discuss:

a)
I found my old mistake related to variable "pending_reset".
Meaning of this variable is to avoid concurrent reset on devices which
are connected to the same controller HW instance.
This variable is stored in wrong place - it should be located not in
"struct grub_usb_controller_dev" but in "struct grub_*hci" (i.e.
grub_uhci, grub_ohci etc.).
In fact, its current location is not totally bad - it is also working,
but it can slow down handling of USB devices (mainly in USB "starting"
phase) in case when there are more controllers of the same type.


b)
There is missing waiting for device stable power in case when device is
connected to ROOT hub later than in USB "starting" time.
This could possibly lead to wrong device reset and malfunction.

I.e. the "first half" of "grub_usb_poll_devices" should be little bit
changed, it is not correct to call "attach_root_port" immediately when
"detect_dev" detected device connection - it should be done e.g. in
similar way as in "grub_usb_controller_dev_register" (or maybe better in
some another, "background" way, like You did for non-root hub - to
prevent unwanted delay in execution of another GRUB parts).


c)
I thought about this old code:

"...
poll_nonroot_hub (grub_usb_device_t dev)
{
   grub_usb_err_t err;
   unsigned i;
   grub_uint8_t changed;
   grub_size_t actual, len;

   if (!dev->hub_transfer)
     return;
..."

I think, as the possible "error recovery", the better than current
immediate return could be to try to call "grub_usb_bulk_read_background"
to schedule new background transfer for this hub before return - ?


d)
Cosmetic thing:
It will be fine to rename declaration
static struct grub_usb_hub *hubs;
to
static struct grub_usb_hub *root_hubs;
to be more self explanative... :-)


What do You think about the points a)-d) ?

BR,
Ales

_______________________________________________
Grub-devel mailing list
address@hidden
https://lists.gnu.org/mailman/listinfo/grub-devel




reply via email to

[Prev in Thread] Current Thread [Next in Thread]