Re: [PATCH v6 1/2] USB host: Move AMD PLL quirk to pci-quirks.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello.

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>
[...]

diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 76179c3..1ec8060 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -44,35 +44,6 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
 	return 0;
 }
-static int ehci_quirk_amd_SB800(struct ehci_hcd *ehci)
-{
-	struct pci_dev *amd_smbus_dev;
-	u8 rev = 0;
-
-	amd_smbus_dev = pci_get_device(PCI_VENDOR_ID_ATI, 0x4385, NULL);
-	if (!amd_smbus_dev)
-		return 0;
-
-	pci_read_config_byte(amd_smbus_dev, PCI_REVISION_ID, &rev);
-	if (rev < 0x40) {
-		pci_dev_put(amd_smbus_dev);
-		amd_smbus_dev = NULL;
-		return 0;
-	}
-
-	if (!amd_nb_dev)
-		amd_nb_dev = pci_get_device(PCI_VENDOR_ID_AMD, 0x1510, NULL);
-	if (!amd_nb_dev)
-		ehci_err(ehci, "QUIRK: unable to get AMD NB device\n");
-
-	ehci_info(ehci, "QUIRK: Enable AMD SB800 L1 fix\n");
-
-	pci_dev_put(amd_smbus_dev);
-	amd_smbus_dev = NULL;
-
-	return 1;
-}
-

Er, this shouldn't apply to the current mainline anymore as this function has been renamed and extended to probe for device ID 0x780B -- see:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=baab93afc2844b68d57b0dcca5e1d34c5d7cf411

This commit is not yet merged to Greg's 'usb-next' branch, when it will be, there's gonna be a reject...

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 4c502c8..344b25a 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -52,6 +52,266 @@
 #define EHCI_USBLEGCTLSTS	4		/* legacy control/status */
 #define EHCI_USBLEGCTLSTS_SOOE	(1 << 13)	/* SMI on ownership change */
+/* AMD quirk use */
+#define	AB_REG_BAR_LOW		0xe0
+#define	AB_REG_BAR_HIGH		0xe1
+#define	AB_REG_BAR_SB700	0xf0
+#define	AB_INDX(addr)		((addr) + 0x00)
+#define	AB_DATA(addr)		((addr) + 0x04)
+#define	AX_INDXC		0x30
+#define	AX_DATAC		0x34
+
+#define	NB_PCIE_INDX_ADDR	0xe0
+#define	NB_PCIE_INDX_DATA	0xe4
+#define	PCIE_P_CNTL		0x10040
+#define	BIF_NB			0x10002
+#define	NB_PIF0_PWRDOWN_0	0x01100012
+#define	NB_PIF0_PWRDOWN_1	0x01100013
+
+static struct amd_chipset_info {
+	struct pci_dev	*nb_dev;
+	struct pci_dev	*smbus_dev;
+	int nb_type;
+	int sb_type;
+	int isoc_reqs;
+	int probe_count;
+	int probe_result;
+} amd_chipset;
+
+static DEFINE_SPINLOCK(amd_lock);
+
+int usb_amd_find_chipset_info(void)
+{
+	u8 rev = 0;
+	unsigned long flags;
+
+	spin_lock_irqsave(&amd_lock, flags);
+
+	amd_chipset.probe_count++;
+	/* probe only once */
+	if (amd_chipset.probe_count > 1) {
+		spin_unlock_irqrestore(&amd_lock, flags);
+		return amd_chipset.probe_result;
+	}
+
+	amd_chipset.smbus_dev = pci_get_device(PCI_VENDOR_ID_ATI, 0x4385, NULL);
+	if (amd_chipset.smbus_dev) {
+		pci_read_config_byte(amd_chipset.smbus_dev,
+					PCI_REVISION_ID, &rev);

There is no need to read the revision ID from the register, it's stored in the 'revision' field of 'struct pci_dev', i.e. 'amd_chipset.smbus_dev->revision'. I was going to post the patch replacing the register access but then realized that you're moving the code from ehci-pci.c and ohci-pci.c here.

+		if (rev >= 0x40)
+			amd_chipset.sb_type = 1;
+		else if (rev >= 0x30 && rev <= 0x3b)
+			amd_chipset.sb_type = 3;
+	} else {
+		amd_chipset.smbus_dev = pci_get_device(PCI_VENDOR_ID_AMD,
+							0x780b, NULL);
+		if (!amd_chipset.smbus_dev) {
+			spin_unlock_irqrestore(&amd_lock, flags);
+			return 0;
+		}
+		pci_read_config_byte(amd_chipset.smbus_dev,
+					PCI_REVISION_ID, &rev);

   Same comment here.

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;

   Returns are not needed.

+}
+#endif  /* CONFIG_PCI */

WBR, Sergei
--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux