[Patch] Simple USB hub support

[Patch] Simple USB hub support

[Aleš Nesrsta]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Hi,

included patch should make USB hubs operational.

It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
and device must be connected to computer BEFORE uhci/ohci module is
loaded.

Note (mainly for Vladimir):
Via hub are normally working also devices which I reported to be not
functional on UHCI. So, from my point of view it looks like there is
surely some problem in powering of UHCI root hubs on my computer. But I
still don't know why only some devices are sensitive to this problem
(all of them are not big consumers of power).

Best regards
Ales


[-- Attachment #2: usb_patch_hub_100620_0 --]
[-- Type: text/x-patch, Size: 5884 bytes --]

diff -urB ./usb/bus/usb/usbhub.c ./usb_patched/bus/usb/usbhub.c
--- ./usb/bus/usb/usbhub.c	2010-06-20 10:20:58.000000000 +0200
+++ ./usb_patched/bus/usb/usbhub.c	2010-06-18 22:34:56.000000000 +0200
@@ -87,14 +87,47 @@
   struct grub_usb_usb_hubdesc hubdesc;
   grub_err_t err;
   int i;
+  grub_uint64_t timeout;
+  grub_usb_device_t next_dev;
+  
+  err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+	  		            | GRUB_USB_REQTYPE_CLASS
+			            | GRUB_USB_REQTYPE_TARGET_DEV),
+                              GRUB_USB_REQ_GET_DESCRIPTOR,
+			      (GRUB_USB_DESCRIPTOR_HUB << 8) | 0,
+			      0, sizeof (hubdesc), (char *) &hubdesc);
+  if (err)
+    return err;
+  grub_dprintf ("usb", "Hub descriptor:\n\t\t len:%d, typ:0x%02x, cnt:%d, char:0x%02x, pwg:%d, curr:%d\n",
+                hubdesc.length, hubdesc.type, hubdesc.portcnt,
+                hubdesc.characteristics, hubdesc.pwdgood,
+                hubdesc.current);
+
+  /* Activate the first configuration. Hubs should have only one conf. */
+  grub_dprintf ("usb", "Hub set configuration\n");
+  grub_usb_set_configuration (dev, 1);
 
-  grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
-			      | GRUB_USB_REQTYPE_CLASS
-			      | GRUB_USB_REQTYPE_TARGET_DEV),
-			GRUB_USB_REQ_GET_DESCRIPTOR,
-			(GRUB_USB_DESCRIPTOR_HUB << 8) | 0,
-			0, sizeof (hubdesc), (char *) &hubdesc);
-
+  /* Power on all Hub ports.  */
+  for (i = 1; i <= hubdesc.portcnt; i++)
+    {
+      grub_dprintf ("usb", "Power on - port %d\n", i);
+      /* Power on the port and wait for possible device connect */
+      err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+					| GRUB_USB_REQTYPE_CLASS
+					| GRUB_USB_REQTYPE_TARGET_OTHER),
+				  GRUB_USB_REQ_SET_FEATURE,
+				  GRUB_USB_HUB_FEATURE_PORT_POWER,
+				  i, 0, NULL);
+      /* Just ignore the device if some error happened */
+      if (err)
+	continue;
+    }
+  /* Wait for port power-on */
+  if (hubdesc.pwdgood >= 50)
+    grub_millisleep (hubdesc.pwdgood * 2);
+  else
+    grub_millisleep (100);
+    
   /* Iterate over the Hub ports.  */
   for (i = 1; i <= hubdesc.portcnt; i++)
     {
@@ -104,9 +137,8 @@
       err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
 					| GRUB_USB_REQTYPE_CLASS
 					| GRUB_USB_REQTYPE_TARGET_OTHER),
-				  GRUB_USB_REQ_HUB_GET_PORT_STATUS,
+				  GRUB_USB_REQ_GET_STATUS,
 				  0, i, sizeof (status), (char *) &status);
-
       /* Just ignore the device if the Hub does not report the
 	 status.  */
       if (err)
@@ -111,7 +143,8 @@
 	 status.  */
       if (err)
 	continue;
-
+      grub_dprintf ("usb", "Hub port %d status: 0x%02x\n", i, status);
+      	    
       /* If connected, reset and enable the port.  */
       if (status & GRUB_USB_HUB_STATUS_CONNECTED)
 	{
@@ -128,21 +161,46 @@
 		speed = GRUB_USB_SPEED_FULL;
 	    }
 
-	  /* A device is actually connected to this port, not enable
-	     the port.  XXX: Why 0x03?  According to some docs it
-	     should be 0x0.  Check the specification!  */
+	  /* A device is actually connected to this port.
+	   * Now do reset of port. */
+          grub_dprintf ("usb", "Reset hub port - port %d\n", i);
 	  err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
 					    | GRUB_USB_REQTYPE_CLASS
 					    | GRUB_USB_REQTYPE_TARGET_OTHER),
-				      0x3, 0x4, i, 0, 0);
-
+				      GRUB_USB_REQ_SET_FEATURE,
+				      GRUB_USB_HUB_FEATURE_PORT_RESET,
+				      i, 0, 0);
 	  /* If the Hub does not cooperate for this port, just skip
 	     the port.  */
 	  if (err)
 	    continue;
 
+          /* Wait for reset procedure done */
+          timeout = grub_get_time_ms () + 1000;
+          do
+            {
+              /* Get the port status.  */
+              err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+	   	 			        | GRUB_USB_REQTYPE_CLASS
+					        | GRUB_USB_REQTYPE_TARGET_OTHER),
+				          GRUB_USB_REQ_GET_STATUS,
+				          0, i, sizeof (status), (char *) &status);
+            }
+          while (!err &&
+                 !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) &&
+                 (grub_get_time_ms() < timeout) );
+          if (err || !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) )
+            continue;
+   
 	  /* Add the device and assign a device address to it.  */
-	  grub_usb_hub_add_dev (&dev->controller, speed);
+          grub_dprintf ("usb", "Call hub_add_dev - port %d\n", i);
+	  next_dev = grub_usb_hub_add_dev (&dev->controller, speed);
+          if (! next_dev)
+            continue;
+
+          /* If the device is a Hub, scan it for more devices.  */
+          if (next_dev->descdev.class == 0x09)
+            grub_usb_add_hub (next_dev);
 	}
     }
 
@@ -156,7 +214,7 @@
   grub_usb_device_t dev;
   grub_err_t err;
 
-  /* Enable the port.  */
+  /* Disable the port. XXX: Why? */
   err = controller->dev->portstatus (controller, portno, 0);
   if (err)
     return;
diff -urB ./usb/include/grub/usbtrans.h ./usb_patched/include/grub/usbtrans.h
--- ./usb/include/grub/usbtrans.h	2010-06-20 10:20:58.000000000 +0200
+++ ./usb_patched/include/grub/usbtrans.h	2010-06-20 10:27:13.000000000 +0200
@@ -87,15 +87,17 @@
 #define GRUB_USB_REQ_SET_INTERFACE	0x0B
 #define GRUB_USB_REQ_SYNC_FRAME		0x0C
 
-#define GRUB_USB_REQ_HUB_GET_PORT_STATUS 0x00
-
 #define GRUB_USB_FEATURE_ENDP_HALT	0x00
 #define GRUB_USB_FEATURE_DEV_REMOTE_WU	0x01
 #define GRUB_USB_FEATURE_TEST_MODE	0x02
 
