Re: [Patch] [bug #26237] multiple problems with usb devices

[Vladimir 'φ-coder/phcoder' Serbinenko]

Aleš Nesrsta wrote:
> Hello,
>
> according to e-mail from Vladimir Serbinenko I am sending patch related
> to (mostly) OHCI problem (bug #26237).
> Workaround related to grub_memalign problem is removed from patch
> because Vladimir Serbinenko wrotes that it will be corrected.
>   
First of all big thanks for debugging this important aspect. And sorry
for being busy lately.

-  GRUB_OHCI_REG_RHUBA = 18,
-  GRUB_OHCI_REG_RHUBPORT = 21
+  GRUB_OHCI_REG_FRAME_REMAINING,
+  GRUB_OHCI_REG_FRAME_NUMBER,
+  GRUB_OHCI_REG_PERIODIC_START,
+  GRUB_OHCI_REG_LS_TRESHOLD,
+  GRUB_OHCI_REG_RHUB_DESC_A,

Please avoid register renaming unless original ones are misnomers. If
they are I would prefer renaming to be a separate patch.

-  grub_ohci_td_t td_list;
+  grub_ohci_td_t td_list ;
This change is spurious

+   * So we should:
+   * - allow WritebackDoneHead interrupt (WDH) by proper setting of last TD
+   *   token - it is done above in transaction settings
+   * - detect setting of WDH bit in HcInterruptStatus register
+   * - compare HccaDoneHead value with address of last-1 TD. If it is not
+   *   equal, check ED for halt and if not so, reset WDH bit and wait again
+   *   - but it should not happen - debug it!
Are the comments from you or any part copied from spec. We need no copy from 
spec as spec is available anyway and sentences copied from it may cause 
copyright problems
+/* Self commenting... */
Such comments should be removed, but an empty line at this point would be nice
+  err_sense = grub_scsi_request_sense (scsi);
+  /* err_sense is ignored for now and Request Sense Data also... */
When you ignore an error you need to clean grub_errno.
Sth like
if (!err)
  grub_errno = GRUB_ERR_NONE;
     grub_scsi_cmd_inquiry = 0x12,
+    grub_scsi_cmd_test_unit_ready = 0x00,
Please keep this list sorted.
-#define GRUB_USB_FEATURE_ENDP_HALT     0x01
-#define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x02
-#define GRUB_USB_FEATURE_TEST_MODE     0x04
+#define GRUB_USB_FEATURE_ENDP_HALT     0x00
+#define GRUB_USB_FEATURE_DEV_REMOTE_WU 0x01
Why do you need to change these constants? Were they wrong?
Tomorrow I'll test your patch on both UHCI and OHCI.

+#define GRUB_USB_FEATURE_TEST_MODE     0x02
     grub_scsi_cmd_read_capacity = 0x25,
+

> Patch was done by:
>
> diff -urB  grub2-1.98~experimental.20100120 grub2-1.98~my_patched.20100312 > 
> grub2-1.98~my_patched.20100312.patch
>
> As I shortly read the patch file, there sometimes remains some few
> things which can be optimized - sorry, it is mostly the rest from
> debugging experiments (even I make cleaning of code today)...
>
> Ales
>
>   
>> Od: Vladimir Serbinenko <address@hidden>
>> Komu: Oliver Henshaw <address@hidden>, Vladimir Serbinenko
>> <address@hidden>, Ales Nesrsta <address@hidden>, address@hidden
>> Předmět: [bug #26237] multiple problems with usb devices
>> Datum: Sat, 13 Mar 2010 22:08:44 +0000
>>
>> Update of bug #26237 (project grub):
>>
>>                  Release:                    None => Bazaar - trunk         
>>
>>     _______________________________________________________
>>
>> Follow-up Comment #6:
>>
>> Hello. I found out the problem with grub_memalign. I'll apply the patch
>> tomorrow. Some of the fixes can go directly in while other require
>> investigation and perhaps even copyright assignment. Can you remove all
>> gratuituous changes (like commenting out dprintfs, adding commented out code
>> and change register names) and send patch in unified format (-u option in
>> diff) to address@hidden Porting code from linux is appropriate only if
>> license is compatible and porting was approved by maintainer. Are perhaps
>> interested in coding EHCI?
>>
>>     _______________________________________________________
>>
>> Reply to this item at:
>>
>>   <http://savannah.gnu.org/bugs/?26237>
>>
>> _______________________________________________
>>   Message sent via/by Savannah
>>   http://savannah.gnu.org/
>>
>>
>>     
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> Grub-devel mailing list
>> address@hidden
>> http://lists.gnu.org/mailman/listinfo/grub-devel


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature