On Fri, Oct 24, 2014 at 10:35:29AM -0600, Bjorn Helgaas wrote: > On Fri, Oct 17, 2014 at 04:53:27PM +0800, Huang Rui wrote: > > The dwc3 controller is the PCI-E device in AMD NL platform, but the class code > > of PCI header is 0x0c0330, the same with xHC. That's because it needs to meet > > the windows enviroment. The dwc3 controller acted as only host mode to bind with > > windows xhci driver. > > But on linux, sometimes, it would auto-bind with xhci driver as well (class code > > 0x0c0330) despite it uses Pid/Vid on dwc3 driver. > > This changelog really doesn't make any sense unless the reader already > knows everything. I think you mean something like this: > > The AMD NL (please explain what "NL" is; Google has no clue) SoC contains > a DesignWare USB3 Dual-Role Device that can be operated either as a USB > Host or a USB Device. In the AMD NL platform, this device ([1022:7912]) > has a class code of PCI_CLASS_SERIAL_USB_XHCI (0x0c0330), which means the > xhci driver will claim it. > > But the dwc3 driver is a more specific driver for this device, and we'd > prefer to use it instead of xhci. To prevent xhci from claiming the > device, change the class code to 0x0c03fe, which the PCI r3.0 spec > defines as "USB device (not host controller)". The dwc3 driver can then > claim it based on its Vendor and Device ID. > > Obviously, this means the device won't work at all unless dwc3 is enabled. > Previously, it probably would work as a host controller with the xhci > driver. I assume this change is what you want. > OK, I will update to this changelog in V3. > > Heikki suggested to use the quirk to fix this issue, and the detailed discussion > > is at below thread: > > http://marc.info/?l=linux-usb&m=141197934712824&w=2 > > This changelog needs to be wrapped so no line is longer than 75 characters. > > Please run "git log --oneline --no-merges drivers/pci/quirks.c" and make > your subject line match the rest. In particular, "PCI" (not "pci"), drop > the "quirks:" part, and capitalize "Add". > I got it, will update subject. > > Suggested-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > > Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > Signed-off-by: Huang Rui <ray.huang@xxxxxxx> > > --- > > drivers/pci/quirks.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index 90acb32..3c911b7 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -378,6 +378,26 @@ static void quirk_ati_exploding_mce(struct pci_dev *dev) > > } > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, PCI_DEVICE_ID_ATI_RS100, quirk_ati_exploding_mce); > > > > +/* FIXME should define in <linux/pci_ids.h> */ > > +#define PCI_DEVICE_ID_AMD_NL 0x7912 > > If you KNOW this should be in linux/pci_ids.h (and it looks like it COULD > go there, since you do use the same definition in patch [01/16]), why don't > you just do it? Why are you wasting our time reviewing trivial stuff like > this? > Apology, I was told that it could not add device id into pci_ids.h from my buddy with below thread discussion last year, but I don't confirm at current. I check the git log at pci_ids.h, it looks like it's able to add device id marco again, is that right? http://marc.info/?l=linux-pci&m=137028614924258&w=2 If I misunderstand your meaning, please correct me. > > + > > +/* > > + * AMD NL SoC 7912 PCI device is dwc3 controller, but the class code of PCI > > + * header is 0x0c0330, the same with xHC. That's because it need to meet the > > + * windows enviroment. The dwc3 controller acted as only host mode to bind with > > + * windows xhci driver. But on linux, sometimes, we auto-bind with xhci driver > > + * as well (class code 0x0c0330) despite we use Pid/Vid on dwc3 driver. So this > > + * quirk used a dummy class code to avoid to bind with xhci driver at booting > > + * phase. > > + */ > > +static void quirk_amd_nl_class(struct pci_dev *pdev) > > +{ > > + /* Use a dummy class value instead of PCI header provided */ > > When you say "dummy class value," that makes it sound like you just chose > a random invalid value. But this value is actually defined by the spec, so > you should use the description from the spec, namely, "USB device (not host > controller)". > Yes, you're right. Will change this description. Thanks, Rui -- 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