-#define GRUB_USB_HUB_STATUS_CONNECTED	(1 << 0)
-#define GRUB_USB_HUB_STATUS_LOWSPEED	(1 << 9)
-#define GRUB_USB_HUB_STATUS_HIGHSPEED	(1 << 10)
+#define GRUB_USB_HUB_FEATURE_PORT_RESET   0x04
+#define GRUB_USB_HUB_FEATURE_PORT_POWER   0x08
+
+#define GRUB_USB_HUB_STATUS_CONNECTED	 (1 << 0)
+#define GRUB_USB_HUB_STATUS_LOWSPEED	 (1 << 9)
+#define GRUB_USB_HUB_STATUS_HIGHSPEED	 (1 << 10)
+#define GRUB_USB_HUB_STATUS_C_PORT_RESET (1 << 20)
 
 struct grub_usb_packet_setup
 {

Re: [Patch] Simple USB hub support

[richardvoigt]

On Sun, Jun 20, 2010 at 4:21 AM, Aleš Nesrsta <starous@volny.cz> wrote:
> Hi,
>
> included patch should make USB hubs operational.
>
> It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
> and device must be connected to computer BEFORE uhci/ohci module is
> loaded.
>
> Note (mainly for Vladimir):
> Via hub are normally working also devices which I reported to be not
> functional on UHCI. So, from my point of view it looks like there is
> surely some problem in powering of UHCI root hubs on my computer. But I
> still don't know why only some devices are sensitive to this problem
> (all of them are not big consumers of power).

When connected via USB-HS hub, even low-speed and full-speed devices
enumerate under the EHCI, not UHCI.  So it's not surprising that the
behavior changes and the devices start working.

Can you verify the speed of the devices in question, and that the hub
is USB 2.0 high-speed?

>
> Best regards
> Ales
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>

Re: [Patch] Simple USB hub support

[Aleš Nesrsta]

richardvoigt@gmail.com wrote:
> On Sun, Jun 20, 2010 at 4:21 AM, Aleš Nesrsta <starous@volny.cz> wrote:
> > Hi,
> >
> > included patch should make USB hubs operational.
> >
> > It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
> > and device must be connected to computer BEFORE uhci/ohci module is
> > loaded.
> >
> > Note (mainly for Vladimir):
> > Via hub are normally working also devices which I reported to be not
> > functional on UHCI. So, from my point of view it looks like there is
> > surely some problem in powering of UHCI root hubs on my computer. But I
> > still don't know why only some devices are sensitive to this problem
> > (all of them are not big consumers of power).
> 
> When connected via USB-HS hub, even low-speed and full-speed devices
> enumerate under the EHCI, not UHCI.  So it's not surprising that the
> behavior changes and the devices start working.
> 
> Can you verify the speed of the devices in question, and that the hub
> is USB 2.0 high-speed?
> 

Yes, the hub is USB 2.0 high-speed.

But I am still confused:

My PC has EHCI-UHCI USB controllers (1x EHCI, 4x UHCI).
GRUB2 supports UHCI only yet, i.e. EHCI is not touched by GRUB2, it is
in some unknown state (maybe initialized in some way by BIOS - but it
does not matter if I enable or disable it in BIOS).

Now:
1.
I connect device APACER USB flashdisk directly to USB port on PC and it
works. APACER is USB 2.0 high-speed device (see lsusb at the end).

2.
I connect another device - Alcor card reader directly to USB port on PC
and it does not work. Alcor is USB 2.0 high-speed device (see lsusb at
the end).

3.
I connect Alcor via USB 2.0 high-speed hub to the same PC and it
works...
(Unfortunately I have no USB1.1 hub to try behavior in this case...)


Both devices are USB 2.0 high-speed but one is working and second does
not - where is the difference...?


There are lsusb outputs of both devices from Linux (I see no significant
differences, only MAxPower is different - but it is lower on device
which is not working, so problem is probably not caused by
overcurrent.):


"Working" device - Apacer:
Bus 005 Device 004: ID 1005:b113 Apacer Technology, Inc. Handy Steno
2.0/HT203
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x1005 Apacer Technology, Inc.
  idProduct          0xb113 Handy Steno 2.0/HT203
  bcdDevice            1.00
  iManufacturer           1         
  iProduct                2 USB FLASH DRIVE 
  iSerial                 3 19771C0607E2
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           32
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              200mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         8 Mass Storage
      bInterfaceSubClass      6 SCSI
      bInterfaceProtocol     80 Bulk (Zip)
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x02  EP 2 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  bNumConfigurations      1
Device Status:     0x0000
  (Bus Powered)


"Not working" device - Alcor:
Bus 005 Device 005: ID 058f:6366 Alcor Micro Corp. Multi Flash Reader
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x058f Alcor Micro Corp.
  idProduct          0x6366 Multi Flash Reader
  bcdDevice            1.00
  iManufacturer           1 Generic
  iProduct                2 Mass Storage Device
  iSerial                 3 058F63666433
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           32
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0x80
      (Bus Powered)
    MaxPower              100mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           2
      bInterfaceClass         8 Mass Storage
      bInterfaceSubClass      6 SCSI
      bInterfaceProtocol     80 Bulk (Zip)
      iInterface              0 
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x01  EP 1 OUT
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            2
          Transfer Type            Bulk
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0200  1x 512 bytes
        bInterval               0
Device Qualifier (for other device speed):
  bLength                10
  bDescriptorType         6
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  bNumConfigurations      1
Device Status:     0x0000
  (Bus Powered)


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
> 

Re: [Patch] Simple USB hub support

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 951 bytes --]

On 06/20/2010 11:21 AM, Aleš Nesrsta wrote:
> Hi,
>
> included patch should make USB hubs operational.
>
> It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
> and device must be connected to computer BEFORE uhci/ohci module is
> loaded.
>
>   
You can go ahead nd commit to mainline
> Note (mainly for Vladimir):
> Via hub are normally working also devices which I reported to be not
> functional on UHCI. So, from my point of view it looks like there is
> surely some problem in powering of UHCI root hubs on my computer. But I
> still don't know why only some devices are sensitive to this problem
> (all of them are not big consumers of power).
>
> Best regards
> Ales
>
>   
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] Simple USB hub support

[Aleš Nesrsta]

Vladimir 'φ-coder/phcoder' Serbinenko píše v Po 28. 06. 2010 v 18:44
+0200:
> On 06/20/2010 11:21 AM, Aleš Nesrsta wrote:
> > Hi,
> >
> > included patch should make USB hubs operational.
> >
> > It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
> > and device must be connected to computer BEFORE uhci/ohci module is
> > loaded.
> >
> >   
> You can go ahead nd commit to mainline

OK, I committed it into trunk as rev. 2521.
Regards, Ales

> > Note (mainly for Vladimir):
> > Via hub are normally working also devices which I reported to be not
> > functional on UHCI. So, from my point of view it looks like there is
> > surely some problem in powering of UHCI root hubs on my computer. But I
> > still don't know why only some devices are sensitive to this problem
> > (all of them are not big consumers of power).
> >
> > 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

Re: [Patch] Simple USB hub support

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

On 07/05/2010 07:11 PM, Aleš Nesrsta wrote:
> Vladimir 'φ-coder/phcoder' Serbinenko píše v Po 28. 06. 2010 v 18:44
> +0200:
>   
>> On 06/20/2010 11:21 AM, Aleš Nesrsta wrote:
>>     
>>> Hi,
>>>
>>> included patch should make USB hubs operational.
>>>
>>> It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
>>> and device must be connected to computer BEFORE uhci/ohci module is
>>> loaded.
>>>
>>>   
>>>       
>> You can go ahead nd commit to mainline
>>     
> OK, I committed it into trunk as rev. 2521.
>   
Apparently your commit has gone wrong. The only change it did was to
include time.h. Could you recommit?
> Regards, Ales
>
>   
>>> Note (mainly for Vladimir):
>>> Via hub are normally working also devices which I reported to be not
>>> functional on UHCI. So, from my point of view it looks like there is
>>> surely some problem in powering of UHCI root hubs on my computer. But I
>>> still don't know why only some devices are sensitive to this problem
>>> (all of them are not big consumers of power).
>>>
>>> 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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] Simple USB hub support

[Aleš Nesrsta]

Vladimir 'φ-coder/phcoder' Serbinenko píše v Út 06. 07. 2010 v 01:59
+0200:
> On 07/05/2010 07:11 PM, Aleš Nesrsta wrote:
> > Vladimir 'φ-coder/phcoder' Serbinenko píše v Po 28. 06. 2010 v 18:44
> > +0200:
> >   
> >> On 06/20/2010 11:21 AM, Aleš Nesrsta wrote:
> >>     
> >>> Hi,
> >>>
> >>> included patch should make USB hubs operational.
> >>>
> >>> It works in the same simple way as whole GRUB2 USB support, i.e. USB hub
> >>> and device must be connected to computer BEFORE uhci/ohci module is
> >>> loaded.
> >>>
> >>>   
> >>>       
> >> You can go ahead nd commit to mainline
> >>     
> > OK, I committed it into trunk as rev. 2521.
> >   
> Apparently your commit has gone wrong. The only change it did was to
> include time.h. Could you recommit?

Hmmm, I pulled current revision 2528 and it looks like changes are
included...?

Maybe I made little bit mess because I did not wrote changes in
ChangeLog (for revisions 2521 and 2522) and I was warned by Colin Watson
that it is needed.
So, I added text into Change log and additionally corrected usbhub.c
(because I forgot include "#include <grub/time.h>" in changes committed
into revision 2521) and committed this as revision 2523.
Now, if you look into changes of revision 2523, you will see only
Changelog and "include" changes...

There is overview of my commits:
2521 - Simple USB hub support (with mistake - missing "include")
2522 - USB UHCI portstatus correction
2523 - ChangeLog and correction of "include" in usbhub.c

Sorry for such troubles... I am still newbee and occasional programmer
only... :-)

Regards
Ales

