From: "Vladimir 'φ-coder/phcoder' Serbinenko" <phcoder@gmail.com>
To: The development of GNU GRUB <grub-devel@gnu.org>,
Ales Nesrsta <starous@volny.cz>
Subject: Re: [grub-devel] loongson-2f mini-pc (fuloong) elf image generation.
Date: Thu, 17 Oct 2013 01:31:11 +0200 [thread overview]
Message-ID: <525F21BF.7070707@gmail.com> (raw)
In-Reply-To: <51E59E32.4080608@volny.cz>
[-- Attachment #1: Type: text/plain, Size: 5006 bytes --]
Is any part of this thread still an issue?
I marked it but it seems that there were couple of patches committed
since then and suppsedly it fixes all problems mentioned.
On 16.07.2013 21:25, Aleš Nesrsta wrote:
> 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
>> Grub-devel@gnu.org
>> https://lists.gnu.org/mailman/listinfo/grub-devel
>>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
> .
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 291 bytes --]
next prev parent reply other threads:[~2013-10-16 23:31 UTC|newest]
Thread overview: 79+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-28 16:31 [grub-devel] loongson-2f mini-pc (fuloong) elf image generation Javier Vasquez
2012-10-28 16:36 ` Javier Vasquez
2012-10-28 17:19 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-10-28 23:19 ` Javier Vasquez
2012-10-29 21:47 ` Aleš Nesrsta
2012-10-29 23:03 ` Javier Vasquez
2012-10-30 20:14 ` Aleš Nesrsta
2012-11-03 21:34 ` Javier Vasquez
2012-11-04 17:31 ` Javier Vasquez
2012-11-04 21:05 ` Aleš Nesrsta
2012-11-05 0:11 ` Javier Vasquez
2012-11-04 21:51 ` Vladimir 'φ-coder/phcoder' Serbinenko
2012-11-05 0:14 ` Javier Vasquez
2013-07-12 14:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-12 16:25 ` Aleš Nesrsta
2013-07-12 18:05 ` Lennart Sorensen
2013-07-13 8:13 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-13 18:10 ` Aleš Nesrsta
2013-07-13 19:54 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-13 21:22 ` Aleš Nesrsta
2013-07-15 0:18 ` Javier Vasquez
2013-07-15 3:19 ` Javier Vasquez
2013-07-15 10:26 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-16 13:40 ` Javier Vasquez
2013-07-16 13:50 ` Javier Vasquez
2013-07-16 17:53 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-16 21:17 ` Javier Vasquez
2013-07-15 20:39 ` Aleš Nesrsta
2013-07-16 19:25 ` Aleš Nesrsta
2013-07-16 19:29 ` Aleš Nesrsta
2013-07-18 16:10 ` [PATCH] " Aleš Nesrsta
2013-07-19 5:00 ` Javier Vasquez
2013-07-20 21:56 ` Aleš Nesrsta
2013-07-20 22:43 ` Javier Vasquez
2013-07-21 15:29 ` Aleš Nesrsta
2013-07-21 20:11 ` Javier Vasquez
2013-07-22 20:14 ` starous
2013-07-22 21:00 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-23 21:05 ` starous
2013-07-26 15:30 ` Aleš Nesrsta
2013-07-26 16:59 ` Javier Vasquez
2013-07-26 17:14 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-07-27 22:40 ` Javier Vasquez
2013-07-28 13:56 ` Aleš Nesrsta
2013-07-28 14:04 ` Aleš Nesrsta
2013-09-17 17:24 ` Javier Vasquez
2013-09-17 17:34 ` Javier Vasquez
2013-09-17 21:10 ` Aleš Nesrsta
2013-09-17 21:35 ` Gregg Levine
2013-09-17 22:17 ` Aleš Nesrsta
2013-09-17 19:27 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-09-17 20:33 ` Javier Vasquez
2013-10-27 17:54 ` Javier Vasquez
2013-10-27 18:02 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-10-27 18:33 ` Javier Vasquez
2013-10-27 20:03 ` Aleš Nesrsta
2013-10-27 20:19 ` Javier Vasquez
2013-10-27 21:20 ` Aleš Nesrsta
2013-10-27 22:04 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-10-27 22:18 ` Javier Vasquez
2013-10-27 22:26 ` Javier Vasquez
2013-10-27 22:43 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-10-27 22:51 ` Javier Vasquez
2013-10-27 23:47 ` Javier Vasquez
2013-10-29 18:35 ` Aleš Nesrsta
2013-10-29 18:46 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-01 21:26 ` Aleš Nesrsta
2013-11-01 21:59 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-04 1:10 ` Javier Vasquez
2013-11-04 1:16 ` Vladimir 'φ-coder/phcoder' Serbinenko
2013-11-10 19:03 ` Javier Vasquez
2013-11-10 19:10 ` Javier Vasquez
2013-11-17 12:04 ` Aleš Nesrsta
2013-11-17 18:31 ` Javier Vasquez
2013-12-05 21:18 ` Aleš Nesrsta
2013-12-05 21:34 ` Aleš Nesrsta
2013-09-19 8:13 ` [PATCH] " Vladimir 'φ-coder/phcoder' Serbinenko
2013-10-16 23:31 ` Vladimir 'φ-coder/phcoder' Serbinenko [this message]
2013-10-16 23:39 ` Javier Vasquez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=525F21BF.7070707@gmail.com \
--to=phcoder@gmail.com \
--cc=grub-devel@gnu.org \
--cc=starous@volny.cz \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).