> -----Original Message----- > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > Sent: Thursday, August 04, 2011 2:10 AM > To: Xu, Andiry > Cc: linux-usb@xxxxxxxxxxxxxxx; gregkh@xxxxxxx; > stern@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 8/8] xHCI: set USB2 hardware LPM > > Hi Andiry, > > I'm trying to get the general overview of this patchset, so please > correct me if I have any assumptions wrong. AFAIK, the USB core > doesn't > support software controlled USB 2.0 LPM. Unless the xHCI host > supports hardware controlled USB 2.0 LPM, the devices won't actually > enter U2, correct? > Yes. > You say this patchset only currently supports devices attached directly > to the roothub. What's your plan for supporting devices behind hubs? I don't know how to support devices behind hubs, since it seems to me that there is no way to enable USB2 hardware LPM for a device behind hubs. I think the SetPortFeature(PORT_L1) request described in USB2 LPM spec needs to be sent by software, not hardware. > Does it make sense at that point to put the testing for LPM support in > khubd? The USB core can always keep track of the "bad" devices in a > list per host controller, in case someone has two xHCI host controllers, > and LPM works on some of them and not on others. > It can be moved to usbcore, but the current "bad" devices are also stored per host controller. > I think it might also make sense to move the clearing of the link state > into the USB core. External USB 3.0 hubs will send a port status > change > event for link changes, and the USB core will need to handle that. I'm > not quite sure how to handle the fact that the xHCI driver will report > link status changes for USB 2.0 ports, when external USB 2.0 hubs won't. > Maybe it makes sense to clear the link state in the xHCI driver only > for > USB 2.0 ports? > As you point out, moving the PLC clearing to usbcore is pointless, because usbcore clears link state change only when it sees a C_PORT_LINK_STATE set in wPortChange, which will never be reported by USB2 ports. The best way is to clear it in handle_port_status() only for USB2 ports. > I have a couple specific questions on other patches, and I'll send > those > separately. > OK, comments appreciated. > > p.s. In general, can you please change this type of style: > > if (condition) { > /* multi-line */ > /* body */ > } else > /* single-line body with no { } */ > > to include square brackets around the else body? I know it adds one > more line, but it makes it easier to identify corresponding if-else > blocks, and I'd like to stick with the current style in the driver. > Sure, will do. Thanks, Andiry -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