> > Regards, Ales
> >
> >   
> >>> Note (mainly for Vladimir):
> >>> Via hub are normally working also devices which I reported to be not
> >>> functional on UHCI. So, from my point of view it looks like there is
> >>> surely some problem in powering of UHCI root hubs on my computer. But I
> >>> still don't know why only some devices are sensitive to this problem
> >>> (all of them are not big consumers of power).
> >>>
> >>> 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
> >
> >   
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel

Re: [Patch] Simple USB hub support

[Colin Watson]

On Tue, Jul 06, 2010 at 07:14:47PM +0200, Aleš Nesrsta wrote:
> Vladimir 'φ-coder/phcoder' Serbinenko píše v Út 06. 07. 2010 v 01:59
> +0200:
> > Apparently your commit has gone wrong. The only change it did was to
> > include time.h. Could you recommit?
> 
> Hmmm, I pulled current revision 2528 and it looks like changes are
> included...?

BVK Chaitanya had to restore the #include <grub/time.h> change this
morning, because Vladimir's revert broke the trunk build.  That was the
smallest change that fixed it.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

[Patch] USB hot-plugging support - usb branch

[Aleš Nesrsta]

[-- Attachment #1: Type: text/plain, Size: 1331 bytes --]

Hi,

I little bit changed "hot-plugging" support added into "usb" branch by
Vladimir - see included patch. For the first look it maybe works on
UHCI, I did not test it on OHCI yet...

Basic info to this patch:

1.
This patch is made against current usb branch code (rev. 2427).
This patch is not committed into usb branch yet - it was tested only
once and very simply and it does not work on hubs (i.e. on non-root
hubs; some timeout happens here).
I will continue on development and testing soon (if it will be
possible...) and I will commit into usb some better version later (I
hope...).

2.
I am not using original comparing of "hub->speed[i]" and "speed" to
detect connection status change (but I left the "hub" structure in code,
it can be maybe useful in future). I am using built-in functions of
OHCI/UHCI controllers (available via root hub port register) which
should be more reliable and are more systematic and closed to
specifications.

3.
There is missing handling of disconnected devices. For this we will need
to know on which port which device is attached - it is currently not
known - and we need to know it "recursively" because devices can be
connected indirectly via non-root hubs.
On this part I probably will not work in near future (currently I have
no simple and suitable idea how to do it).

Regards
Ales

[-- Attachment #2: usb_patch_hotplug_100707_0 --]
[-- Type: text/x-patch, Size: 14122 bytes --]

diff -urB ./my-repo/usb/bus/usb/ohci.c ./usb/bus/usb/ohci.c
--- ./my-repo/usb/bus/usb/ohci.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/ohci.c	2010-07-06 22:33:35.000000000 +0200
@@ -148,6 +148,7 @@
 #define GRUB_OHCI_REG_CONTROL_BULK_ENABLE (1 << 5)
 #define GRUB_OHCI_REG_CONTROL_CONTROL_ENABLE (1 << 4)
 
+#define GRUB_OHCI_RESET_CONNECT_CHANGE (1 << 16)
 #define GRUB_OHCI_CTRL_EDS 16
 #define GRUB_OHCI_BULK_EDS 16
 #define GRUB_OHCI_TDS 256
@@ -1219,6 +1220,11 @@
 			   GRUB_OHCI_CLEAR_PORT_ENABLE);
    grub_millisleep (10);
 
+   if (enable)
+     /* Reset bit Connect Status Change */
+     grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
+                           GRUB_OHCI_RESET_CONNECT_CHANGE);
+
    grub_dprintf ("ohci", "end of portstatus=0x%02x\n",
 		 grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port));
  
@@ -1226,7 +1232,7 @@
 }
 
 static grub_usb_speed_t
-grub_ohci_detect_dev (grub_usb_controller_t dev, int port)
+grub_ohci_detect_dev (grub_usb_controller_t dev, int port, int *changed)
 {
    struct grub_ohci *o = (struct grub_ohci *) dev->data;
    grub_uint32_t status;
@@ -1235,6 +1241,9 @@
 
    grub_dprintf ("ohci", "detect_dev status=0x%02x\n", status);
 
+  /* Connect Status Change bit - it detects change of connection */
+  *changed = ((status & GRUB_OHCI_RESET_CONNECT_CHANGE) != 0);
+
    if (! (status & 1))
      return GRUB_USB_SPEED_NONE;
    else if (status & (1 << 9))
diff -urB ./my-repo/usb/bus/usb/uhci.c ./usb/bus/usb/uhci.c
--- ./my-repo/usb/bus/usb/uhci.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/uhci.c	2010-07-06 22:24:31.000000000 +0200
@@ -644,10 +644,14 @@
   grub_dprintf ("uhci", "waiting for the port to be enabled\n");
 
   endtime = grub_get_time_ms () + 1000;
-  while (! (grub_uhci_readreg16 (u, reg) & (1 << 2)))
+  while (! ((status = grub_uhci_readreg16 (u, reg)) & (1 << 2)))
     if (grub_get_time_ms () > endtime)
       return grub_error (GRUB_ERR_IO, "UHCI Timed out");
 
+  /* Reset bit Connect Status Change */
+  grub_uhci_writereg16 (u, reg, status | (1 << 1));
+
+  /* Read final port status */
   status = grub_uhci_readreg16 (u, reg);
   grub_dprintf ("uhci", ">3detect=0x%02x\n", status);
 
@@ -656,7 +660,7 @@
 }
 
 static grub_usb_speed_t
-grub_uhci_detect_dev (grub_usb_controller_t dev, int port)
+grub_uhci_detect_dev (grub_usb_controller_t dev, int port, int *changed)
 {
   struct grub_uhci *u = (struct grub_uhci *) dev->data;
   int reg;
@@ -676,6 +680,9 @@
 
   grub_dprintf ("uhci", "detect=0x%02x port=%d\n", status, port);
 
+  /* Connect Status Change bit - it detects change of connection */
+  *changed = ((status & (1 << 1)) != 0);
+    
   if (! (status & 1))
     return GRUB_USB_SPEED_NONE;
   else if (status & (1 << 8))
diff -urB ./my-repo/usb/bus/usb/usb.c ./usb/bus/usb/usb.c
--- ./my-repo/usb/bus/usb/usb.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/usb.c	2010-07-07 00:23:38.000000000 +0200
@@ -225,6 +225,20 @@
 	}
     }
 
+  return GRUB_USB_ERR_NONE;
+
+ fail:
+
+  for (i = 0; i < 8; i++)
+    grub_free (dev->config[i].descconf);
+
+  return err;
+}
+
+void grub_usb_device_attach (grub_usb_device_t dev)
+{
+  int i;
+  
   /* XXX: Just check configuration 0 for now.  */
   for (i = 0; i < dev->config[0].descconf->numif; i++)
     {
@@ -243,15 +257,6 @@
 	if (interf->class == desc->class && desc->hook (dev, 0, i))
 	  dev->config[0].interf[i].attached = 1;
     }
-
-  return GRUB_USB_ERR_NONE;
-
- fail:
-
-  for (i = 0; i < 8; i++)
-    grub_free (dev->config[i].descconf);
-
-  return err;
 }
 
 void
diff -urB ./my-repo/usb/bus/usb/usbhub.c ./usb/bus/usb/usbhub.c
--- ./my-repo/usb/bus/usb/usbhub.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/usbhub.c	2010-07-07 00:19:13.000000000 +0200
@@ -23,8 +23,10 @@
 #include <grub/misc.h>
 #include <grub/time.h>
 
+#define GRUB_USBHUB_MAX_DEVICES 128
+
 /* USB Supports 127 devices, with device 0 as special case.  */
-static struct grub_usb_device *grub_usb_devs[128];
+static struct grub_usb_device *grub_usb_devs[GRUB_USBHUB_MAX_DEVICES];
 
 struct grub_usb_hub
 {
@@ -44,6 +46,7 @@
 {
   grub_usb_device_t dev;
   int i;
+  grub_usb_err_t err;
 
   dev = grub_zalloc (sizeof (struct grub_usb_device));
   if (! dev)
@@ -52,31 +55,48 @@
   dev->controller = *controller;
   dev->speed = speed;
 
-  grub_usb_device_initialize (dev);
+  err = grub_usb_device_initialize (dev);
+  if (err)
+    {
+      grub_free (dev);
+      return NULL;
+    }
 
   /* Assign a new address to the device.  */
-  for (i = 1; i < 128; i++)
+  for (i = 1; i < GRUB_USBHUB_MAX_DEVICES; i++)
     {
       if (! grub_usb_devs[i])
 	break;
     }
-  if (i == 128)
+  if (i == GRUB_USBHUB_MAX_DEVICES)
     {
       grub_error (GRUB_ERR_IO, "can't assign address to USB device");
+      for (i = 0; i < 8; i++)
+        grub_free (dev->config[i].descconf);
+      grub_free (dev);
       return NULL;
     }
 
-  grub_usb_control_msg (dev,
-			(GRUB_USB_REQTYPE_OUT
-			 | GRUB_USB_REQTYPE_STANDARD
-			 | GRUB_USB_REQTYPE_TARGET_DEV),
-			GRUB_USB_REQ_SET_ADDRESS,
-			i, 0, 0, NULL);
+  err = grub_usb_control_msg (dev,
+			      (GRUB_USB_REQTYPE_OUT
+			       | GRUB_USB_REQTYPE_STANDARD
+			       | GRUB_USB_REQTYPE_TARGET_DEV),
+			      GRUB_USB_REQ_SET_ADDRESS,
+			      i, 0, 0, NULL);
+  if (err)
+    {
+      for (i = 0; i < 8; i++)
+        grub_free (dev->config[i].descconf);
+      grub_free (dev);
+      return NULL;
+    }
 
   dev->addr = i;
   dev->initialized = 1;
   grub_usb_devs[i] = dev;
 
+  grub_usb_device_attach (dev);
+  
   return dev;
 }
 
@@ -144,7 +164,7 @@
       if (err)
 	continue;
       grub_dprintf ("usb", "Hub port %d status: 0x%02x\n", i, status);
-      	    
+
       /* If connected, reset and enable the port.  */
       if (status & GRUB_USB_HUB_STATUS_CONNECTED)
 	{
@@ -192,6 +212,18 @@
           if (err || !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) )
             continue;
    
+          /* Do reset of connection change bit */
+          err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+                                            | GRUB_USB_REQTYPE_CLASS
+                                            | GRUB_USB_REQTYPE_TARGET_OTHER),
+                                      GRUB_USB_REQ_CLEAR_FEATURE,
+				      GRUB_USB_HUB_FEATURE_C_CONNECTED,
+				      i, 0, 0);
+          /* Just ignore the device if the Hub reports some error */
+          if (err)
+	     continue;
+          grub_dprintf ("usb", "Hub port - cleared connection change\n");
+
 	  /* Add the device and assign a device address to it.  */
           grub_dprintf ("usb", "Call hub_add_dev - port %d\n", i);
 	  next_dev = grub_usb_hub_add_dev (&dev->controller, speed);
@@ -239,6 +271,7 @@
 {
   int i;
   struct grub_usb_hub *hub;
+  int changed=0;
 
   hub = grub_malloc (sizeof (*hub));
   if (!hub)
@@ -248,7 +281,10 @@
   hubs = hub;
   hub->controller = grub_malloc (sizeof (*controller));
   if (!hub->controller)
-    return GRUB_USB_ERR_INTERNAL;
+    {
+      grub_free (hub);
+      return GRUB_USB_ERR_INTERNAL;
+    }
 
   grub_memcpy (hub->controller, controller, sizeof (*controller));
   hub->dev = 0;
@@ -258,13 +294,15 @@
   hub->speed = grub_malloc (sizeof (hub->speed[0]) * hub->nports);
   if (!hub->speed)
     {
+      grub_free (hub->controller);
       grub_free (hub);
       return GRUB_USB_ERR_INTERNAL;
     }
 
   for (i = 0; i < hub->nports; i++)
     {
-      hub->speed[i] = controller->dev->detect_dev (hub->controller, i);
+      hub->speed[i] = controller->dev->detect_dev (hub->controller, i,
+                                                   &changed);
 
       if (hub->speed[i] != GRUB_USB_SPEED_NONE)
 	attach_root_port (hub->controller, i, hub->speed[i]);
@@ -273,30 +311,154 @@
   return GRUB_USB_ERR_NONE;
 }
 
+static void
+poll_nonroot_hub (grub_usb_device_t dev)
+{
+  struct grub_usb_usb_hubdesc hubdesc;
+  grub_err_t err;
+  int i;
+  grub_uint64_t timeout;
+  grub_usb_device_t next_dev;
+  
+  err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+	  		            | GRUB_USB_REQTYPE_CLASS
+			            | GRUB_USB_REQTYPE_TARGET_DEV),
+                              GRUB_USB_REQ_GET_DESCRIPTOR,
+			      (GRUB_USB_DESCRIPTOR_HUB << 8) | 0,
+			      0, sizeof (hubdesc), (char *) &hubdesc);
+  if (err)
+    return;
+    
+  /* Iterate over the Hub ports.  */
+  for (i = 1; i <= hubdesc.portcnt; i++)
+    {
+      grub_uint32_t status;
+
+      /* Get the port status.  */
+      err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+					| GRUB_USB_REQTYPE_CLASS
+					| GRUB_USB_REQTYPE_TARGET_OTHER),
+				  GRUB_USB_REQ_GET_STATUS,
+				  0, i, sizeof (status), (char *) &status);
+      /* Just ignore the device if the Hub does not report the
+	 status.  */
+      if (err)
+	continue;
+      	    
+      /* Connected and status of connection changed ? */
+      if ((status & GRUB_USB_HUB_STATUS_CONNECTED)
+          && (status & GRUB_USB_HUB_STATUS_C_CONNECTED))
+	{
+	  grub_usb_speed_t speed;
+
+	  /* Determine the device speed.  */
+	  if (status & GRUB_USB_HUB_STATUS_LOWSPEED)
+	    speed = GRUB_USB_SPEED_LOW;
+	  else
+	    {
+	      if (status & GRUB_USB_HUB_STATUS_HIGHSPEED)
+		speed = GRUB_USB_SPEED_HIGH;
+	      else
+		speed = GRUB_USB_SPEED_FULL;
+	    }
+
+	  /* A device is actually connected to this port.
+	   * Now do reset of port. */
+	  err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+					    | GRUB_USB_REQTYPE_CLASS
+					    | GRUB_USB_REQTYPE_TARGET_OTHER),
+				      GRUB_USB_REQ_SET_FEATURE,
+				      GRUB_USB_HUB_FEATURE_PORT_RESET,
+				      i, 0, 0);
+	  /* If the Hub does not cooperate for this port, just skip
+	     the port.  */
+	  if (err)
+	    continue;
+
+          /* Wait for reset procedure done */
+          timeout = grub_get_time_ms () + 1000;
+          do
+            {
+              /* Get the port status.  */
+              err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+	   	 			        | GRUB_USB_REQTYPE_CLASS
+					        | GRUB_USB_REQTYPE_TARGET_OTHER),
+				          GRUB_USB_REQ_GET_STATUS,
+				          0, i, sizeof (status), (char *) &status);
+            }
+          while (!err &&
+                 !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) &&
+                 (grub_get_time_ms() < timeout) );
+          if (err || !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) )
+            continue;
+
+          /* Do reset of connection change bit */
+          err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+                                            | GRUB_USB_REQTYPE_CLASS
+                                            | GRUB_USB_REQTYPE_TARGET_OTHER),
+                                      GRUB_USB_REQ_CLEAR_FEATURE,
+				      GRUB_USB_HUB_FEATURE_C_CONNECTED,
+				      i, 0, 0);
+          /* Just ignore the device if the Hub reports some error */
+          if (err)
+	     continue;
+
+	  /* Add the device and assign a device address to it.  */
+	  next_dev = grub_usb_hub_add_dev (&dev->controller, speed);
+          if (! next_dev)
+            continue;
+
+          /* If the device is a Hub, scan it for more devices.  */
+          if (next_dev->descdev.class == 0x09)
+            grub_usb_add_hub (next_dev);
+	}
+    }
+
+  return;
+}
+
 void
 grub_usb_poll_devices (void)
 {
   struct grub_usb_hub *hub;
+  int i;
 
   for (hub = hubs; hub; hub = hub->next)
     {
-      int i;
+      int changed=0;
       /* Do we have to recheck number of ports?  */
+      /* No, it should be never changed, it should be constant. */
       for (i = 0; i < hub->nports; i++)
 	{
 	  grub_usb_speed_t speed;
 
-	  speed = hub->controller->dev->detect_dev (hub->controller, i);
+	  speed = hub->controller->dev->detect_dev (hub->controller, i,
+	                                            &changed);
 
-	  if (speed == hub->speed[i])
-	    continue;
+	  if (speed != GRUB_USB_SPEED_NONE)
+	    {
+              if (changed)
+                attach_root_port (hub->controller, i, speed);
+	    }
 
-	  if (hub->speed[i] == GRUB_USB_SPEED_NONE
-	      && speed != GRUB_USB_SPEED_NONE)
-	    attach_root_port (hub->controller, i, speed);
+          /* XXX: There should be also handling
+           * of disconnected devices. */
+           
 	  hub->speed[i] = speed;
 	}
     }
+
+  /* We should check changes of non-root hubs too. */
+  for (i = 0; i < GRUB_USBHUB_MAX_DEVICES; i++)
+    {
+      grub_usb_device_t dev = grub_usb_devs[i];
+
+      if (dev && dev->descdev.class == 0x09)
+	{
+          poll_nonroot_hub (dev);
+	}
+    }
+
 }
 
 int
