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

[Aleš Nesrsta]

> I fixed the usb keyboard bug on fuloong. Looks like some "antivirus"
> software blocked my message with GRUB binary. Also thers is grub.elf
> (from 2.00~rc1) in downloads section.

Hi Vladimir,
I made some tests with the last changes of USB and there are results:

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