On Mon, Feb 28, 2011 at 11:40:38AM +0800, Xu, Andiry wrote: > > -----Original Message----- > > From: Greg KH [mailto:greg@xxxxxxxxx] > > Sent: Saturday, February 26, 2011 3:30 AM > > To: Xu, Andiry > > Cc: Sergei Shtylyov; gregkh@xxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > > dbrownell@xxxxxxxxxxxxxxxxxxxxx; sarah.a.sharp@xxxxxxxxxxxxxxx; He, > Alex > > Subject: Re: [PATCH resend] USB host: Move AMD PLL quirk to > pci-quirks.c > > > > On Fri, Feb 18, 2011 at 04:30:01PM +0300, Sergei Shtylyov wrote: > > > Hello. > > > > > > On 18-02-2011 12:48, Andiry Xu wrote: > > > > > > >This patch moves the AMD PLL quirk code in OHCI/EHCI driver to pci- > > quirks.c, > > > >and exports the functions to be used by xHCI driver later. > > > > > > >AMD PLL quirk disable the optional PM feature inside specific > > > >SB700/SB800/Hudson-2/3 platforms under the following conditions: > > > > > > >1. If an isochronous device is connected to OHCI/EHCI/xHCI port and > is > > active; > > > >2. Optional PM feature that powers down the internal Bus PLL when > the > > link is > > > > in low power state is enabled. > > > > > > >Without AMD PLL quirk, USB isochronous stream may stutter or have > > breaks > > > >occasionally, which greatly impair the performance of audio/video > > streams. > > > > > > >Currently AMD PLL quirk is implemented in OHCI and EHCI driver, and > > will be > > > >added to xHCI driver too. They are doing similar things actually, > so > > move > > > >the quirk code to pci-quirks.c, which has several advantages: > > > > > > >1. Remove duplicate defines and functions in OHCI/EHCI (and xHCI) > > driver and > > > > make them cleaner; > > > >2. AMD chipset information will be probed only once and then > stored. > > > > Currently they're probed during every OHCI/EHCI initialization, > > move > > > > the detect code to pci-quirks.c saves the repeat detect cost; > > > >3. Build up synchronization among OHCI/EHCI/xHCI driver. In current > > > > code, every host controller enable/disable PLL only according > to > > > > its own status, and may enable PLL while there is still isoc > > transfer on > > > > other HCs. Move the quirk to pci-quirks.c prevents this issue. > > > > > > >Signed-off-by: Andiry Xu<andiry.xu@xxxxxxx> > > > >Cc: David Brownell<dbrownell@xxxxxxxxxxxxxxxxxxxxx> > > > >Cc: Alex He<alex.he@xxxxxxx> > > > > > > This is not a resend, this is a new version. I see you've dropped > > > the revision ID register reads -- you should have mentioned this > > > under --- tearline. > > > > > > >diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci- > > quirks.h > > > >index 1564edf..4d2118c 100644 > > > >--- a/drivers/usb/host/pci-quirks.h > > > >+++ b/drivers/usb/host/pci-quirks.h > > > >@@ -1,7 +1,26 @@ > > > > #ifndef __LINUX_USB_PCI_QUIRKS_H > > > > #define __LINUX_USB_PCI_QUIRKS_H > > > > > > > >+#ifdef CONFIG_PCI > > > > void uhci_reset_hc(struct pci_dev *pdev, unsigned long base); > > > > int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long > base); > > > >+int usb_amd_find_chipset_info(void); > > > >+void usb_amd_dev_put(void); > > > >+void usb_amd_quirk_pll_disable(void); > > > >+void usb_amd_quirk_pll_enable(void); > > > >+#else > > > >+static inline void usb_amd_quirk_pll_disable(void) > > > >+{ > > > >+ return; > > > >+} > > > >+static inline void usb_amd_quirk_pll_enable(void) > > > >+{ > > > >+ return; > > > >+} > > > >+static inline void usb_amd_dev_put(void) > > > >+{ > > > >+ return; > > > >+} > > > >+#endif /* CONFIG_PCI */ > > > > > > As I said, these *return* statements are not needed.. > > > > Andiry, care to redo this patch? > > > > Hmm, I wonder how to redo it? Have a new version based on the latest > usb-next? Yes. > Does any code change required? Sergei says return statements > are not needed but have them there is OK. They are not needed, so please remove them. thanks, greg k-h -- 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