@@ -304,7 +466,7 @@
 {
   int i;
 
-  for (i = 0; i < 128; i++)
+  for (i = 0; i < GRUB_USBHUB_MAX_DEVICES; i++)
     {
       if (grub_usb_devs[i])
 	{
diff -urB ./my-repo/usb/include/grub/usb.h ./usb/include/grub/usb.h
--- ./my-repo/usb/include/grub/usb.h	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/include/grub/usb.h	2010-07-07 00:01:43.000000000 +0200
@@ -104,7 +104,7 @@
   grub_err_t (*portstatus) (grub_usb_controller_t dev, unsigned int port,
 			    unsigned int enable);
 
-  grub_usb_speed_t (*detect_dev) (grub_usb_controller_t dev, int port);
+  grub_usb_speed_t (*detect_dev) (grub_usb_controller_t dev, int port, int *changed);
 
   /* The next host controller.  */
   struct grub_usb_controller_dev *next;
@@ -229,4 +229,6 @@
 
 void grub_usb_poll_devices (void);
 
+void grub_usb_device_attach (grub_usb_device_t dev);
+
 #endif /* GRUB_USB_H */
diff -urB ./my-repo/usb/include/grub/usbtrans.h ./usb/include/grub/usbtrans.h
--- ./my-repo/usb/include/grub/usbtrans.h	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/include/grub/usbtrans.h	2010-07-06 22:04:26.000000000 +0200
@@ -93,10 +93,12 @@
 
 #define GRUB_USB_HUB_FEATURE_PORT_RESET   0x04
 #define GRUB_USB_HUB_FEATURE_PORT_POWER   0x08
+#define GRUB_USB_HUB_FEATURE_C_CONNECTED  0x10
 
 #define GRUB_USB_HUB_STATUS_CONNECTED	 (1 << 0)
 #define GRUB_USB_HUB_STATUS_LOWSPEED	 (1 << 9)
 #define GRUB_USB_HUB_STATUS_HIGHSPEED	 (1 << 10)
+#define GRUB_USB_HUB_STATUS_C_CONNECTED  (1 << 16)
 #define GRUB_USB_HUB_STATUS_C_PORT_RESET (1 << 20)
 
 struct grub_usb_packet_setup

Re: [Patch] USB hot-plugging support - usb branch

[Aleš Nesrsta]

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

Hi,

attached new patch includes improved hot-plug support.
It is also committed into usb branch (rev. 2428).
It should work now on UHCI, OHCI and also on non-root hubs.

Could somebody test it ?
(New plugged device should be accessible after "ls" command.
Disconnected devices remain listed but they are not working.)


Changed files:
bus/usb/ohci.c
bus/usb/uhci.c
bus/usb/usb.c
bus/usb/usbhub.c
include/grub/usb.h
include/grub/usbtrans.h


To do:

- hot-plugging on OHCI is limited because of limited number of
"statically" allocated EDs:
  -- number of EDs could be increased in ohci.c
  -- de-allocation of EDs should be added in ohci.c

- when device disconnect is detected on some port, related USB device
structures should be de-allocated
  -- if disconnected device is hub, it is necessary to de-allocate also
all devices possibly connected to this hub.
  -- to have chance to do it, it is necessary to implement some
structures to know which USB device is connected to which port of which
UHCI/OHCI device and non-root hub
  -- to avoid problem in disk cache or somewhere else, it will be maybe
better to keep de-allocated device numbers not usable in future even if
related device is disconnected (i.e. if device "usb0" is disconnected,
no newly connected device can be named as "usb0". "ls" command should
not display disconnected devices.)

Regards
Ales


[-- Attachment #2: usb_patch_hotplug_100708_0 --]
[-- Type: text/x-patch, Size: 19679 bytes --]

diff -urB ./my-repo/usb/bus/usb/ohci.c ./usb/bus/usb/ohci.c
--- ./my-repo/usb/bus/usb/ohci.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/ohci.c	2010-07-08 21:18:37.000000000 +0200
@@ -148,6 +148,7 @@
 #define GRUB_OHCI_REG_CONTROL_BULK_ENABLE (1 << 5)
 #define GRUB_OHCI_REG_CONTROL_CONTROL_ENABLE (1 << 4)
 
+#define GRUB_OHCI_RESET_CONNECT_CHANGE (1 << 16)
 #define GRUB_OHCI_CTRL_EDS 16
 #define GRUB_OHCI_BULK_EDS 16
 #define GRUB_OHCI_TDS 256
@@ -420,23 +421,8 @@
                        (grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBA)
                         & ~GRUB_OHCI_RHUB_PORT_POWER_MASK)
                        | GRUB_OHCI_RHUB_PORT_ALL_POWERED);
-  /* Wait for stable power (100ms) and stable attachment (100ms) */
-  /* I.e. minimum wait time should be probably 200ms. */
-  /* We assume that device is attached when ohci is loaded. */
-  /* Some devices take long time to power-on or indicate attach. */
-  /* Here is some experimental value which should probably mostly work. */
-  /* Cameras with manual USB mode selection and maybe some other similar
-   * devices will not work in some cases - they are repowered during
-   * ownership change and then they are starting slowly and mostly they
-   * are wanting select proper mode again...
-   * The same situation can be on computers where BIOS not set-up OHCI
-   * to be at least powered USB bus (maybe it is Yeelong case...?)
-   * Possible workaround could be for example some prompt
-   * for user with confirmation of proper USB device connection.
-   * Another workaround - "rmmod usbms", "rmmod ohci", proper start
-   * and configuration of USB device and then "insmod ohci"
-   * and "insmod usbms". */
-  grub_millisleep (500);	
+  /* Now we have hot-plugging, we need to wait for stable power only */
+  grub_millisleep (100);
 
   /* Link to ohci now that initialisation is successful.  */
   o->next = ohci;
@@ -998,6 +984,15 @@
             }
         }
 
+      /* Even if we have "good" OHCI, in some cases
+       * tderr_phys can be zero, check it */
+      else if ( !tderr_phys )
+        { /* Retired TD with error should be previous TD to ED->td_head */
+          tderr_phys = GRUB_OHCI_TD_PHYS2VIRT (o,
+                         grub_le_to_cpu32 ( ed_virt->td_head) & ~0xf )
+                       ->prev_td_phys;
+        }
+
       /* Prepare pointer to last processed TD and get error code */
       tderr_virt = GRUB_OHCI_TD_PHYS2VIRT (o, tderr_phys);
       /* Set index of last processed TD */
@@ -1095,8 +1090,6 @@
 	  break;
 	}
 
-      /* Set empty ED - set HEAD = TAIL = last (not processed) TD */
-      ed_virt->td_head = ed_virt->td_tail & ~0xf; 
     }
         
   else if (err_unrec)      
@@ -1117,7 +1110,6 @@
       grub_dprintf ("ohci", "Unrecoverable error - OHCI reset\n");
 
       /* Misc. resets. */
-      ed_virt->td_head = ed_virt->td_tail & ~0xf; /* Set empty ED - set HEAD = TAIL = last (not processed) TD */
       o->hcca->donehead = 0;
       grub_ohci_writereg32 (o, GRUB_OHCI_REG_INTSTATUS, 0x7f); /* Clears everything */
       grub_ohci_writereg32 (o, GRUB_OHCI_REG_CONTROLHEAD, o->ed_ctrl_addr);
@@ -1160,10 +1152,12 @@
       else
         transfer->last_trans = -1;
 
-      /* Set empty ED - set HEAD = TAIL = last (not processed) TD */
-      ed_virt->td_head = ed_virt->td_tail & ~0xf; 
     }
 
