On 06/26/2010 12:03 AM, Aleš Nesrsta wrote:
> Vladimir 'φ-coder/phcoder' Serbinenko wrote:
>
>> On 06/20/2010 11:23 AM, Aleš Nesrsta wrote:
>>
>>> Hi,
>>>
>>> I found some mistake in uhci.c in grub_uhci_portstatus when enable=0.
>>> There is proposal of correction.
>>>
>>> Without correction portstatus reported false timeout when enable=0
>>> because it is waiting for reset to be done but none is performed...
>>>
>>>
>>>
>> This patch seems to change much more that you say. In particular
>> enable=0 is a request to disable port and you seem to always enable it.
>> This is likely to break other code.
>>
> Hi,
> You are right according to some possible side-effects. But the lines
> ...
> if (!enable) /* We don't need reset port */
> {
> /* Disable the port. */
> grub_uhci_writereg16 (u, reg, 0 << 2);
> ...
> should disable the port as the bit "Port Enable" is reset.
>
> Port reset should be not necessary when disabling port - according to
> USB specification, reset of port should be done only to enable port and
> mainly to bring newly connected device to "Default" state (i.e. to state
> when device accepts communication via address 0).
>
> Of course:
> - I can be wrong
> - it should be tested according to some side-effect
>
> But in original code is real bug - if this function is called with
> enable=0, it reports false timeout as it is waiting for bit which will
> never set in this case.
> This bug should be corrected in some way.
>
>
I have nothing against that change. I was mainly referring to:- grub_uhci_writereg16 (u, reg, enable << 9); + grub_uhci_writereg16 (u, reg, 1 << 9); Which seems to always enable the port. > There is also question, why does the function attach_root_port (in > usbhub.c) disable and enable of port before initialize connected > device ? > Reset & enable of port (if new device is connected) should be enough, > because, according to USB specification: > - hub should automatically disable the port if device is disconnected or > port is not powered > - ports should be disabled by hub after power-up of hub > But maybe there are some special cases or buggy hubs/devices which needs > such behavior (?) - I don't know, so I didn't touch this part of code. > > It's the right strategy: if it doesn't bug and unlikely to, leave it alone. > Best regards > Ales > > >>> Best regards >>> Ales >>> >>> >>> >>> _______________________________________________ >>> Grub-devel mailing list >>> Grub-devel@gnu.org >>> http://lists.gnu.org/mailman/listinfo/grub-devel >>> >>> >> >> _______________________________________________ >> Grub-devel mailing list >> Grub-devel@gnu.org >> http://lists.gnu.org/mailman/listinfo/grub-devel >> > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > http://lists.gnu.org/mailman/listinfo/grub-devel > > -- Regards Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
