USB device not enumerated by iPXE USB stack

[eugenio]

Hi all,

I am working on a customization of iPXE that reads the device ID of the USB devices attached to the system when iPXE runs.

I have found that the USB stack of iPXE does not enumerate a device of one of out customers. In my tests the device is attached to one port of the root hub of an EHCI host controller.

The failure happens while the port is being enabled (function ehci_root_enable). The reset of the port is successfully completed (bit 8 of the PORTSC register of the port is set to 0), but the port is not enabled (bit 2 of the PORTSC register of the port is set 0 and bit 0 of PORTSC register of the port is set to 1).

Any help will be wellcomed.

Thanks in advance and excuse my English.

Eugenio

[NiKiZe]

Does building with debug give anything helpful?

This might be something where the mailing list would be seen by more people that understand the USB internals

[eugenio]

(2016-09-14 17:48)NiKiZe Wrote:  Does building with debug give anything helpful?

This might be something where the mailing list would be seen by more people that understand the USB internals

Hi NiKiZe,

Yes, I am building iPXE with debug information.
On the same machine where iPXE is not able to enumerate the device, Windows 7 enumerates it with no errors. If I plug the device into another machine with Ubuntu 14.04.2 LTS it is also enumerated with no errors.

Regards,

Eugenio

[eugenio]

Hi all,

The USB enumeration process failed because the device failed enumeration as a USB 2.0 device, so the iPXE EHCI USB stack handed ownership of the USB port to the iPXE UHCI USB stack, but the enumeration also failed as a USB 1.x device.

So I took a look at the UHCI source code of iPXE. I think that I have found a bug in three functions of the UHCI code. The functions are called uhci_root_enable, uhci_root_speed and uhci_root_poll.

Below you can find the three functions with the changes I have made to solve this enumeration problem.

static int uhci_root_enable ( struct usb_hub *hub, struct usb_port *port ) {
struct uhci_device *uhci = usb_hub_get_drvdata ( hub );
uint16_t portsc;
unsigned int i;

/* Reset port */
portsc = inw ( uhci->regs + UHCI_PORTSC ( port->address ) );
portsc |= UHCI_PORTSC_PR;
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );
mdelay ( USB_RESET_DELAY_MS );
portsc &= ~UHCI_PORTSC_PR;
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );
mdelay ( USB_RESET_RECOVER_DELAY_MS );

/* Enable port */
portsc |= UHCI_PORTSC_PED;
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );
mdelay ( USB_RESET_RECOVER_DELAY_MS );

/* Wait for port to become enabled */
for ( i = 0 ; i < UHCI_PORT_ENABLE_MAX_WAIT_MS ; i++ ) {

/* Check port status */
portsc = inw ( uhci->regs + UHCI_PORTSC ( port->address ) );
if ( portsc & UHCI_PORTSC_PED && portsc & UHCI_PORTSC_CCS )
{
port->disconnected = 0;
return 0;
}

/* Delay */
mdelay ( 1 );
}

DBGC ( uhci, "UHCI %s-%d timed out waiting for port to enable "
"(status %04x)\n", uhci->name, port->address, portsc );
return -ETIMEDOUT;
}

static int uhci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
struct uhci_device *uhci = usb_hub_get_drvdata ( hub );
struct pci_device pci;
uint16_t portsc;
unsigned int speed;

/* Read port status */
portsc = inw ( uhci->regs + UHCI_PORTSC ( port->address ) );
if ( ! ( portsc & UHCI_PORTSC_CCS ) ) {
/* Port not connected */
speed = USB_SPEED_NONE;
} else if ( uhci->companion &&
! find_usb_bus_by_location ( BUS_TYPE_PCI,
uhci->companion ) ) {
/* Defer connection detection until companion
* controller has been enumerated.
*/
pci_init ( &pci, uhci->companion );
DBGC ( uhci, "UHCI %s-%d deferring for companion " PCI_FMT "\n",
uhci->name, port->address, PCI_ARGS ( &pci ) );
speed = USB_SPEED_NONE;
} else if ( portsc & UHCI_PORTSC_LS ) {
/* Low-speed device */
speed = USB_SPEED_LOW;
} else {
/* Full-speed device */
speed = USB_SPEED_FULL;
}
port->speed = speed;

/* Record disconnections and clear changes */
port->disconnected |= ( (portsc & UHCI_PORTSC_CCS) == 0 );
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );

return 0;
}

static void uhci_root_poll ( struct usb_hub *hub, struct usb_port *port ) {
struct uhci_device *uhci = usb_hub_get_drvdata ( hub );
uint16_t portsc;
uint16_t change;

/* Do nothing unless something has changed */
portsc = inw ( uhci->regs + UHCI_PORTSC ( port->address ) );
change = ( portsc & UHCI_PORTSC_CHANGE );
if ( ! change )
return;

/* Record disconnections and clear changes */
port->disconnected |= ( (portsc & UHCI_PORTSC_CCS) == 0 );
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );

/* Report port status change */
usb_port_changed ( port );
}

In the function uhci_root_enable I initialize with 0 the disconnected variable of the port structure when the port is enabled and the current connect status bit is set. This part was using the UHCI_PORTSC_CSC mask instead of the UHCI_PORTSC_CCS mask I have put into the code.

The only change to the other two functions is the use of the UHCI_PORTSC_CCS mask instead of the UHCI_PORTSC_CSC mask.

I am not an expert on the iPXE source code, so some with deep knowledge of the iPXE USB stack could take a look at the changes I have made and integrate them into the codebase if they are deemed worthy.

Thanks in advance and execuse my English.

Eugenio

[NiKiZe]

Could you post the output of

Code:

git diff

or maybe even post it to the mailinglist or make a pull request on github?

See http://ipxe.org/admin#commit_log_messages for how to format commits the iPXE way, but even without it a pull request or git diff would simplify review greatly.

[eugenio]

@@ -1176,8 +1176,11 @@ static int uhci_root_enable ( struct usb_hub *hub, struct usb_port *port ) {

/* Check port status */
portsc = inw ( uhci->regs + UHCI_PORTSC ( port->address ) );
- if ( portsc & UHCI_PORTSC_PED )
+ if ( portsc & UHCI_PORTSC_PED && portsc & UHCI_PORTSC_CCS )
+ {
+ port->disconnected = 0;
return 0;
+ }

/* Delay */
mdelay ( 1 );
@@ -1245,7 +1248,7 @@ static int uhci_root_speed ( struct usb_hub *hub, struct usb_port *port ) {
port->speed = speed;

/* Record disconnections and clear changes */
- port->disconnected |= ( portsc & UHCI_PORTSC_CSC );
+ port->disconnected |= ( (portsc & UHCI_PORTSC_CCS) == 0 );
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );

return 0;
@@ -1288,7 +1291,7 @@ static void uhci_root_poll ( struct usb_hub *hub, struct usb_port *port ) {
return;

/* Record disconnections and clear changes */
- port->disconnected |= ( portsc & UHCI_PORTSC_CSC );
+ port->disconnected |= ( (portsc & UHCI_PORTSC_CCS) == 0 );
outw ( portsc, uhci->regs + UHCI_PORTSC ( port->address ) );

/* Report port status change */