+    /* Set empty ED - set HEAD = TAIL = last (not processed) TD */
+    ed_virt->td_head = grub_cpu_to_le32 ( grub_le_to_cpu32 (
+                                            ed_virt->td_tail) & ~0xf); 
+
   /* At this point always should be:
    * ED has skip bit set and halted or empty or after next SOF,
    *    i.e. it is safe to free all TDs except last not processed
@@ -1198,10 +1192,28 @@
 		      unsigned int port, unsigned int enable)
 {
    struct grub_ohci *o = (struct grub_ohci *) dev->data;
+   grub_uint64_t endtime;
 
    grub_dprintf ("ohci", "begin of portstatus=0x%02x\n",
                  grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port));
 
+   if (!enable) /* We don't need reset port */
+     {
+       /* Disable the port and wait for it. */
+       grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
+                             GRUB_OHCI_CLEAR_PORT_ENABLE);
+       endtime = grub_get_time_ms () + 1000;
+       while ((grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port)
+               & (1 << 1)))
+         if (grub_get_time_ms () > endtime)
+           return grub_error (GRUB_ERR_IO, "OHCI Timed out - disable");
+
+       grub_dprintf ("ohci", "end of portstatus=0x%02x\n",
+         grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port));
+       return GRUB_ERR_NONE;
+     }
+     
+   /* Reset the port */
    grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
 			 GRUB_OHCI_SET_PORT_RESET);
    grub_millisleep (50); /* For root hub should be nominaly 50ms */
@@ -1211,14 +1223,21 @@
 			 GRUB_OHCI_SET_PORT_RESET_STATUS_CHANGE);
    grub_millisleep (10);
 
-   if (enable)
-     grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
-			   GRUB_OHCI_SET_PORT_ENABLE);
-   else
-     grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
-			   GRUB_OHCI_CLEAR_PORT_ENABLE);
+   /* Enable the port and wait for it. */
+   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
+                         GRUB_OHCI_SET_PORT_ENABLE);
+   endtime = grub_get_time_ms () + 1000;
+   while (! (grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port)
+           & (1 << 1)))
+     if (grub_get_time_ms () > endtime)
+       return grub_error (GRUB_ERR_IO, "OHCI Timed out - enable");
+
    grub_millisleep (10);
 
+   /* Reset bit Connect Status Change */
+   grub_ohci_writereg32 (o, GRUB_OHCI_REG_RHUBPORT + port,
+                         GRUB_OHCI_RESET_CONNECT_CHANGE);
+
    grub_dprintf ("ohci", "end of portstatus=0x%02x\n",
 		 grub_ohci_readreg32 (o, GRUB_OHCI_REG_RHUBPORT + port));
  
@@ -1226,7 +1245,7 @@
 }
 
 static grub_usb_speed_t
-grub_ohci_detect_dev (grub_usb_controller_t dev, int port)
+grub_ohci_detect_dev (grub_usb_controller_t dev, int port, int *changed)
 {
    struct grub_ohci *o = (struct grub_ohci *) dev->data;
    grub_uint32_t status;
@@ -1235,6 +1254,9 @@
 
    grub_dprintf ("ohci", "detect_dev status=0x%02x\n", status);
 
+  /* Connect Status Change bit - it detects change of connection */
+  *changed = ((status & GRUB_OHCI_RESET_CONNECT_CHANGE) != 0);
+
    if (! (status & 1))
      return GRUB_USB_SPEED_NONE;
    else if (status & (1 << 9))
@@ -1253,7 +1275,6 @@
 
   grub_dprintf ("ohci", "root hub ports=%d\n", portinfo & 0xFF);
 
-  /* The root hub has exactly two ports.  */
   return portinfo & 0xFF;
 }
 
diff -urB ./my-repo/usb/bus/usb/uhci.c ./usb/bus/usb/uhci.c
--- ./my-repo/usb/bus/usb/uhci.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/uhci.c	2010-07-06 22:24:31.000000000 +0200
@@ -644,10 +644,14 @@
   grub_dprintf ("uhci", "waiting for the port to be enabled\n");
 
   endtime = grub_get_time_ms () + 1000;
-  while (! (grub_uhci_readreg16 (u, reg) & (1 << 2)))
+  while (! ((status = grub_uhci_readreg16 (u, reg)) & (1 << 2)))
     if (grub_get_time_ms () > endtime)
       return grub_error (GRUB_ERR_IO, "UHCI Timed out");
 
+  /* Reset bit Connect Status Change */
+  grub_uhci_writereg16 (u, reg, status | (1 << 1));
+
+  /* Read final port status */
   status = grub_uhci_readreg16 (u, reg);
   grub_dprintf ("uhci", ">3detect=0x%02x\n", status);
 
@@ -656,7 +660,7 @@
 }
 
 static grub_usb_speed_t
-grub_uhci_detect_dev (grub_usb_controller_t dev, int port)
+grub_uhci_detect_dev (grub_usb_controller_t dev, int port, int *changed)
 {
   struct grub_uhci *u = (struct grub_uhci *) dev->data;
   int reg;
@@ -676,6 +680,9 @@
 
   grub_dprintf ("uhci", "detect=0x%02x port=%d\n", status, port);
 
+  /* Connect Status Change bit - it detects change of connection */
+  *changed = ((status & (1 << 1)) != 0);
+    
   if (! (status & 1))
     return GRUB_USB_SPEED_NONE;
   else if (status & (1 << 8))
diff -urB ./my-repo/usb/bus/usb/usb.c ./usb/bus/usb/usb.c
--- ./my-repo/usb/bus/usb/usb.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/usb.c	2010-07-07 00:23:38.000000000 +0200
@@ -225,6 +225,20 @@
 	}
     }
 
+  return GRUB_USB_ERR_NONE;
+
+ fail:
+
+  for (i = 0; i < 8; i++)
+    grub_free (dev->config[i].descconf);
+
+  return err;
+}
+
+void grub_usb_device_attach (grub_usb_device_t dev)
+{
+  int i;
+  
   /* XXX: Just check configuration 0 for now.  */
   for (i = 0; i < dev->config[0].descconf->numif; i++)
     {
@@ -243,15 +257,6 @@
 	if (interf->class == desc->class && desc->hook (dev, 0, i))
 	  dev->config[0].interf[i].attached = 1;
     }
-
-  return GRUB_USB_ERR_NONE;
-
- fail:
-
-  for (i = 0; i < 8; i++)
-    grub_free (dev->config[i].descconf);
-
-  return err;
 }
 
 void
diff -urB ./my-repo/usb/bus/usb/usbhub.c ./usb/bus/usb/usbhub.c
--- ./my-repo/usb/bus/usb/usbhub.c	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/bus/usb/usbhub.c	2010-07-08 00:28:15.000000000 +0200
@@ -23,8 +23,10 @@
 #include <grub/misc.h>
 #include <grub/time.h>
 
+#define GRUB_USBHUB_MAX_DEVICES 128
+
 /* USB Supports 127 devices, with device 0 as special case.  */
-static struct grub_usb_device *grub_usb_devs[128];
+static struct grub_usb_device *grub_usb_devs[GRUB_USBHUB_MAX_DEVICES];
 
 struct grub_usb_hub
 {
@@ -44,6 +46,7 @@
 {
   grub_usb_device_t dev;
   int i;
+  grub_usb_err_t err;
 
   dev = grub_zalloc (sizeof (struct grub_usb_device));
   if (! dev)
@@ -52,31 +55,51 @@
   dev->controller = *controller;
   dev->speed = speed;
 
-  grub_usb_device_initialize (dev);
+  err = grub_usb_device_initialize (dev);
+  if (err)
+    {
+      grub_free (dev);
+      return NULL;
+    }
 
   /* Assign a new address to the device.  */
-  for (i = 1; i < 128; i++)
+  for (i = 1; i < GRUB_USBHUB_MAX_DEVICES; i++)
     {
       if (! grub_usb_devs[i])
 	break;
     }
-  if (i == 128)
+  if (i == GRUB_USBHUB_MAX_DEVICES)
     {
       grub_error (GRUB_ERR_IO, "can't assign address to USB device");
+      for (i = 0; i < 8; i++)
+        grub_free (dev->config[i].descconf);
+      grub_free (dev);
       return NULL;
     }
 
-  grub_usb_control_msg (dev,
-			(GRUB_USB_REQTYPE_OUT
-			 | GRUB_USB_REQTYPE_STANDARD
-			 | GRUB_USB_REQTYPE_TARGET_DEV),
-			GRUB_USB_REQ_SET_ADDRESS,
-			i, 0, 0, NULL);
+  err = grub_usb_control_msg (dev,
+			      (GRUB_USB_REQTYPE_OUT
+			       | GRUB_USB_REQTYPE_STANDARD
+			       | GRUB_USB_REQTYPE_TARGET_DEV),
+			      GRUB_USB_REQ_SET_ADDRESS,
+			      i, 0, 0, NULL);
+  if (err)
+    {
+      for (i = 0; i < 8; i++)
+        grub_free (dev->config[i].descconf);
+      grub_free (dev);
+      return NULL;
+    }
 
   dev->addr = i;
   dev->initialized = 1;
   grub_usb_devs[i] = dev;
 
+  /* Wait "recovery interval", spec. says 2ms */
+  grub_millisleep (2);
+  
+  grub_usb_device_attach (dev);
+  
   return dev;
 }
 
@@ -144,7 +167,7 @@
       if (err)
 	continue;
       grub_dprintf ("usb", "Hub port %d status: 0x%02x\n", i, status);
-      	    
+
       /* If connected, reset and enable the port.  */
       if (status & GRUB_USB_HUB_STATUS_CONNECTED)
 	{
@@ -191,7 +214,22 @@
                  (grub_get_time_ms() < timeout) );
           if (err || !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) )
             continue;
+
+          /* Wait a recovery time after reset, spec. says 10ms */
+          grub_millisleep (10);
    
+          /* Do reset of connection change bit */
+          err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+                                            | GRUB_USB_REQTYPE_CLASS
+                                            | GRUB_USB_REQTYPE_TARGET_OTHER),
+                                      GRUB_USB_REQ_CLEAR_FEATURE,
+				      GRUB_USB_HUB_FEATURE_C_CONNECTED,
+				      i, 0, 0);
+          /* Just ignore the device if the Hub reports some error */
+          if (err)
+	     continue;
+          grub_dprintf ("usb", "Hub port - cleared connection change\n");
+
 	  /* Add the device and assign a device address to it.  */
           grub_dprintf ("usb", "Call hub_add_dev - port %d\n", i);
 	  next_dev = grub_usb_hub_add_dev (&dev->controller, speed);
@@ -239,6 +277,7 @@
 {
   int i;
   struct grub_usb_hub *hub;
+  int changed=0;
 
   hub = grub_malloc (sizeof (*hub));
   if (!hub)
@@ -248,7 +287,10 @@
   hubs = hub;
   hub->controller = grub_malloc (sizeof (*controller));
   if (!hub->controller)
-    return GRUB_USB_ERR_INTERNAL;
+    {
+      grub_free (hub);
+      return GRUB_USB_ERR_INTERNAL;
+    }
 
   grub_memcpy (hub->controller, controller, sizeof (*controller));
   hub->dev = 0;
@@ -258,13 +300,15 @@
   hub->speed = grub_malloc (sizeof (hub->speed[0]) * hub->nports);
   if (!hub->speed)
     {
+      grub_free (hub->controller);
       grub_free (hub);
       return GRUB_USB_ERR_INTERNAL;
     }
 
   for (i = 0; i < hub->nports; i++)
     {
-      hub->speed[i] = controller->dev->detect_dev (hub->controller, i);
+      hub->speed[i] = controller->dev->detect_dev (hub->controller, i,
+                                                   &changed);
 
       if (hub->speed[i] != GRUB_USB_SPEED_NONE)
 	attach_root_port (hub->controller, i, hub->speed[i]);
@@ -273,30 +317,157 @@
   return GRUB_USB_ERR_NONE;
 }
 
+static void
+poll_nonroot_hub (grub_usb_device_t dev)
+{
+  struct grub_usb_usb_hubdesc hubdesc;
+  grub_err_t err;
+  int i;
+  grub_uint64_t timeout;
+  grub_usb_device_t next_dev;
+  
+  err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+	  		            | GRUB_USB_REQTYPE_CLASS
+			            | GRUB_USB_REQTYPE_TARGET_DEV),
+                              GRUB_USB_REQ_GET_DESCRIPTOR,
+			      (GRUB_USB_DESCRIPTOR_HUB << 8) | 0,
+			      0, sizeof (hubdesc), (char *) &hubdesc);
+  if (err)
+    return;
+    
+  /* Iterate over the Hub ports.  */
+  for (i = 1; i <= hubdesc.portcnt; i++)
+    {
+      grub_uint32_t status;
+
+      /* Get the port status.  */
+      err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+					| GRUB_USB_REQTYPE_CLASS
+					| GRUB_USB_REQTYPE_TARGET_OTHER),
+				  GRUB_USB_REQ_GET_STATUS,
+				  0, i, sizeof (status), (char *) &status);
+      /* Just ignore the device if the Hub does not report the
+	 status.  */
+      if (err)
+	continue;
+      	    
+      /* Connected and status of connection changed ? */
+      if ((status & GRUB_USB_HUB_STATUS_CONNECTED)
+          && (status & GRUB_USB_HUB_STATUS_C_CONNECTED))
+	{
+	  grub_usb_speed_t speed;
+
+	  /* Determine the device speed.  */
+	  if (status & GRUB_USB_HUB_STATUS_LOWSPEED)
+	    speed = GRUB_USB_SPEED_LOW;
+	  else
+	    {
+	      if (status & GRUB_USB_HUB_STATUS_HIGHSPEED)
+		speed = GRUB_USB_SPEED_HIGH;
+	      else
+		speed = GRUB_USB_SPEED_FULL;
+	    }
+
+	  /* A device is actually connected to this port.
+	   * Now do reset of port. */
+	  err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+					    | GRUB_USB_REQTYPE_CLASS
+					    | GRUB_USB_REQTYPE_TARGET_OTHER),
+				      GRUB_USB_REQ_SET_FEATURE,
+				      GRUB_USB_HUB_FEATURE_PORT_RESET,
+				      i, 0, 0);
+	  /* If the Hub does not cooperate for this port, just skip
+	     the port.  */
+	  if (err)
+	    continue;
+
+          /* Wait for reset procedure done */
+          timeout = grub_get_time_ms () + 1000;
+          do
+            {
+              /* Get the port status.  */
+              err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_IN
+	   	 			        | GRUB_USB_REQTYPE_CLASS
+					        | GRUB_USB_REQTYPE_TARGET_OTHER),
+				          GRUB_USB_REQ_GET_STATUS,
+				          0, i, sizeof (status), (char *) &status);
+            }
+          while (!err &&
+                 !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) &&
+                 (grub_get_time_ms() < timeout) );
+          if (err || !(status & GRUB_USB_HUB_STATUS_C_PORT_RESET) )
+            continue;
+
+          /* Wait a recovery time after reset, spec. says 10ms */
+          grub_millisleep (10);
+
+          /* Do reset of connection change bit */
+          err = grub_usb_control_msg (dev, (GRUB_USB_REQTYPE_OUT
+                                            | GRUB_USB_REQTYPE_CLASS
+                                            | GRUB_USB_REQTYPE_TARGET_OTHER),
+                                      GRUB_USB_REQ_CLEAR_FEATURE,
+				      GRUB_USB_HUB_FEATURE_C_CONNECTED,
+				      i, 0, 0);
+          /* Just ignore the device if the Hub reports some error */
+          if (err)
+	     continue;
+
+	  /* Add the device and assign a device address to it.  */
+	  next_dev = grub_usb_hub_add_dev (&dev->controller, speed);
+          if (! next_dev)
+            continue;
+
+          /* If the device is a Hub, scan it for more devices.  */
+          if (next_dev->descdev.class == 0x09)
+            grub_usb_add_hub (next_dev);
+	}
+    }
+
+  return;
+}
+
 void
 grub_usb_poll_devices (void)
 {
   struct grub_usb_hub *hub;
+  int i;
 
   for (hub = hubs; hub; hub = hub->next)
     {
-      int i;
+      int changed=0;
       /* Do we have to recheck number of ports?  */
+      /* No, it should be never changed, it should be constant. */
       for (i = 0; i < hub->nports; i++)
 	{
 	  grub_usb_speed_t speed;
 
-	  speed = hub->controller->dev->detect_dev (hub->controller, i);
+	  speed = hub->controller->dev->detect_dev (hub->controller, i,
+	                                            &changed);
 
-	  if (speed == hub->speed[i])
-	    continue;
+	  if (speed != GRUB_USB_SPEED_NONE)
+	    {
+              if (changed)
+                attach_root_port (hub->controller, i, speed);
+	    }
 
-	  if (hub->speed[i] == GRUB_USB_SPEED_NONE
-	      && speed != GRUB_USB_SPEED_NONE)
-	    attach_root_port (hub->controller, i, speed);
+          /* XXX: There should be also handling
+           * of disconnected devices. */
+           
 	  hub->speed[i] = speed;
 	}
     }
+
+  /* We should check changes of non-root hubs too. */
+  for (i = 0; i < GRUB_USBHUB_MAX_DEVICES; i++)
+    {
+      grub_usb_device_t dev = grub_usb_devs[i];
+
+      if (dev && dev->descdev.class == 0x09)
+	{
+          poll_nonroot_hub (dev);
+	}
+    }
+
 }
 
 int
@@ -304,7 +475,7 @@
 {
   int i;
 
-  for (i = 0; i < 128; i++)
+  for (i = 0; i < GRUB_USBHUB_MAX_DEVICES; i++)
     {
       if (grub_usb_devs[i])
 	{
diff -urB ./my-repo/usb/include/grub/usb.h ./usb/include/grub/usb.h
--- ./my-repo/usb/include/grub/usb.h	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/include/grub/usb.h	2010-07-07 00:01:43.000000000 +0200
@@ -104,7 +104,7 @@
   grub_err_t (*portstatus) (grub_usb_controller_t dev, unsigned int port,
 			    unsigned int enable);
 
-  grub_usb_speed_t (*detect_dev) (grub_usb_controller_t dev, int port);
+  grub_usb_speed_t (*detect_dev) (grub_usb_controller_t dev, int port, int *changed);
 
   /* The next host controller.  */
   struct grub_usb_controller_dev *next;
@@ -229,4 +229,6 @@
 
 void grub_usb_poll_devices (void);
 
+void grub_usb_device_attach (grub_usb_device_t dev);
+
 #endif /* GRUB_USB_H */
diff -urB ./my-repo/usb/include/grub/usbtrans.h ./usb/include/grub/usbtrans.h
--- ./my-repo/usb/include/grub/usbtrans.h	2010-07-06 19:48:28.000000000 +0200
+++ ./usb/include/grub/usbtrans.h	2010-07-06 22:04:26.000000000 +0200
@@ -93,10 +93,12 @@
 
 #define GRUB_USB_HUB_FEATURE_PORT_RESET   0x04
 #define GRUB_USB_HUB_FEATURE_PORT_POWER   0x08
+#define GRUB_USB_HUB_FEATURE_C_CONNECTED  0x10
 
 #define GRUB_USB_HUB_STATUS_CONNECTED	 (1 << 0)
 #define GRUB_USB_HUB_STATUS_LOWSPEED	 (1 << 9)
 #define GRUB_USB_HUB_STATUS_HIGHSPEED	 (1 << 10)
+#define GRUB_USB_HUB_STATUS_C_CONNECTED  (1 << 16)
 #define GRUB_USB_HUB_STATUS_C_PORT_RESET (1 << 20)
 
 struct grub_usb_packet_setup

Re: [Patch] USB hot-plugging support - usb branch

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On 07/08/2010 11:11 PM, Aleš Nesrsta wrote:
> Hi,
>
> attached new patch includes improved hot-plug support.
> It is also committed into usb branch (rev. 2428).
> It should work now on UHCI, OHCI and also on non-root hubs.
>
> Could somebody test it ?
>   
Successfully tested on Yeeloong and merged into trunk.
> (New plugged device should be accessible after "ls" command.
> Disconnected devices remain listed but they are not working.)
>
>
> Changed files:
> bus/usb/ohci.c
> bus/usb/uhci.c
> bus/usb/usb.c
> bus/usb/usbhub.c
> include/grub/usb.h
> include/grub/usbtrans.h
>
>
> To do:
>
> - hot-plugging on OHCI is limited because of limited number of
> "statically" allocated EDs:
>   -- number of EDs could be increased in ohci.c
>   -- de-allocation of EDs should be added in ohci.c
>
>   
You can have at most 255 devices on one controller. The easiest way is
to allocate enough EDs for 255 devices
> - when device disconnect is detected on some port, related USB device
> structures should be de-allocated
>   -- if disconnected device is hub, it is necessary to de-allocate also
> all devices possibly connected to this hub.
>   -- to have chance to do it, it is necessary to implement some
> structures to know which USB device is connected to which port of which
> UHCI/OHCI device and non-root hub
>   -- to avoid problem in disk cache or somewhere else, it will be maybe
> better to keep de-allocated device numbers not usable in future even if
> related device is disconnected (i.e. if device "usb0" is disconnected,
> no newly connected device can be named as "usb0". "ls" command should
> not display disconnected devices.)
>
>   
Agreed.
> Regards
> Ales
>
>   
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] USB hot-plugging support - usb branch

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

On 07/07/2010 01:06 AM, Aleš Nesrsta wrote:
> Hi,
>
> I little bit changed "hot-plugging" support added into "usb" branch by
> Vladimir - see included patch. For the first look it maybe works on
> UHCI, I did not test it on OHCI yet...
>
> Basic info to this patch:
>   

> 2.
> I am not using original comparing of "hub->speed[i]" and "speed" to
> detect connection status change (but I left the "hub" structure in code,
> it can be maybe useful in future). I am using built-in functions of
> OHCI/UHCI controllers (available via root hub port register) which
> should be more reliable and are more systematic and closed to
> specifications.
>
>   
I thought of this at first too but haven't found such register on UHCI
and so decided to do it the same way on all systems. Since you've found
the needed register on UHCI I agree with your approach.
> 3.
> There is missing handling of disconnected devices. For this we will need
> to know on which port which device is attached - it is currently not
> known
But it can be easily stored in usbdevice structure.
>  - and we need to know it "recursively" because devices can be
> connected indirectly via non-root hubs.
> On this part I probably will not work in near future (currently I have
> no simple and suitable idea how to do it).
>
> Regards
> Ales
>   
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>   


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]

Re: [Patch] USB hot-plugging support - usb branch

[Aleš Nesrsta]

Hi Vladimir,

Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > ...
> "statically" allocated EDs:
> >   -- number of EDs could be increased in ohci.c
> >   -- de-allocation of EDs should be added in ohci.c
> >
> >   
> You can have at most 255 devices on one controller. The easiest way is
> to allocate enough EDs for 255 devices

Yes, it is one of possible and simplest ways... In first try we can
increase number of EDs to approx. 255 + 1 control and 2*255 bulk EDs per
OHCI/UHCI controller. (We need one control ED and two bulk EDs per one
mass storage device plus one control zero ED for new connected device.)
I.e. currently we can simply change in ohci.c:
#define GRUB_OHCI_CTRL_EDS 256
#define GRUB_OHCI_BULK_EDS 510
and it should be enough...

But it will be better to finally implement also some ED de-allocating in
OHCI. I want to try to implement device-port information into usbdevice
structure and together with it add de-allocating of OHCI EDs - see
below.

> ...
> > There is missing handling of disconnected devices. For this we will
> need
> > to know on which port which device is attached - it is currently not
> > known
> But it can be easily stored in usbdevice structure.

I have similar idea, do you mean this structure ?

usb.h: struct grub_usb_device

(I am thinking how to simply implement recursive structure of USB bus -
root hub, hubs, devices on hubs at any level... Exists some similar
problems/structures somewhere in GRUB (to use the same or similar
implementation or, at least, to be inspired) ?)

But as I wrote in another today's e-mail to jonatan perry, I will
probably too busy in next weeks/months, so it may take some longer
time... ( BTW - What do you mean about EHCI driver and its priority in
GRUB development ? But it is probably theme for another mail thread...)

Regards
Ales

Re: [Patch] USB hot-plugging support - usb branch

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2181 bytes --]

On 07/24/2010 09:15 PM, Aleš Nesrsta wrote:
> Yes, it is one of possible and simplest ways... In first try we can
> increase number of EDs to approx. 255 + 1 control and 2*255 bulk EDs per
> OHCI/UHCI controller. (We need one control ED and two bulk EDs per one
> mass storage device plus one control zero ED for new connected device.)
> I.e. currently we can simply change in ohci.c:
> #define GRUB_OHCI_CTRL_EDS 256
> #define GRUB_OHCI_BULK_EDS 510
> and it should be enough...
>
> But it will be better to finally implement also some ED de-allocating in
> OHCI. I want to try to implement device-port information into usbdevice
> structure and together with it add de-allocating of OHCI EDs - see
> below.
>
>   
Allocating and deallocating is relatively expensive to maintain. Usually
the benefit is far greater. In this case it's only about saving few KB
of memory. It's simply not worth it. Don't bother about it.
>> ...
>>     
>>> There is missing handling of disconnected devices. For this we will
>>>       
>> need
>>     
>>> to know on which port which device is attached - it is currently not
>>> known
>>>       
>> But it can be easily stored in usbdevice structure.
>>     
> I have similar idea, do you mean this structure ?
>
> usb.h: struct grub_usb_device
>
> (I am thinking how to simply implement recursive structure of USB bus -
> root hub, hubs, devices on hubs at any level... Exists some similar
> problems/structures somewhere in GRUB (to use the same or similar
> implementation or, at least, to be inspired) ?)
>
>   
I would use linked lists.
> But as I wrote in another today's e-mail to jonatan perry, I will
> probably too busy in next weeks/months, so it may take some longer
> time... ( BTW - What do you mean about EHCI driver and its priority in
> GRUB development ? But it is probably theme for another mail thread...)
>
>   
EHCI is important.
> Regards
> Ales
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
>
>   


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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 294 bytes --]