Skip to contentSkip to conversation

x86, usb, pci: Disable usb legacy support early

PATCH In review Yinghai Lu 8 participants 57 messages Jan 9, 2011 – Jan 16, 2011 1 message not in the archive

Subsystems (5)
x86 USB PCI USB xHCI driver +1 more subsystems
lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

cover x86, usb, pci: Disable usb legacy support early

move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

Thanks

Yinghai Lu

author
#1 lore raw

cover x86, usb, pci: Disable usb legacy support early

Move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

-v2: according to BenH, keep pci_dev *pdev with those quirk function.

Thanks

Yinghai Lu

author
#51 lore raw

PATCH 1/4 pci, usb: Seperate usb handoff func to another file

In reply to a message not in the archive

So later could reuse them to do usb handoff much early for x86.

will make arch early code get MMIO BAR and do remapping itself.

-v2: still keep pci_device *pdev as parameter according to BenH.

---
 drivers/usb/host/pci-quirks.c  |  382 +-------------------------------------
 drivers/usb/host/usb_handoff.c |  402 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 420 insertions(+), 364 deletions(-)
drivers/usb/host/pci-quirks.c +18 -364
Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================
@@ -15,122 +15,28 @@
 #include <linux/delay.h>
 #include <linux/acpi.h>
 #include "pci-quirks.h"
-#include "xhci-ext-caps.h"
 
+static void usb_handoff_udelay(unsigned long usecs)
+{
+	udelay(usecs);
+}
+
+static void usb_handoff_msleep(unsigned long msecs)
+{
+	msleep(msecs);
+}
+
+#include "usb_handoff.c"
 
-#define UHCI_USBLEGSUP		0xc0		/* legacy support */
-#define UHCI_USBCMD		0		/* command register */
-#define UHCI_USBINTR		4		/* interrupt register */
-#define UHCI_USBLEGSUP_RWC	0x8f00		/* the R/WC bits */
-#define UHCI_USBLEGSUP_RO	0x5040		/* R/O and reserved bits */
-#define UHCI_USBCMD_RUN		0x0001		/* RUN/STOP bit */
-#define UHCI_USBCMD_HCRESET	0x0002		/* Host Controller reset */
-#define UHCI_USBCMD_EGSM	0x0008		/* Global Suspend Mode */
-#define UHCI_USBCMD_CONFIGURE	0x0040		/* Config Flag */
-#define UHCI_USBINTR_RESUME	0x0002		/* Resume interrupt enable */
-
-#define OHCI_CONTROL		0x04
-#define OHCI_CMDSTATUS		0x08
-#define OHCI_INTRSTATUS		0x0c
-#define OHCI_INTRENABLE		0x10
-#define OHCI_INTRDISABLE	0x14
-#define OHCI_OCR		(1 << 3)	/* ownership change request */
-#define OHCI_CTRL_RWC		(1 << 9)	/* remote wakeup connected */
-#define OHCI_CTRL_IR		(1 << 8)	/* interrupt routing */
-#define OHCI_INTR_OC		(1 << 30)	/* ownership change */
-
-#define EHCI_HCC_PARAMS		0x08		/* extended capabilities */
-#define EHCI_USBCMD		0		/* command register */
-#define EHCI_USBCMD_RUN		(1 << 0)	/* RUN/STOP bit */
-#define EHCI_USBSTS		4		/* status register */
-#define EHCI_USBSTS_HALTED	(1 << 12)	/* HCHalted bit */
-#define EHCI_USBINTR		8		/* interrupt register */
-#define EHCI_CONFIGFLAG		0x40		/* configured flag register */
-#define EHCI_USBLEGSUP		0		/* legacy support register */
-#define EHCI_USBLEGSUP_BIOS	(1 << 16)	/* BIOS semaphore */
-#define EHCI_USBLEGSUP_OS	(1 << 24)	/* OS semaphore */
-#define EHCI_USBLEGCTLSTS	4		/* legacy control/status */
-#define EHCI_USBLEGCTLSTS_SOOE	(1 << 13)	/* SMI on ownership change */
-
-
-/*
- * Make sure the controller is completely inactive, unable to
- * generate interrupts or do DMA.
- */
 void uhci_reset_hc(struct pci_dev *pdev, unsigned long base)
 {
-	/* Turn off PIRQ enable and SMI enable.  (This also turns off the
-	 * BIOS's USB Legacy Support.)  Turn off all the R/WC bits too.
-	 */
-	pci_write_config_word(pdev, UHCI_USBLEGSUP, UHCI_USBLEGSUP_RWC);
-
-	/* Reset the HC - this will force us to get a
-	 * new notification of any already connected
-	 * ports due to the virtual disconnect that it
-	 * implies.
-	 */
-	outw(UHCI_USBCMD_HCRESET, base + UHCI_USBCMD);
-	mb();
-	udelay(5);
-	if (inw(base + UHCI_USBCMD) & UHCI_USBCMD_HCRESET)
-		dev_warn(&pdev->dev, "HCRESET not completed yet!\n");
-
-	/* Just to be safe, disable interrupt requests and
-	 * make sure the controller is stopped.
-	 */
-	outw(0, base + UHCI_USBINTR);
-	outw(0, base + UHCI_USBCMD);
+	__uhci_reset_hc(pdev, base);
 }
 EXPORT_SYMBOL_GPL(uhci_reset_hc);
 
-/*
- * Initialize a controller that was newly discovered or has just been
- * resumed.  In either case we can't be sure of its previous state.
- *
- * Returns: 1 if the controller was reset, 0 otherwise.
- */
 int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base)
 {
-	u16 legsup;
-	unsigned int cmd, intr;
-
-	/*
-	 * When restarting a suspended controller, we expect all the
-	 * settings to be the same as we left them:
-	 *
-	 *	PIRQ and SMI disabled, no R/W bits set in USBLEGSUP;
-	 *	Controller is stopped and configured with EGSM set;
-	 *	No interrupts enabled except possibly Resume Detect.
-	 *
-	 * If any of these conditions are violated we do a complete reset.
-	 */
-	pci_read_config_word(pdev, UHCI_USBLEGSUP, &legsup);
-	if (legsup & ~(UHCI_USBLEGSUP_RO | UHCI_USBLEGSUP_RWC)) {
-		dev_dbg(&pdev->dev, "%s: legsup = 0x%04x\n",
-				__func__, legsup);
-		goto reset_needed;
-	}
-
-	cmd = inw(base + UHCI_USBCMD);
-	if ((cmd & UHCI_USBCMD_RUN) || !(cmd & UHCI_USBCMD_CONFIGURE) ||
-			!(cmd & UHCI_USBCMD_EGSM)) {
-		dev_dbg(&pdev->dev, "%s: cmd = 0x%04x\n",
-				__func__, cmd);
-		goto reset_needed;
-	}
-
-	intr = inw(base + UHCI_USBINTR);
-	if (intr & (~UHCI_USBINTR_RESUME)) {
-		dev_dbg(&pdev->dev, "%s: intr = 0x%04x\n",
-				__func__, intr);
-		goto reset_needed;
-	}
-	return 0;
-
-reset_needed:
-	dev_dbg(&pdev->dev, "Performing full reset\n");
-	uhci_reset_hc(pdev, base);
-	return 1;
+	return	__uhci_check_and_reset_hc(pdev, base);
 }
 EXPORT_SYMBOL_GPL(uhci_check_and_reset_hc);
 
@@ -169,7 +75,6 @@ static int __devinit mmio_resource_enabl
 static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
 {
 	void __iomem *base;
-	u32 control;
 
 	if (!mmio_resource_enabled(pdev, 0))
 		return;
@@ -178,50 +83,14 @@ static void __devinit quirk_usb_handoff_
 	if (base == NULL)
 		return;
 
-	control = readl(base + OHCI_CONTROL);
-
-/* On PA-RISC, PDC can leave IR set incorrectly; ignore it there. */
-#ifdef __hppa__
-#define	OHCI_CTRL_MASK		(OHCI_CTRL_RWC | OHCI_CTRL_IR)
-#else
-#define	OHCI_CTRL_MASK		OHCI_CTRL_RWC
-
-	if (control & OHCI_CTRL_IR) {
-		int wait_time = 500; /* arbitrary; 5 seconds */
-		writel(OHCI_INTR_OC, base + OHCI_INTRENABLE);
-		writel(OHCI_OCR, base + OHCI_CMDSTATUS);
-		while (wait_time > 0 &&
-				readl(base + OHCI_CONTROL) & OHCI_CTRL_IR) {
-			wait_time -= 10;
-			msleep(10);
-		}
-		if (wait_time <= 0)
-			dev_warn(&pdev->dev, "OHCI: BIOS handoff failed"
-					" (BIOS bug?) %08x\n",
-					readl(base + OHCI_CONTROL));
-	}
-#endif
-
-	/* reset controller, preserving RWC (and possibly IR) */
-	writel(control & OHCI_CTRL_MASK, base + OHCI_CONTROL);
-
-	/*
-	 * disable interrupts
-	 */
-	writel(~(u32)0, base + OHCI_INTRDISABLE);
-	writel(~(u32)0, base + OHCI_INTRSTATUS);
+	__usb_handoff_ohci(pdev, base);
 
 	iounmap(base);
 }
 
 static void __devinit quirk_usb_disable_ehci(struct pci_dev *pdev)
 {
-	int wait_time, delta;
-	void __iomem *base, *op_reg_base;
-	u32	hcc_params, val;
-	u8	offset, cap_length;
-	int	count = 256/4;
-	int	tried_handoff = 0;
+	void __iomem *base;
 
 	if (!mmio_resource_enabled(pdev, 0))
 		return;
@@ -230,238 +99,23 @@ static void __devinit quirk_usb_disable_
 	if (base == NULL)
 		return;
 
-	cap_length = readb(base);
-	op_reg_base = base + cap_length;
-
-	/* EHCI 0.96 and later may have "extended capabilities"
-	 * spec section 5.1 explains the bios handoff, e.g. for
-	 * booting from USB disk or using a usb keyboard
-	 */
-	hcc_params = readl(base + EHCI_HCC_PARAMS);
-	offset = (hcc_params >> 8) & 0xff;
-	while (offset && --count) {
-		u32		cap;
-		int		msec;
-
-		pci_read_config_dword(pdev, offset, &cap);
-		switch (cap & 0xff) {
-		case 1:			/* BIOS/SMM/... handoff support */
-			if ((cap & EHCI_USBLEGSUP_BIOS)) {
-				dev_dbg(&pdev->dev, "EHCI: BIOS handoff\n");
-
-#if 0
-/* [email protected] reports that some systems need SMI forced on,
- * but that seems dubious in general (the BIOS left it off intentionally)
- * and is known to prevent some systems from booting.  so we won't do this
- * unless maybe we can determine when we're on a system that needs SMI forced.
- */
-				/* BIOS workaround (?): be sure the
-				 * pre-Linux code receives the SMI
-				 */
-				pci_read_config_dword(pdev,
-						offset + EHCI_USBLEGCTLSTS,
-						&val);
-				pci_write_config_dword(pdev,
-						offset + EHCI_USBLEGCTLSTS,
-						val | EHCI_USBLEGCTLSTS_SOOE);
-#endif
-
-				/* some systems get upset if this semaphore is
-				 * set for any other reason than forcing a BIOS
-				 * handoff..
-				 */
-				pci_write_config_byte(pdev, offset + 3, 1);
-			}
-
-			/* if boot firmware now owns EHCI, spin till
-			 * it hands it over.
-			 */
-			msec = 1000;
-			while ((cap & EHCI_USBLEGSUP_BIOS) && (msec > 0)) {
-				tried_handoff = 1;
-				msleep(10);
-				msec -= 10;
-				pci_read_config_dword(pdev, offset, &cap);
-			}
-
-			if (cap & EHCI_USBLEGSUP_BIOS) {
-				/* well, possibly buggy BIOS... try to shut
-				 * it down, and hope nothing goes too wrong
-				 */
-				dev_warn(&pdev->dev, "EHCI: BIOS handoff failed"
-						" (BIOS bug?) %08x\n", cap);
-				pci_write_config_byte(pdev, offset + 2, 0);
-			}
-
-			/* just in case, always disable EHCI SMIs */
-			pci_write_config_dword(pdev,
-					offset + EHCI_USBLEGCTLSTS,
-					0);
-
-			/* If the BIOS ever owned the controller then we
-			 * can't expect any power sessions to remain intact.
-			 */
-			if (tried_handoff)
-				writel(0, op_reg_base + EHCI_CONFIGFLAG);
-			break;
-		case 0:			/* illegal reserved capability */
-			cap = 0;
-			/* FALLTHROUGH */
-		default:
-			dev_warn(&pdev->dev, "EHCI: unrecognized capability "
-					"%02x\n", cap & 0xff);
-			break;
-		}
-		offset = (cap >> 8) & 0xff;
-	}
-	if (!count)
-		dev_printk(KERN_DEBUG, &pdev->dev, "EHCI: capability loop?\n");
-
-	/*
-	 * halt EHCI & disable its interrupts in any case
-	 */
-	val = readl(op_reg_base + EHCI_USBSTS);
-	if ((val & EHCI_USBSTS_HALTED) == 0) {
-		val = readl(op_reg_base + EHCI_USBCMD);
-		val &= ~EHCI_USBCMD_RUN;
-		writel(val, op_reg_base + EHCI_USBCMD);
-
-		wait_time = 2000;
-		delta = 100;
-		do {
-			writel(0x3f, op_reg_base + EHCI_USBSTS);
-			udelay(delta);
-			wait_time -= delta;
-			val = readl(op_reg_base + EHCI_USBSTS);
-			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED)) {
-				break;
-			}
-		} while (wait_time > 0);
-	}
-	writel(0, op_reg_base + EHCI_USBINTR);
-	writel(0x3f, op_reg_base + EHCI_USBSTS);
+	__usb_handoff_ehci(pdev, base);
 
 	iounmap(base);
 }
 
-/*
- * handshake - spin reading a register until handshake completes
- * @ptr: address of hc register to be read
- * @mask: bits to look at in result of read
- * @done: value of those bits when handshake succeeds
- * @wait_usec: timeout in microseconds
- * @delay_usec: delay in microseconds to wait between polling
- *
- * Polls a register every delay_usec microseconds.
- * Returns 0 when the mask bits have the value done.
- * Returns -ETIMEDOUT if this condition is not true after
- * wait_usec microseconds have passed.
- */
-static int handshake(void __iomem *ptr, u32 mask, u32 done,
-		int wait_usec, int delay_usec)
-{
-	u32	result;
-
-	do {
-		result = readl(ptr);
-		result &= mask;
-		if (result == done)
-			return 0;
-		udelay(delay_usec);
-		wait_usec -= delay_usec;
-	} while (wait_usec > 0);
-	return -ETIMEDOUT;
-}
-
-/**
- * PCI Quirks for xHCI.
- *
- * Takes care of the handoff between the Pre-OS (i.e. BIOS) and the OS.
- * It signals to the BIOS that the OS wants control of the host controller,
- * and then waits 5 seconds for the BIOS to hand over control.
- * If we timeout, assume the BIOS is broken and take control anyway.
- */
 static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
 {
 	void __iomem *base;
-	int ext_cap_offset;
-	void __iomem *op_reg_base;
-	u32 val;
-	int timeout;
 
 	if (!mmio_resource_enabled(pdev, 0))
 		return;
 
-	base = ioremap_nocache(pci_resource_start(pdev, 0),
-				pci_resource_len(pdev, 0));
+	base = pci_ioremap_bar(pdev, 0);
 	if (base == NULL)
 		return;
 
-	/*
-	 * Find the Legacy Support Capability register -
-	 * this is optional for xHCI host controllers.
-	 */
-	ext_cap_offset = xhci_find_next_cap_offset(base, XHCI_HCC_PARAMS_OFFSET);
-	do {
-		if (!ext_cap_offset)
-			/* We've reached the end of the extended capabilities */
-			goto hc_init;
-		val = readl(base + ext_cap_offset);
-		if (XHCI_EXT_CAPS_ID(val) == XHCI_EXT_CAPS_LEGACY)
-			break;
-		ext_cap_offset = xhci_find_next_cap_offset(base, ext_cap_offset);
-	} while (1);
-
-	/* If the BIOS owns the HC, signal that the OS wants it, and wait */
-	if (val & XHCI_HC_BIOS_OWNED) {
-		writel(val & XHCI_HC_OS_OWNED, base + ext_cap_offset);
-
-		/* Wait for 5 seconds with 10 microsecond polling interval */
-		timeout = handshake(base + ext_cap_offset, XHCI_HC_BIOS_OWNED,
-				0, 5000, 10);
-
-		/* Assume a buggy BIOS and take HC ownership anyway */
-		if (timeout) {
-			dev_warn(&pdev->dev, "xHCI BIOS handoff failed"
-					" (BIOS bug ?) %08x\n", val);
-			writel(val & ~XHCI_HC_BIOS_OWNED, base + ext_cap_offset);
-		}
-	}
-
-	/* Disable any BIOS SMIs */
-	writel(XHCI_LEGACY_DISABLE_SMI,
-			base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
-
-hc_init:
-	op_reg_base = base + XHCI_HC_LENGTH(readl(base));
-
-	/* Wait for the host controller to be ready before writing any
-	 * operational or runtime registers.  Wait 5 seconds and no more.
-	 */
-	timeout = handshake(op_reg_base + XHCI_STS_OFFSET, XHCI_STS_CNR, 0,
-			5000, 10);
-	/* Assume a buggy HC and start HC initialization anyway */
-	if (timeout) {
-		val = readl(op_reg_base + XHCI_STS_OFFSET);
-		dev_warn(&pdev->dev,
-				"xHCI HW not ready after 5 sec (HC bug?) "
-				"status = 0x%x\n", val);
-	}
-
-	/* Send the halt and disable interrupts command */
-	val = readl(op_reg_base + XHCI_CMD_OFFSET);
-	val &= ~(XHCI_CMD_RUN | XHCI_IRQS);
-	writel(val, op_reg_base + XHCI_CMD_OFFSET);
-
-	/* Wait for the HC to halt - poll every 125 usec (one microframe). */
-	timeout = handshake(op_reg_base + XHCI_STS_OFFSET, XHCI_STS_HALT, 1,
-			XHCI_MAX_HALT_USEC, 125);
-	if (timeout) {
-		val = readl(op_reg_base + XHCI_STS_OFFSET);
-		dev_warn(&pdev->dev,
-				"xHCI HW did not halt within %d usec "
-				"status = 0x%x\n", XHCI_MAX_HALT_USEC, val);
-	}
+	__usb_handoff_xhci(pdev, base);
 
 	iounmap(base);
 }
drivers/usb/host/usb_handoff.c new file +402 -0

402 lines not on this page · raw message

author
#54 lore raw

PATCH 3/4 x86, pci: add dummy pci device for early stage

In reply to a message not in the archive

So we can pass pci_dev *dev to reuse some generic pci functions.

---
 arch/x86/include/asm/pci-direct.h |    2 +
 arch/x86/pci/early.c              |   75 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
arch/x86/include/asm/pci-direct.h +2 -0
Index: linux-2.6/arch/x86/include/asm/pci-direct.h
===================================================================
@@ -18,4 +18,6 @@ extern int early_pci_allowed(void);
 extern unsigned int pci_early_dump_regs;
 extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
 extern void early_dump_pci_devices(void);
+
+struct pci_dev *get_early_pci_dev(int num, int slot, int func);
 #endif /* _ASM_X86_PCI_DIRECT_H */
arch/x86/pci/early.c +75 -0

78 lines not on this page · raw message

author
#53 lore raw

PATCH v2 4/4 x86: usb handoff in early_quirk

In reply to a message not in the archive

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

reuse code from drivers/usb/host/pci-quirks.c
with changes
1. delay and sleep ===> io_delay
2. dev_warn etc to pr_warn(num, slot, func...)

-v2: use get_early_pci_dev() etc.
fix typo with PARAVIRT according to Sander Eikelenboom <[email protected]>

---
 arch/x86/include/asm/pci-direct.h |    2 
 arch/x86/kernel/early-quirks.c    |  170 ++++++++++++++++++++++++++++++++++++++
 arch/x86/pci/early.c              |   72 ++++++++++++++++
 3 files changed, 244 insertions(+)
arch/x86/kernel/early-quirks.c +170 -0
Index: linux-2.6/arch/x86/kernel/early-quirks.c
===================================================================
@@ -19,6 +19,174 @@
 #include <asm/iommu.h>
 #include <asm/gart.h>
 
+static inline void early_udelay2(void)
+{
+#ifndef CONFIG_PARAVIRT
+	native_io_delay();
+#else
+	pv_cpu_ops.io_delay();
+#endif
+}
+
+static void usb_handoff_udelay(unsigned long usecs)
+{
+	unsigned long count;
+
+	count = usecs >> 1;
+
+	if (!count)
+		count = 1;
+
+	while (count-- > 0)
+		early_udelay2();
+}
+
+static void usb_handoff_msleep(unsigned long msecs)
+{
+	while (msecs-- > 0)
+		usb_handoff_udelay(1000);
+}
+
+#include "../../../drivers/usb/host/usb_handoff.c"
+
+static inline int io_type_enabled(int num, int slot, int func, unsigned int mask)
+{
+	return read_pci_config_16(num, slot, func, PCI_COMMAND) & mask;
+}
+
+#define pio_enabled(num, slot, func) io_type_enabled(num, slot, func, PCI_COMMAND_IO)
+#define mmio_enabled(num, slot, func) io_type_enabled(num, slot, func, PCI_COMMAND_MEMORY)
+
+static __init u32 get_usb_io_port(int num, int slot, int func)
+{
+	int i;
+
+	if (!pio_enabled(num, slot, func))
+		return 0;
+
+	for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+		u32 addr = read_pci_config(num, slot, func, 0x10+(i<<2));
+
+		if (!addr)
+			continue;
+		if ((addr & PCI_BASE_ADDRESS_SPACE) != PCI_BASE_ADDRESS_SPACE_IO)
+			continue;
+
+		addr &= PCI_BASE_ADDRESS_IO_MASK;
+		if (addr)
+			return addr;
+	}
+
+	return 0;
+}
+
+static void __init quirk_usb_handoff_uhci(int num, int slot, int func)
+{
+	unsigned long base;
+
+	base = get_usb_io_port(num, slot, func);
+	if (!base)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: uhci ioport = 0x%04lx\n",
+					num, slot, func, base);
+	__uhci_check_and_reset_hc(get_early_pci_dev(num, slot, func), base);
+}
+
+static __init u32 get_usb_mmio_addr(int num, int slot, int func)
+{
+	u32 addr;
+
+	if (!mmio_enabled(num, slot, func))
+		return 0;
+
+	addr = read_pci_config(num, slot, func, 0x10);
+	if (!addr)
+		return 0;
+	if ((addr & PCI_BASE_ADDRESS_SPACE) != PCI_BASE_ADDRESS_SPACE_MEMORY)
+		return 0;
+
+	addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+	return addr;
+}
+
+static __init void quirk_usb_handoff_ohci(int num, int slot, int func)
+{
+	void __iomem *base;
+	u32 addr;
+
+	addr = get_usb_mmio_addr(num, slot, func);
+	if (!addr)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: ohci mmio = 0x%08x\n", num, slot, func, addr);
+	base = early_ioremap(addr, 0x1000);
+	if (!base)
+		return;
+
+	__usb_handoff_ohci(get_early_pci_dev(num, slot, func), base);
+
+	early_iounmap(base, 0x1000);
+}
+
+static __init void quirk_usb_handoff_ehci(int num, int slot, int func)
+{
+	void __iomem *base;
+	u32 addr;
+
+	addr = get_usb_mmio_addr(num, slot, func);
+	if (!addr)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: ehci mmio = 0x%08x\n",
+				 num, slot, func, addr);
+	base = early_ioremap(addr, 0x1000);
+	if (!base)
+		return;
+
+	__usb_handoff_ehci(get_early_pci_dev(num, slot, func), base);
+
+	early_iounmap(base, 0x1000);
+}
+
+static __init void quirk_usb_handoff_xhci(int num, int slot, int func)
+{
+	void __iomem *base;
+	u32 addr;
+
+	addr = get_usb_mmio_addr(num, slot, func);
+	if (!addr)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: xhci mmio = 0x%08x\n",
+				num, slot, func, addr);
+	base = early_ioremap(addr, 0x1000);
+	if (!base)
+		return;
+
+	__usb_handoff_xhci(get_early_pci_dev(num, slot, func), base);
+
+	early_iounmap(base, 0x1000);
+}
+
+static __init void quirk_usb_handoff(int num, int slot, int func)
+{
+	u32 class;
+
+	class = read_pci_config(num, slot, func, PCI_CLASS_REVISION);
+	class >>= 8;
+
+	if (class == PCI_CLASS_SERIAL_USB_UHCI)
+		quirk_usb_handoff_uhci(num, slot, func);
+	else if (class == PCI_CLASS_SERIAL_USB_OHCI)
+		quirk_usb_handoff_ohci(num, slot, func);
+	else if (class == PCI_CLASS_SERIAL_USB_EHCI)
+		quirk_usb_handoff_ehci(num, slot, func);
+	else if (class == PCI_CLASS_SERIAL_USB_XHCI)
+		quirk_usb_handoff_xhci(num, slot, func);
+}
+
 static void __init fix_hypertransport_config(int num, int slot, int func)
 {
 	u32 htcfg;
@@ -208,6 +376,8 @@ struct chipset {
  * only matching on bus 0.
  */
 static struct chipset early_qrk[] __initdata = {
+	{ PCI_ANY_ID, PCI_ANY_ID,
+	  PCI_CLASS_SERIAL_USB, PCI_ANY_ID, 0, quirk_usb_handoff },
 	{ PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
 	  PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs },
 	{ PCI_VENDOR_ID_VIA, PCI_ANY_ID,
author
#52 lore raw

PATCH 2/4 x86: early_quirk check all dev/func in domain 0

In reply to a message not in the archive

go with all buses instead of bus0.
also only check header type on func0.

---
 arch/x86/kernel/early-quirks.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)
arch/x86/kernel/early-quirks.c +16 -13
Index: linux-2.6/arch/x86/kernel/early-quirks.c
===================================================================
@@ -243,7 +243,7 @@ static int __init check_dev_quirk(int nu
 	class = read_pci_config_16(num, slot, func, PCI_CLASS_DEVICE);
 
 	if (class == 0xffff)
-		return -1; /* no class, treat as single function */
+		return 0; /* no class, func gap */
 
 	vendor = read_pci_config_16(num, slot, func, PCI_VENDOR_ID);
 
@@ -263,27 +263,30 @@ static int __init check_dev_quirk(int nu
 			}
 	}
 
-	type = read_pci_config_byte(num, slot, func,
-				    PCI_HEADER_TYPE);
-	if (!(type & 0x80))
-		return -1;
+	if (func == 0) {
+		type = read_pci_config_byte(num, slot, func,
+					    PCI_HEADER_TYPE);
+		if (!(type & 0x80))
+			return -1;
+	}
 
 	return 0;
 }
 
 void __init early_quirks(void)
 {
-	int slot, func;
+	int num, slot, func;
 
 	if (!early_pci_allowed())
 		return;
 
 	/* Poor man's PCI discovery */
-	/* Only scan the root bus */
-	for (slot = 0; slot < 32; slot++)
-		for (func = 0; func < 8; func++) {
-			/* Only probe function 0 on single fn devices */
-			if (check_dev_quirk(0, slot, func))
-				break;
-		}
+	/* Only can scan first domain */
+	for (num = 0; num < 256; num++)
+		for (slot = 0; slot < 32; slot++)
+			for (func = 0; func < 8; func++) {
+				/* Only probe func 0 on single fn devices */
+				if (check_dev_quirk(num, slot, func))
+					break;
+			}
 }
#55 lore raw
In reply to Yinghai Lu
Quoted 3 lines · Yinghai Lu

move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

Is this a real problem today? What is the symptom of the issue? Where was it reported as a problem?

thanks,

greg k-h

#56 lore raw
Quoted 7 lines · On Mon, 10 Jan 2011 07:57:24 -0800 Greg KH <[email protected]> wrote:
Quoted 3 lines · Yinghai Lu

move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

Is this a real problem today? What is the symptom of the issue? Where was it reported as a problem?

Yes, please give us a rationale for all this mess. I really don't like the sleep wrappers or the "early" PCI device; would it be easier to extract some of the core disabling MMIO from the quirks and re-use it for a totally separate set of early quirk functions? Like Ben suggested, making it totally arch specific would be a good option, though presumably most arches don't need this (again if you explained why/when this was needed maybe we could come up with a better solution, or just ignore the problem if you're trying to fix a pre-production board again with all sorts of broken stuff).

Signature · 1 line
Jesse Barnes, Intel Open Source Technology Center
author
#57 lore raw
Quoted 11 lines · Jesse Barnes
Quoted 7 lines · On Mon, 10 Jan 2011 07:57:24 -0800 Greg KH <[email protected]> wrote:
Quoted 3 lines · Yinghai Lu

move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

Is this a real problem today? What is the symptom of the issue? Where was it reported as a problem?

Quoted 6 lines

Yes, please give us a rationale for all this mess. I really don't like the sleep wrappers or the "early" PCI device; would it be easier to extract some of the core disabling MMIO from the quirks and re-use it for a totally separate set of early quirk functions? Like Ben suggested, making it totally arch specific would be a good option, though presumably most arches don't need this

sure, will have new version to address that.

Quoted 5 lines

(again if you explained
why/when this was needed maybe we could come up with a better solution, or just ignore the problem if you're trying to fix a pre-production board again with all sorts of broken stuff).

not on pre-production board.

v3 should be clean enough. will send out later.

Thanks

Yinghai

author
#2 lore raw

cover x86, usb, pci: Disable usb legacy support early

In reply to Yinghai Lu

Move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

-v2: according to BenH, keep pci_dev *pdev with those quirk function. -v3: expose related functions in header files to avoid including c files.

Thanks

Yinghai Lu

author
#43 lore raw

PATCH 3/4 x86, pci: add dummy pci device for early stage

So we can pass pci_dev *dev to reuse some generic pci functions.

---
 arch/x86/include/asm/pci-direct.h |    2 +
 arch/x86/pci/early.c              |   75 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)
arch/x86/include/asm/pci-direct.h +2 -0
Index: linux-2.6/arch/x86/include/asm/pci-direct.h
===================================================================
@@ -18,4 +18,6 @@ extern int early_pci_allowed(void);
 extern unsigned int pci_early_dump_regs;
 extern void early_dump_pci_device(u8 bus, u8 slot, u8 func);
 extern void early_dump_pci_devices(void);
+
+struct pci_dev *get_early_pci_dev(int num, int slot, int func);
 #endif /* _ASM_X86_PCI_DIRECT_H */
arch/x86/pci/early.c +75 -0

78 lines not on this page · raw message

author
#38 lore raw

PATCH 2/4 x86: early_quirk check all dev/func in domain 0

In reply to Yinghai Lu

go with all buses instead of bus0.
also only check header type on func0.

---
 arch/x86/kernel/early-quirks.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)
arch/x86/kernel/early-quirks.c +16 -13
Index: linux-2.6/arch/x86/kernel/early-quirks.c
===================================================================
@@ -243,7 +243,7 @@ static int __init check_dev_quirk(int nu
 	class = read_pci_config_16(num, slot, func, PCI_CLASS_DEVICE);
 
 	if (class == 0xffff)
-		return -1; /* no class, treat as single function */
+		return 0; /* no class, func gap */
 
 	vendor = read_pci_config_16(num, slot, func, PCI_VENDOR_ID);
 
@@ -263,27 +263,30 @@ static int __init check_dev_quirk(int nu
 			}
 	}
 
-	type = read_pci_config_byte(num, slot, func,
-				    PCI_HEADER_TYPE);
-	if (!(type & 0x80))
-		return -1;
+	if (func == 0) {
+		type = read_pci_config_byte(num, slot, func,
+					    PCI_HEADER_TYPE);
+		if (!(type & 0x80))
+			return -1;
+	}
 
 	return 0;
 }
 
 void __init early_quirks(void)
 {
-	int slot, func;
+	int num, slot, func;
 
 	if (!early_pci_allowed())
 		return;
 
 	/* Poor man's PCI discovery */
-	/* Only scan the root bus */
-	for (slot = 0; slot < 32; slot++)
-		for (func = 0; func < 8; func++) {
-			/* Only probe function 0 on single fn devices */
-			if (check_dev_quirk(0, slot, func))
-				break;
-		}
+	/* Only can scan first domain */
+	for (num = 0; num < 256; num++)
+		for (slot = 0; slot < 32; slot++)
+			for (func = 0; func < 8; func++) {
+				/* Only probe func 0 on single fn devices */
+				if (check_dev_quirk(num, slot, func))
+					break;
+			}
 }
author
#3 lore raw

PATCH 1/4 pci, usb: Make usb handoff func all take base remapping

In reply to Yinghai Lu

So later could reuse them to do usb handoff much early for x86.

will make arch early code get MMIO BAR and do remapping itself.

-v2: still keep pci_device *pdev as parameter according to BenH. -v3: expose three functions that take *base instead of including .c file.

---
 drivers/usb/host/pci-quirks.c |  195 ++++++++++++++++++++++++------------------
 drivers/usb/host/pci-quirks.h |    6 +
 2 files changed, 120 insertions(+), 81 deletions(-)
drivers/usb/host/pci-quirks.c +114 -81
Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================
@@ -17,6 +17,19 @@
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
+static void default_usb_handoff_udelay(unsigned long usecs)
+{
+	udelay(usecs);
+}
+
+static void __devinit default_usb_handoff_msleep(unsigned long msecs)
+{
+	msleep(msecs);
+}
+
+void (*usb_handoff_udelay)(unsigned long) = default_usb_handoff_udelay;
+void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+			default_usb_handoff_msleep;
 
 #define UHCI_USBLEGSUP		0xc0		/* legacy support */
 #define UHCI_USBCMD		0		/* command register */
@@ -71,7 +84,7 @@ void uhci_reset_hc(struct pci_dev *pdev,
 	 */
 	outw(UHCI_USBCMD_HCRESET, base + UHCI_USBCMD);
 	mb();
-	udelay(5);
+	(*usb_handoff_udelay)(5);
 	if (inw(base + UHCI_USBCMD) & UHCI_USBCMD_HCRESET)
 		dev_warn(&pdev->dev, "HCRESET not completed yet!\n");
 
@@ -106,78 +119,38 @@ int uhci_check_and_reset_hc(struct pci_d
 	 */
 	pci_read_config_word(pdev, UHCI_USBLEGSUP, &legsup);
 	if (legsup & ~(UHCI_USBLEGSUP_RO | UHCI_USBLEGSUP_RWC)) {
-		dev_dbg(&pdev->dev, "%s: legsup = 0x%04x\n",
-				__func__, legsup);
+		dev_printk(KERN_DEBUG, &pdev->dev,
+				 "legsup = 0x%04x\n", legsup);
 		goto reset_needed;
 	}
 
 	cmd = inw(base + UHCI_USBCMD);
 	if ((cmd & UHCI_USBCMD_RUN) || !(cmd & UHCI_USBCMD_CONFIGURE) ||
 			!(cmd & UHCI_USBCMD_EGSM)) {
-		dev_dbg(&pdev->dev, "%s: cmd = 0x%04x\n",
-				__func__, cmd);
+		dev_printk(KERN_DEBUG, &pdev->dev, "cmd = 0x%04x\n", cmd);
 		goto reset_needed;
 	}
 
 	intr = inw(base + UHCI_USBINTR);
 	if (intr & (~UHCI_USBINTR_RESUME)) {
-		dev_dbg(&pdev->dev, "%s: intr = 0x%04x\n",
-				__func__, intr);
+		dev_printk(KERN_DEBUG, &pdev->dev, "intr = 0x%04x\n", intr);
 		goto reset_needed;
 	}
 	return 0;
 
 reset_needed:
-	dev_dbg(&pdev->dev, "Performing full reset\n");
+	dev_printk(KERN_DEBUG, &pdev->dev, "Performing full reset\n");
+
 	uhci_reset_hc(pdev, base);
+
 	return 1;
 }
 EXPORT_SYMBOL_GPL(uhci_check_and_reset_hc);
 
-static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask)
-{
-	u16 cmd;
-	return !pci_read_config_word(pdev, PCI_COMMAND, &cmd) && (cmd & mask);
-}
-
-#define pio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_IO)
-#define mmio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_MEMORY)
-
-static void __devinit quirk_usb_handoff_uhci(struct pci_dev *pdev)
-{
-	unsigned long base = 0;
-	int i;
-
-	if (!pio_enabled(pdev))
-		return;
-
-	for (i = 0; i < PCI_ROM_RESOURCE; i++)
-		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
-			base = pci_resource_start(pdev, i);
-			break;
-		}
-
-	if (base)
-		uhci_check_and_reset_hc(pdev, base);
-}
-
-static int __devinit mmio_resource_enabled(struct pci_dev *pdev, int idx)
+void __devinit __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base)
 {
-	return pci_resource_start(pdev, idx) && mmio_enabled(pdev);
-}
-
-static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
-{
-	void __iomem *base;
 	u32 control;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = pci_ioremap_bar(pdev, 0);
-	if (base == NULL)
-		return;
-
 	control = readl(base + OHCI_CONTROL);
 
 /* On PA-RISC, PDC can leave IR set incorrectly; ignore it there. */
@@ -193,7 +166,7 @@ static void __devinit quirk_usb_handoff_
 		while (wait_time > 0 &&
 				readl(base + OHCI_CONTROL) & OHCI_CTRL_IR) {
 			wait_time -= 10;
-			msleep(10);
+			(*usb_handoff_msleep)(10);
 		}
 		if (wait_time <= 0)
 			dev_warn(&pdev->dev, "OHCI: BIOS handoff failed"
@@ -210,26 +183,17 @@ static void __devinit quirk_usb_handoff_
 	 */
 	writel(~(u32)0, base + OHCI_INTRDISABLE);
 	writel(~(u32)0, base + OHCI_INTRSTATUS);
-
-	iounmap(base);
 }
 
-static void __devinit quirk_usb_disable_ehci(struct pci_dev *pdev)
+void __devinit __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base)
 {
 	int wait_time, delta;
-	void __iomem *base, *op_reg_base;
+	void *op_reg_base;
 	u32	hcc_params, val;
 	u8	offset, cap_length;
 	int	count = 256/4;
 	int	tried_handoff = 0;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = pci_ioremap_bar(pdev, 0);
-	if (base == NULL)
-		return;
-
 	cap_length = readb(base);
 	op_reg_base = base + cap_length;
 
@@ -247,7 +211,8 @@ static void __devinit quirk_usb_disable_
 		switch (cap & 0xff) {
 		case 1:			/* BIOS/SMM/... handoff support */
 			if ((cap & EHCI_USBLEGSUP_BIOS)) {
-				dev_dbg(&pdev->dev, "EHCI: BIOS handoff\n");
+				dev_printk(KERN_DEBUG, &pdev->dev,
+						"EHCI: BIOS handoff\n");
 
 #if 0
 /* [email protected] reports that some systems need SMI forced on,
@@ -279,7 +244,7 @@ static void __devinit quirk_usb_disable_
 			msec = 1000;
 			while ((cap & EHCI_USBLEGSUP_BIOS) && (msec > 0)) {
 				tried_handoff = 1;
-				msleep(10);
+				(*usb_handoff_msleep)(10);
 				msec -= 10;
 				pci_read_config_dword(pdev, offset, &cap);
 			}
@@ -330,18 +295,15 @@ static void __devinit quirk_usb_disable_
 		delta = 100;
 		do {
 			writel(0x3f, op_reg_base + EHCI_USBSTS);
-			udelay(delta);
+			(*usb_handoff_udelay)(delta);
 			wait_time -= delta;
 			val = readl(op_reg_base + EHCI_USBSTS);
-			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED)) {
+			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED))
 				break;
-			}
 		} while (wait_time > 0);
 	}
 	writel(0, op_reg_base + EHCI_USBINTR);
 	writel(0x3f, op_reg_base + EHCI_USBSTS);
-
-	iounmap(base);
 }
 
 /*
@@ -357,7 +319,7 @@ static void __devinit quirk_usb_disable_
  * Returns -ETIMEDOUT if this condition is not true after
  * wait_usec microseconds have passed.
  */
-static int handshake(void __iomem *ptr, u32 mask, u32 done,
+static int __devinit handshake(void __iomem *ptr, u32 mask, u32 done,
 		int wait_usec, int delay_usec)
 {
 	u32	result;
@@ -367,7 +329,7 @@ static int handshake(void __iomem *ptr,
 		result &= mask;
 		if (result == done)
 			return 0;
-		udelay(delay_usec);
+		(*usb_handoff_udelay)(delay_usec);
 		wait_usec -= delay_usec;
 	} while (wait_usec > 0);
 	return -ETIMEDOUT;
@@ -381,22 +343,13 @@ static int handshake(void __iomem *ptr,
  * and then waits 5 seconds for the BIOS to hand over control.
  * If we timeout, assume the BIOS is broken and take control anyway.
  */
-static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base)
 {
-	void __iomem *base;
 	int ext_cap_offset;
 	void __iomem *op_reg_base;
 	u32 val;
 	int timeout;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = ioremap_nocache(pci_resource_start(pdev, 0),
-				pci_resource_len(pdev, 0));
-	if (base == NULL)
-		return;
-
 	/*
 	 * Find the Legacy Support Capability register -
 	 * this is optional for xHCI host controllers.
@@ -462,6 +415,86 @@ hc_init:
 				"xHCI HW did not halt within %d usec "
 				"status = 0x%x\n", XHCI_MAX_HALT_USEC, val);
 	}
+}
+
+static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask)
+{
+	u16 cmd;
+	return !pci_read_config_word(pdev, PCI_COMMAND, &cmd) && (cmd & mask);
+}
+
+#define pio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_IO)
+#define mmio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_MEMORY)
+
+static void __devinit quirk_usb_handoff_uhci(struct pci_dev *pdev)
+{
+	unsigned long base = 0;
+	int i;
+
+	if (!pio_enabled(pdev))
+		return;
+
+	for (i = 0; i < PCI_ROM_RESOURCE; i++)
+		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
+			base = pci_resource_start(pdev, i);
+			break;
+		}
+
+	if (!base)
+		return;
+
+	uhci_check_and_reset_hc(pdev, base);
+}
+
+static int __devinit mmio_resource_enabled(struct pci_dev *pdev, int idx)
+{
+	return pci_resource_start(pdev, idx) && mmio_enabled(pdev);
+}
+
+static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_ohci(pdev, base);
+
+	iounmap(base);
+}
+
+static void __devinit quirk_usb_handoff_ehci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_ehci(pdev, base);
+
+	iounmap(base);
+}
+
+static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_xhci(pdev, base);
 
 	iounmap(base);
 }
@@ -473,7 +506,7 @@ static void __devinit quirk_usb_early_ha
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_OHCI)
 		quirk_usb_handoff_ohci(pdev);
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_EHCI)
-		quirk_usb_disable_ehci(pdev);
+		quirk_usb_handoff_ehci(pdev);
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
 		quirk_usb_handoff_xhci(pdev);
 }
drivers/usb/host/pci-quirks.h +6 -0
Index: linux-2.6/drivers/usb/host/pci-quirks.h
===================================================================
@@ -3,5 +3,11 @@
 
 void uhci_reset_hc(struct pci_dev *pdev, unsigned long base);
 int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base);
+void __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base);
+
+extern void (*usb_handoff_udelay)(unsigned long);
+extern void (*usb_handoff_msleep)(unsigned long);
 
 #endif  /*  __LINUX_USB_PCI_QUIRKS_H  */
author
#44 lore raw

PATCH 4/4 x86: usb handoff in early_quirk

In reply to Yinghai Lu

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

reuse code from drivers/usb/host/pci-quirks.c
with changes
1. delay and sleep ===> io_delay
2. dev_warn etc to pr_warn(num, slot, func...)

-v2: use get_early_pci_dev() etc.
fix typo with PARAVIRT according to Sander Eikelenboom <[email protected]>
-v3: include .h instead of .c file

---
 arch/x86/kernel/early-quirks.c |  175 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 175 insertions(+)
arch/x86/kernel/early-quirks.c +175 -0
Index: linux-2.6/arch/x86/kernel/early-quirks.c
===================================================================
@@ -19,6 +19,179 @@
 #include <asm/iommu.h>
 #include <asm/gart.h>
 
+#include "../../../drivers/usb/host/pci-quirks.h"
+
+static inline void early_udelay2(void)
+{
+#ifndef CONFIG_PARAVIRT
+	native_io_delay();
+#else
+	pv_cpu_ops.io_delay();
+#endif
+}
+
+static void early_usb_handoff_udelay(unsigned long usecs)
+{
+	unsigned long count;
+
+	count = usecs >> 1;
+
+	if (!count)
+		count = 1;
+
+	while (count-- > 0)
+		early_udelay2();
+}
+
+static void early_usb_handoff_msleep(unsigned long msecs)
+{
+	while (msecs-- > 0)
+		early_usb_handoff_udelay(1000);
+}
+
+static __init u32 get_usb_io_port(int num, int slot, int func)
+{
+	int i;
+	u16 cmd;
+
+	cmd = read_pci_config_16(num, slot, func, PCI_COMMAND);
+	if (!(cmd & PCI_COMMAND_IO))
+		return 0;
+
+	for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+		u32 addr = read_pci_config(num, slot, func, 0x10 + (i<<2));
+
+		if (!addr)
+			continue;
+		if ((addr&PCI_BASE_ADDRESS_SPACE) != PCI_BASE_ADDRESS_SPACE_IO)
+			continue;
+
+		addr &= PCI_BASE_ADDRESS_IO_MASK;
+		if (addr)
+			return addr;
+	}
+
+	return 0;
+}
+
+static void __init quirk_usb_handoff_uhci(int num, int slot, int func)
+{
+	unsigned long base;
+
+	base = get_usb_io_port(num, slot, func);
+	if (!base)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: uhci ioport = 0x%04lx\n",
+					num, slot, func, base);
+	uhci_check_and_reset_hc(get_early_pci_dev(num, slot, func), base);
+}
+
+static __init u32 get_usb_mmio_addr(int num, int slot, int func)
+{
+	u32 addr;
+	u16 cmd;
+
+	cmd = read_pci_config_16(num, slot, func, PCI_COMMAND);
+	if (!(cmd & PCI_COMMAND_MEMORY))
+		return 0;
+
+	addr = read_pci_config(num, slot, func, 0x10);
+	if (!addr)
+		return 0;
+	if ((addr & PCI_BASE_ADDRESS_SPACE) != PCI_BASE_ADDRESS_SPACE_MEMORY)
+		return 0;
+
+	addr &= PCI_BASE_ADDRESS_MEM_MASK;
+
+	return addr;
+}
+
+static __init void quirk_usb_handoff_ohci(int num, int slot, int func)
+{
+	void __iomem *base;
+	u32 addr;
+
+	addr = get_usb_mmio_addr(num, slot, func);
+	if (!addr)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: ohci mmio = 0x%08x\n",
+				num, slot, func, addr);
+	base = early_ioremap(addr, 0x1000);
+	if (!base)
+		return;
+
+	__usb_handoff_ohci(get_early_pci_dev(num, slot, func), base);
+
+	early_iounmap(base, 0x1000);
+}
+
+static __init void quirk_usb_handoff_ehci(int num, int slot, int func)
+{
+	void __iomem *base;
+	u32 addr;
+
+	addr = get_usb_mmio_addr(num, slot, func);
+	if (!addr)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: ehci mmio = 0x%08x\n",
+				 num, slot, func, addr);
+	base = early_ioremap(addr, 0x1000);
+	if (!base)
+		return;
+
+	__usb_handoff_ehci(get_early_pci_dev(num, slot, func), base);
+
+	early_iounmap(base, 0x1000);
+}
+
+static __init void quirk_usb_handoff_xhci(int num, int slot, int func)
+{
+	void __iomem *base;
+	u32 addr;
+
+	addr = get_usb_mmio_addr(num, slot, func);
+	if (!addr)
+		return;
+
+	printk(KERN_DEBUG "%02x:%02x.%01x: xhci mmio = 0x%08x\n",
+				num, slot, func, addr);
+	base = early_ioremap(addr, 0x1000);
+	if (!base)
+		return;
+
+	__usb_handoff_xhci(get_early_pci_dev(num, slot, func), base);
+
+	early_iounmap(base, 0x1000);
+}
+
+static __init void quirk_usb_handoff(int num, int slot, int func)
+{
+	u32 class;
+	void (*old_usb_handoff_udelay)(unsigned long) = usb_handoff_udelay;
+	void (*old_usb_handoff_msleep)(unsigned long) = usb_handoff_msleep;
+
+	usb_handoff_udelay = early_usb_handoff_udelay;
+	usb_handoff_msleep = early_usb_handoff_msleep;
+
+	class = read_pci_config(num, slot, func, PCI_CLASS_REVISION);
+	class >>= 8;
+
+	if (class == PCI_CLASS_SERIAL_USB_UHCI)
+		quirk_usb_handoff_uhci(num, slot, func);
+	else if (class == PCI_CLASS_SERIAL_USB_OHCI)
+		quirk_usb_handoff_ohci(num, slot, func);
+	else if (class == PCI_CLASS_SERIAL_USB_EHCI)
+		quirk_usb_handoff_ehci(num, slot, func);
+	else if (class == PCI_CLASS_SERIAL_USB_XHCI)
+		quirk_usb_handoff_xhci(num, slot, func);
+
+	usb_handoff_udelay = old_usb_handoff_udelay;
+	usb_handoff_msleep = old_usb_handoff_msleep;
+}
+
 static void __init fix_hypertransport_config(int num, int slot, int func)
 {
 	u32 htcfg;
@@ -208,6 +381,8 @@ struct chipset {
  * only matching on bus 0.
  */
 static struct chipset early_qrk[] __initdata = {
+	{ PCI_ANY_ID, PCI_ANY_ID,
+	  PCI_CLASS_SERIAL_USB, PCI_ANY_ID, 0, quirk_usb_handoff },
 	{ PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID,
 	  PCI_CLASS_BRIDGE_PCI, PCI_ANY_ID, QFLAG_APPLY_ONCE, nvidia_bugs },
 	{ PCI_VENDOR_ID_VIA, PCI_ANY_ID,
#4 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

In reply to Yinghai Lu
Quoted 22 lines · Yinghai Lu

So later could reuse them to do usb handoff much early for x86.

will make arch early code get MMIO BAR and do remapping itself.

(quoted diff) +0 -2
-v2: still keep pci_device *pdev as parameter according to BenH.
-v3: expose three functions that take *base instead of including .c file.
---
 drivers/usb/host/pci-quirks.c |  195 ++++++++++++++++++++++++------------------
 drivers/usb/host/pci-quirks.h |    6 +
 2 files changed, 120 insertions(+), 81 deletions(-)

Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================

drivers/usb/host/pci-quirks.c +0 -0
@@ -17,6 +17,19 @@
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"

drivers/usb/host/pci-quirks.c line 28

Quoted 10 lines
drivers/usb/host/pci-quirks.c +9 -0
 
+static void default_usb_handoff_udelay(unsigned long usecs)
+{
+	udelay(usecs);
+}
+
+static void __devinit default_usb_handoff_msleep(unsigned long msecs)
+{
+	msleep(msecs);
+}

What?

Why in the world would you not just call the real functions here? That's not acceptable, sorry.

Quoted 324 lines
drivers/usb/host/pci-quirks.c +105 -81
+
+void (*usb_handoff_udelay)(unsigned long) = default_usb_handoff_udelay;
+void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+			default_usb_handoff_msleep;
 
 #define UHCI_USBLEGSUP		0xc0		/* legacy support */
 #define UHCI_USBCMD		0		/* command register */
@@ -71,7 +84,7 @@ void uhci_reset_hc(struct pci_dev *pdev,
 	 */
 	outw(UHCI_USBCMD_HCRESET, base + UHCI_USBCMD);
 	mb();
-	udelay(5);
+	(*usb_handoff_udelay)(5);
 	if (inw(base + UHCI_USBCMD) & UHCI_USBCMD_HCRESET)
 		dev_warn(&pdev->dev, "HCRESET not completed yet!\n");
 
@@ -106,78 +119,38 @@ int uhci_check_and_reset_hc(struct pci_d
 	 */
 	pci_read_config_word(pdev, UHCI_USBLEGSUP, &legsup);
 	if (legsup & ~(UHCI_USBLEGSUP_RO | UHCI_USBLEGSUP_RWC)) {
-		dev_dbg(&pdev->dev, "%s: legsup = 0x%04x\n",
-				__func__, legsup);
+		dev_printk(KERN_DEBUG, &pdev->dev,
+				 "legsup = 0x%04x\n", legsup);
 		goto reset_needed;
 	}
 
 	cmd = inw(base + UHCI_USBCMD);
 	if ((cmd & UHCI_USBCMD_RUN) || !(cmd & UHCI_USBCMD_CONFIGURE) ||
 			!(cmd & UHCI_USBCMD_EGSM)) {
-		dev_dbg(&pdev->dev, "%s: cmd = 0x%04x\n",
-				__func__, cmd);
+		dev_printk(KERN_DEBUG, &pdev->dev, "cmd = 0x%04x\n", cmd);
 		goto reset_needed;
 	}
 
 	intr = inw(base + UHCI_USBINTR);
 	if (intr & (~UHCI_USBINTR_RESUME)) {
-		dev_dbg(&pdev->dev, "%s: intr = 0x%04x\n",
-				__func__, intr);
+		dev_printk(KERN_DEBUG, &pdev->dev, "intr = 0x%04x\n", intr);
 		goto reset_needed;
 	}
 	return 0;
 
 reset_needed:
-	dev_dbg(&pdev->dev, "Performing full reset\n");
+	dev_printk(KERN_DEBUG, &pdev->dev, "Performing full reset\n");
+
 	uhci_reset_hc(pdev, base);
+
 	return 1;
 }
 EXPORT_SYMBOL_GPL(uhci_check_and_reset_hc);
 
-static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask)
-{
-	u16 cmd;
-	return !pci_read_config_word(pdev, PCI_COMMAND, &cmd) && (cmd & mask);
-}
-
-#define pio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_IO)
-#define mmio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_MEMORY)
-
-static void __devinit quirk_usb_handoff_uhci(struct pci_dev *pdev)
-{
-	unsigned long base = 0;
-	int i;
-
-	if (!pio_enabled(pdev))
-		return;
-
-	for (i = 0; i < PCI_ROM_RESOURCE; i++)
-		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
-			base = pci_resource_start(pdev, i);
-			break;
-		}
-
-	if (base)
-		uhci_check_and_reset_hc(pdev, base);
-}
-
-static int __devinit mmio_resource_enabled(struct pci_dev *pdev, int idx)
+void __devinit __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base)
 {
-	return pci_resource_start(pdev, idx) && mmio_enabled(pdev);
-}
-
-static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
-{
-	void __iomem *base;
 	u32 control;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = pci_ioremap_bar(pdev, 0);
-	if (base == NULL)
-		return;
-
 	control = readl(base + OHCI_CONTROL);
 
 /* On PA-RISC, PDC can leave IR set incorrectly; ignore it there. */
@@ -193,7 +166,7 @@ static void __devinit quirk_usb_handoff_
 		while (wait_time > 0 &&
 				readl(base + OHCI_CONTROL) & OHCI_CTRL_IR) {
 			wait_time -= 10;
-			msleep(10);
+			(*usb_handoff_msleep)(10);
 		}
 		if (wait_time <= 0)
 			dev_warn(&pdev->dev, "OHCI: BIOS handoff failed"
@@ -210,26 +183,17 @@ static void __devinit quirk_usb_handoff_
 	 */
 	writel(~(u32)0, base + OHCI_INTRDISABLE);
 	writel(~(u32)0, base + OHCI_INTRSTATUS);
-
-	iounmap(base);
 }
 
-static void __devinit quirk_usb_disable_ehci(struct pci_dev *pdev)
+void __devinit __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base)
 {
 	int wait_time, delta;
-	void __iomem *base, *op_reg_base;
+	void *op_reg_base;
 	u32	hcc_params, val;
 	u8	offset, cap_length;
 	int	count = 256/4;
 	int	tried_handoff = 0;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = pci_ioremap_bar(pdev, 0);
-	if (base == NULL)
-		return;
-
 	cap_length = readb(base);
 	op_reg_base = base + cap_length;
 
@@ -247,7 +211,8 @@ static void __devinit quirk_usb_disable_
 		switch (cap & 0xff) {
 		case 1:			/* BIOS/SMM/... handoff support */
 			if ((cap & EHCI_USBLEGSUP_BIOS)) {
-				dev_dbg(&pdev->dev, "EHCI: BIOS handoff\n");
+				dev_printk(KERN_DEBUG, &pdev->dev,
+						"EHCI: BIOS handoff\n");
 
 #if 0
 /* [email protected] reports that some systems need SMI forced on,
@@ -279,7 +244,7 @@ static void __devinit quirk_usb_disable_
 			msec = 1000;
 			while ((cap & EHCI_USBLEGSUP_BIOS) && (msec > 0)) {
 				tried_handoff = 1;
-				msleep(10);
+				(*usb_handoff_msleep)(10);
 				msec -= 10;
 				pci_read_config_dword(pdev, offset, &cap);
 			}
@@ -330,18 +295,15 @@ static void __devinit quirk_usb_disable_
 		delta = 100;
 		do {
 			writel(0x3f, op_reg_base + EHCI_USBSTS);
-			udelay(delta);
+			(*usb_handoff_udelay)(delta);
 			wait_time -= delta;
 			val = readl(op_reg_base + EHCI_USBSTS);
-			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED)) {
+			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED))
 				break;
-			}
 		} while (wait_time > 0);
 	}
 	writel(0, op_reg_base + EHCI_USBINTR);
 	writel(0x3f, op_reg_base + EHCI_USBSTS);
-
-	iounmap(base);
 }
 
 /*
@@ -357,7 +319,7 @@ static void __devinit quirk_usb_disable_
  * Returns -ETIMEDOUT if this condition is not true after
  * wait_usec microseconds have passed.
  */
-static int handshake(void __iomem *ptr, u32 mask, u32 done,
+static int __devinit handshake(void __iomem *ptr, u32 mask, u32 done,
 		int wait_usec, int delay_usec)
 {
 	u32	result;
@@ -367,7 +329,7 @@ static int handshake(void __iomem *ptr,
 		result &= mask;
 		if (result == done)
 			return 0;
-		udelay(delay_usec);
+		(*usb_handoff_udelay)(delay_usec);
 		wait_usec -= delay_usec;
 	} while (wait_usec > 0);
 	return -ETIMEDOUT;
@@ -381,22 +343,13 @@ static int handshake(void __iomem *ptr,
  * and then waits 5 seconds for the BIOS to hand over control.
  * If we timeout, assume the BIOS is broken and take control anyway.
  */
-static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base)
 {
-	void __iomem *base;
 	int ext_cap_offset;
 	void __iomem *op_reg_base;
 	u32 val;
 	int timeout;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = ioremap_nocache(pci_resource_start(pdev, 0),
-				pci_resource_len(pdev, 0));
-	if (base == NULL)
-		return;
-
 	/*
 	 * Find the Legacy Support Capability register -
 	 * this is optional for xHCI host controllers.
@@ -462,6 +415,86 @@ hc_init:
 				"xHCI HW did not halt within %d usec "
 				"status = 0x%x\n", XHCI_MAX_HALT_USEC, val);
 	}
+}
+
+static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask)
+{
+	u16 cmd;
+	return !pci_read_config_word(pdev, PCI_COMMAND, &cmd) && (cmd & mask);
+}
+
+#define pio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_IO)
+#define mmio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_MEMORY)
+
+static void __devinit quirk_usb_handoff_uhci(struct pci_dev *pdev)
+{
+	unsigned long base = 0;
+	int i;
+
+	if (!pio_enabled(pdev))
+		return;
+
+	for (i = 0; i < PCI_ROM_RESOURCE; i++)
+		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
+			base = pci_resource_start(pdev, i);
+			break;
+		}
+
+	if (!base)
+		return;
+
+	uhci_check_and_reset_hc(pdev, base);
+}
+
+static int __devinit mmio_resource_enabled(struct pci_dev *pdev, int idx)
+{
+	return pci_resource_start(pdev, idx) && mmio_enabled(pdev);
+}
+
+static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_ohci(pdev, base);
+
+	iounmap(base);
+}
+
+static void __devinit quirk_usb_handoff_ehci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_ehci(pdev, base);
+
+	iounmap(base);
+}
+
+static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_xhci(pdev, base);
 
 	iounmap(base);
 }
@@ -473,7 +506,7 @@ static void __devinit quirk_usb_early_ha
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_OHCI)
 		quirk_usb_handoff_ohci(pdev);
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_EHCI)
-		quirk_usb_disable_ehci(pdev);
+		quirk_usb_handoff_ehci(pdev);
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
 		quirk_usb_handoff_xhci(pdev);
 }

Index: linux-2.6/drivers/usb/host/pci-quirks.h
===================================================================

drivers/usb/host/pci-quirks.h line 8

Quoted 6 lines
drivers/usb/host/pci-quirks.h +3 -0
@@ -3,5 +3,11 @@
 
 void uhci_reset_hc(struct pci_dev *pdev, unsigned long base);
 int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base);
+void __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base);

External functions should never have "__" at the start of them, sorry.

thanks,

greg k-h

#45 lore raw

Re: [PATCH -v3 4/4] x86: usb handoff in early_quirk

In reply to Yinghai Lu
Quoted 10 lines · Yinghai Lu

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

So all of this work was for one warning that really doesn't mean much, if anything? That seems very pointless, wouldn't you agree?

Wouldn't it be nicer if the BIOS fixed the SMI code to not have such problems?

thanks,

greg k-h

#47 lore raw
In reply to Yinghai Lu
Quoted 4 lines · Yinghai Lu

Move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

I still fail to understand _why_ you are making these changes.

Exactly what problem do you currently have, on what hardware, that this patch set solves?

confused,

greg k-h

#39 lore raw

Re: [PATCH 2/4] x86: early_quirk check all dev/func in domain 0

In reply to Yinghai Lu
Quoted 2 lines · Yinghai Lu

go with all buses instead of bus0.

Why?

Quoted 1 line

also only check header type on func0.

Why?

Please be more verbose in your change log comments, it cuts down on emails like this one...

thanks,

greg k-h

author
#5 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

In reply to Greg KH
Quoted 36 lines · Greg KH
Quoted 22 lines · Yinghai Lu

So later could reuse them to do usb handoff much early for x86.

will make arch early code get MMIO BAR and do remapping itself.

(quoted diff) +0 -2
-v2: still keep pci_device *pdev as parameter according to BenH.
-v3: expose three functions that take *base instead of including .c file.
---
 drivers/usb/host/pci-quirks.c |  195 ++++++++++++++++++++++++------------------
 drivers/usb/host/pci-quirks.h |    6 +
 2 files changed, 120 insertions(+), 81 deletions(-)

Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================

drivers/usb/host/pci-quirks.c +0 -0
@@ -17,6 +17,19 @@
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"

drivers/usb/host/pci-quirks.c line 28

Quoted 10 lines
drivers/usb/host/pci-quirks.c +9 -0
 
+static void default_usb_handoff_udelay(unsigned long usecs)
+{
+	udelay(usecs);
+}
+
+static void __devinit default_usb_handoff_msleep(unsigned long msecs)
+{
+	msleep(msecs);
+}

What?

Quoted 2 lines

Why in the world would you not just call the real functions here? That's not acceptable, sorry.

for early access, can not use udelay yet, it will take some one. Also msleep will cause crash, because it needs scheduler there.

Quoted 6 lines

drivers/usb/host/pci-quirks.c line 32

Quoted 6 lines
drivers/usb/host/pci-quirks.c +4 -0
+
+void (*usb_handoff_udelay)(unsigned long) = default_usb_handoff_udelay;
+void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+			default_usb_handoff_msleep;

or do you mean use ..
+void (*usb_handoff_udelay)(unsigned long) = udelay; +void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+ msleep;

Quoted 11 lines

drivers/usb/host/pci-quirks.h line 8

Quoted 9 lines
drivers/usb/host/pci-quirks.h +3 -0
@@ -3,5 +3,11 @@
 
 void uhci_reset_hc(struct pci_dev *pdev, unsigned long base);
 int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base);
+void __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base);

External functions should never have "__" at the start of them, sorry.

will change that.

Thanks

Yinghai

author
#48 lore raw
In reply to Greg KH
Quoted 10 lines · Greg KH
Quoted 4 lines · Yinghai Lu

Move quirks for usb handoff early for x86.

So we can get stable result when trying to calibrate apic timer with pm-timer.

I still fail to understand _why_ you are making these changes.

Exactly what problem do you currently have, on what hardware, that this patch set solves?

1. remove the warning
2. apic timer clockevent will be more accurate instead of use PM to calibrate it.

on Intel newer system with 128 logical cpus or more.

guess SMI handler for USB legacy support may need more time coordinate between those cpus.

Thanks

Yinghai

author
#46 lore raw

Re: [PATCH -v3 4/4] x86: usb handoff in early_quirk

In reply to Greg KH
Quoted 12 lines · Greg KH
Quoted 10 lines · Yinghai Lu

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

Quoted 5 lines

So all of this work was for one warning that really doesn't mean much, if anything? That seems very pointless, wouldn't you agree?

Wouldn't it be nicer if the BIOS fixed the SMI code to not have such problems?

find detail info that calibration code from Thomas
http://lkml.indiana.edu/hypermail/linux/kernel/0703.2/0420.html

Thomas said:
The wrong calibration values are probably caused by SMM code trying to emulate a PS/2 keyboard from a (maybe connected or not) USB keyboard. This prohibits the accurate delivery of PIT interrupts, which are used to calibrate the local APIC timer. Unfortunately we have no way to disable this BIOS misfeature in the early boot process.
---

We could disable it early with x86 early_quirks.

Thanks

Yinghai

author
#40 lore raw

Re: [PATCH 2/4] x86: early_quirk check all dev/func in domain 0

In reply to Greg KH
Quoted 5 lines · Greg KH
Quoted 2 lines · Yinghai Lu

go with all buses instead of bus0.

Why?

in cause USB host controller is not on Bus0.

Quoted 4 lines
Quoted 2 lines

also only check header type on func0.

Why?

that is bug.

notice when there is 00:1a.0, 00:1a.1, 00:1a.2 00:1a.7

the quirk code will not be applied with 00:1a.2 and 00:1a.7

it will exit after checking 00:1a.1's header type.

Quoted 3 lines

Please be more verbose in your change log comments, it cuts down on emails like this one...

sure.

Thanks

Yinghai

#49 lore raw
In reply to Yinghai Lu
Quoted 5 lines · Yinghai Lu
Quoted 4 lines · Greg KH
Quoted 3 lines · Yinghai Lu

Move quirks for usb handoff early for x86.

Quoted 7 lines
Quoted 6 lines
Quoted 1 line

So we can get stable result when trying to calibrate apic timer with pm-timer.

I still fail to understand _why_ you are making these changes.

Exactly what problem do you currently have, on what hardware, that this patch set solves?

1. remove the warning

What warning?

Quoted 2 lines

2. apic timer clockevent will be more accurate instead of use PM to calibrate it. on Intel newer system with 128 logical cpus or more.

Is that a problem?

Quoted 1 line

guess SMI handler for USB legacy support may need more time coordinate between those cpus.

Are you sure?

And again, please post this in your messages, otherwise we are just guessing, and if we have to guess, we will error on the "ignore the patch" side.

You need to sell us on why we should accept these patches, so far, I see no real reason at all for them.

good luck,

greg k-h

#41 lore raw

Re: [PATCH 2/4] x86: early_quirk check all dev/func in domain 0

In reply to Yinghai Lu
Quoted 8 lines · Yinghai Lu
Quoted 5 lines · Greg KH
Quoted 2 lines · Yinghai Lu

go with all buses instead of bus0.

Why?

in cause USB host controller is not on Bus0.

Why would that be?

Again, spell it out in great detail exactly why you are doing this, not what you are doing.

Quoted 11 lines
Quoted 3 lines
Quoted 1 line

also only check header type on func0.

Why?

that is bug.

notice when there is 00:1a.0, 00:1a.1, 00:1a.2 00:1a.7

the quirk code will not be applied with 00:1a.2 and 00:1a.7

it will exit after checking 00:1a.1's header type.

Again, you need to be very descriptive, I still don't understand what you have fixed.

thanks,

greg k-h

#42 lore raw

Re: [PATCH 2/4] x86: early_quirk check all dev/func in domain 0

In reply to Yinghai Lu
Quoted 14 lines · Yinghai Lu
Quoted 5 lines · Greg KH
Quoted 2 lines · Yinghai Lu

go with all buses instead of bus0.

Why?

in cause USB host controller is not on Bus0.

Quoted 4 lines
Quoted 2 lines

also only check header type on func0.

Why?

Quoted 7 lines

that is bug.

notice when there is 00:1a.0, 00:1a.1, 00:1a.2 00:1a.7

the quirk code will not be applied with 00:1a.2 and 00:1a.7

it will exit after checking 00:1a.1's header type.

Also, if this is a real bug, then just fix this in one patch, separate from the other patches. Don't do more than one thing per patch.

thanks,

greg k-h

#6 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

In reply to Yinghai Lu
Quoted 37 lines · Yinghai Lu
Quoted 36 lines · Greg KH
Quoted 22 lines · Yinghai Lu

So later could reuse them to do usb handoff much early for x86.

will make arch early code get MMIO BAR and do remapping itself.

(quoted diff) +0 -2
-v2: still keep pci_device *pdev as parameter according to BenH.
-v3: expose three functions that take *base instead of including .c file.
---
 drivers/usb/host/pci-quirks.c |  195 ++++++++++++++++++++++++------------------
 drivers/usb/host/pci-quirks.h |    6 +
 2 files changed, 120 insertions(+), 81 deletions(-)

Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================

drivers/usb/host/pci-quirks.c +0 -0
@@ -17,6 +17,19 @@
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"

drivers/usb/host/pci-quirks.c line 28

Quoted 10 lines
drivers/usb/host/pci-quirks.c +9 -0
 
+static void default_usb_handoff_udelay(unsigned long usecs)
+{
+	udelay(usecs);
+}
+
+static void __devinit default_usb_handoff_msleep(unsigned long msecs)
+{
+	msleep(msecs);
+}

What?

Quoted 4 lines
Quoted 2 lines

Why in the world would you not just call the real functions here? That's not acceptable, sorry.

for early access, can not use udelay yet, it will take some one.

Then don't use it.

Quoted 1 line

Also msleep will cause crash, because it needs scheduler there.

Also, don't use it.

Review

Quoted 9 lines
Quoted 4 lines

drivers/usb/host/pci-quirks.c line 32

Quoted 4 lines
drivers/usb/host/pci-quirks.c +4 -0
+
+void (*usb_handoff_udelay)(unsigned long) = default_usb_handoff_udelay;
+void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+			default_usb_handoff_msleep;

or do you mean use ..

(quoted diff) +3 -0
+void (*usb_handoff_udelay)(unsigned long) = udelay;
+void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+			msleep;

Yes, only if I could understand _why_ you are doing such a thing, when you do have access to these function pointers anyway, right?

confused,

greg k-h

#37 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

In reply to Yinghai Lu
Quoted 7 lines · Yinghai Lu

Index: linux-2.6/drivers/usb/host/pci-quirks.c
===================================================================

drivers/usb/host/pci-quirks.c +0 -0
@@ -17,6 +17,19 @@
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"

drivers/usb/host/pci-quirks.c line 28

Quoted 10 lines
drivers/usb/host/pci-quirks.c +9 -0
 
+static void default_usb_handoff_udelay(unsigned long usecs)
+{
+	udelay(usecs);
+}
+
+static void __devinit default_usb_handoff_msleep(unsigned long msecs)
+{
+	msleep(msecs);
+}

Use system_state or something like that here. The function pointer stuff is gross.

Quoted 161 lines
drivers/usb/host/pci-quirks.c +18 -63
+void (*usb_handoff_udelay)(unsigned long) = default_usb_handoff_udelay;
+void (*usb_handoff_msleep)(unsigned long) __devinitdata =
+			default_usb_handoff_msleep;
 
 #define UHCI_USBLEGSUP		0xc0		/* legacy support */
 #define UHCI_USBCMD		0		/* command register */
@@ -71,7 +84,7 @@ void uhci_reset_hc(struct pci_dev *pdev,
 	 */
 	outw(UHCI_USBCMD_HCRESET, base + UHCI_USBCMD);
 	mb();
-	udelay(5);
+	(*usb_handoff_udelay)(5);
 	if (inw(base + UHCI_USBCMD) & UHCI_USBCMD_HCRESET)
 		dev_warn(&pdev->dev, "HCRESET not completed yet!\n");
 
@@ -106,78 +119,38 @@ int uhci_check_and_reset_hc(struct pci_d
 	 */
 	pci_read_config_word(pdev, UHCI_USBLEGSUP, &legsup);
 	if (legsup & ~(UHCI_USBLEGSUP_RO | UHCI_USBLEGSUP_RWC)) {
-		dev_dbg(&pdev->dev, "%s: legsup = 0x%04x\n",
-				__func__, legsup);
+		dev_printk(KERN_DEBUG, &pdev->dev,
+				 "legsup = 0x%04x\n", legsup);
 		goto reset_needed;
 	}
 
 	cmd = inw(base + UHCI_USBCMD);
 	if ((cmd & UHCI_USBCMD_RUN) || !(cmd & UHCI_USBCMD_CONFIGURE) ||
 			!(cmd & UHCI_USBCMD_EGSM)) {
-		dev_dbg(&pdev->dev, "%s: cmd = 0x%04x\n",
-				__func__, cmd);
+		dev_printk(KERN_DEBUG, &pdev->dev, "cmd = 0x%04x\n", cmd);
 		goto reset_needed;
 	}
 
 	intr = inw(base + UHCI_USBINTR);
 	if (intr & (~UHCI_USBINTR_RESUME)) {
-		dev_dbg(&pdev->dev, "%s: intr = 0x%04x\n",
-				__func__, intr);
+		dev_printk(KERN_DEBUG, &pdev->dev, "intr = 0x%04x\n", intr);
 		goto reset_needed;
 	}
 	return 0;
 
 reset_needed:
-	dev_dbg(&pdev->dev, "Performing full reset\n");
+	dev_printk(KERN_DEBUG, &pdev->dev, "Performing full reset\n");
+
 	uhci_reset_hc(pdev, base);
+
 	return 1;
 }
 EXPORT_SYMBOL_GPL(uhci_check_and_reset_hc);
 
-static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask)
-{
-	u16 cmd;
-	return !pci_read_config_word(pdev, PCI_COMMAND, &cmd) && (cmd & mask);
-}
-
-#define pio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_IO)
-#define mmio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_MEMORY)
-
-static void __devinit quirk_usb_handoff_uhci(struct pci_dev *pdev)
-{
-	unsigned long base = 0;
-	int i;
-
-	if (!pio_enabled(pdev))
-		return;
-
-	for (i = 0; i < PCI_ROM_RESOURCE; i++)
-		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
-			base = pci_resource_start(pdev, i);
-			break;
-		}
-
-	if (base)
-		uhci_check_and_reset_hc(pdev, base);
-}
-
-static int __devinit mmio_resource_enabled(struct pci_dev *pdev, int idx)
+void __devinit __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base)
 {
-	return pci_resource_start(pdev, idx) && mmio_enabled(pdev);
-}
-
-static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
-{
-	void __iomem *base;
 	u32 control;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = pci_ioremap_bar(pdev, 0);
-	if (base == NULL)
-		return;
-
 	control = readl(base + OHCI_CONTROL);
 
 /* On PA-RISC, PDC can leave IR set incorrectly; ignore it there. */
@@ -193,7 +166,7 @@ static void __devinit quirk_usb_handoff_
 		while (wait_time > 0 &&
 				readl(base + OHCI_CONTROL) & OHCI_CTRL_IR) {
 			wait_time -= 10;
-			msleep(10);
+			(*usb_handoff_msleep)(10);
 		}
 		if (wait_time <= 0)
 			dev_warn(&pdev->dev, "OHCI: BIOS handoff failed"
@@ -210,26 +183,17 @@ static void __devinit quirk_usb_handoff_
 	 */
 	writel(~(u32)0, base + OHCI_INTRDISABLE);
 	writel(~(u32)0, base + OHCI_INTRSTATUS);
-
-	iounmap(base);
 }
 
-static void __devinit quirk_usb_disable_ehci(struct pci_dev *pdev)
+void __devinit __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base)
 {
 	int wait_time, delta;
-	void __iomem *base, *op_reg_base;
+	void *op_reg_base;
 	u32	hcc_params, val;
 	u8	offset, cap_length;
 	int	count = 256/4;
 	int	tried_handoff = 0;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = pci_ioremap_bar(pdev, 0);
-	if (base == NULL)
-		return;
-
 	cap_length = readb(base);
 	op_reg_base = base + cap_length;
 
@@ -247,7 +211,8 @@ static void __devinit quirk_usb_disable_
 		switch (cap & 0xff) {
 		case 1:			/* BIOS/SMM/... handoff support */
 			if ((cap & EHCI_USBLEGSUP_BIOS)) {
-				dev_dbg(&pdev->dev, "EHCI: BIOS handoff\n");
+				dev_printk(KERN_DEBUG, &pdev->dev,
+						"EHCI: BIOS handoff\n");
 
 #if 0
 /* [email protected] reports that some systems need SMI forced on,
@@ -279,7 +244,7 @@ static void __devinit quirk_usb_disable_
 			msec = 1000;
 			while ((cap & EHCI_USBLEGSUP_BIOS) && (msec > 0)) {
 				tried_handoff = 1;
-				msleep(10);
+				(*usb_handoff_msleep)(10);
 				msec -= 10;
 				pci_read_config_dword(pdev, offset, &cap);
 			}
@@ -330,18 +295,15 @@ static void __devinit quirk_usb_disable_
 		delta = 100;

drivers/usb/host/pci-quirks.c static void __devinit quirk_usb_disable_ · line 338

Quoted 10 lines
drivers/usb/host/pci-quirks.c +2 -3
 		do {
 			writel(0x3f, op_reg_base + EHCI_USBSTS);
-			udelay(delta);
+			(*usb_handoff_udelay)(delta);
 			wait_time -= delta;
 			val = readl(op_reg_base + EHCI_USBSTS);
-			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED)) {
+			if ((val == ~(u32)0) || (val & EHCI_USBSTS_HALTED))
 				break;
-			}

Don't mix-in cosmetic changes.

Quoted 168 lines
drivers/usb/host/pci-quirks.c +84 -15
 		} while (wait_time > 0);
 	}
 	writel(0, op_reg_base + EHCI_USBINTR);
 	writel(0x3f, op_reg_base + EHCI_USBSTS);
-
-	iounmap(base);
 }
 
 /*
@@ -357,7 +319,7 @@ static void __devinit quirk_usb_disable_
  * Returns -ETIMEDOUT if this condition is not true after
  * wait_usec microseconds have passed.
  */
-static int handshake(void __iomem *ptr, u32 mask, u32 done,
+static int __devinit handshake(void __iomem *ptr, u32 mask, u32 done,
 		int wait_usec, int delay_usec)
 {
 	u32	result;
@@ -367,7 +329,7 @@ static int handshake(void __iomem *ptr,
 		result &= mask;
 		if (result == done)
 			return 0;
-		udelay(delay_usec);
+		(*usb_handoff_udelay)(delay_usec);
 		wait_usec -= delay_usec;
 	} while (wait_usec > 0);
 	return -ETIMEDOUT;
@@ -381,22 +343,13 @@ static int handshake(void __iomem *ptr,
  * and then waits 5 seconds for the BIOS to hand over control.
  * If we timeout, assume the BIOS is broken and take control anyway.
  */
-static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base)
 {
-	void __iomem *base;
 	int ext_cap_offset;
 	void __iomem *op_reg_base;
 	u32 val;
 	int timeout;
 
-	if (!mmio_resource_enabled(pdev, 0))
-		return;
-
-	base = ioremap_nocache(pci_resource_start(pdev, 0),
-				pci_resource_len(pdev, 0));
-	if (base == NULL)
-		return;
-
 	/*
 	 * Find the Legacy Support Capability register -
 	 * this is optional for xHCI host controllers.
@@ -462,6 +415,86 @@ hc_init:
 				"xHCI HW did not halt within %d usec "
 				"status = 0x%x\n", XHCI_MAX_HALT_USEC, val);
 	}
+}
+
+static inline int io_type_enabled(struct pci_dev *pdev, unsigned int mask)
+{
+	u16 cmd;
+	return !pci_read_config_word(pdev, PCI_COMMAND, &cmd) && (cmd & mask);
+}
+
+#define pio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_IO)
+#define mmio_enabled(dev) io_type_enabled(dev, PCI_COMMAND_MEMORY)
+
+static void __devinit quirk_usb_handoff_uhci(struct pci_dev *pdev)
+{
+	unsigned long base = 0;
+	int i;
+
+	if (!pio_enabled(pdev))
+		return;
+
+	for (i = 0; i < PCI_ROM_RESOURCE; i++)
+		if ((pci_resource_flags(pdev, i) & IORESOURCE_IO)) {
+			base = pci_resource_start(pdev, i);
+			break;
+		}
+
+	if (!base)
+		return;
+
+	uhci_check_and_reset_hc(pdev, base);
+}
+
+static int __devinit mmio_resource_enabled(struct pci_dev *pdev, int idx)
+{
+	return pci_resource_start(pdev, idx) && mmio_enabled(pdev);
+}
+
+static void __devinit quirk_usb_handoff_ohci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_ohci(pdev, base);
+
+	iounmap(base);
+}
+
+static void __devinit quirk_usb_handoff_ehci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_ehci(pdev, base);
+
+	iounmap(base);
+}
+
+static void __devinit quirk_usb_handoff_xhci(struct pci_dev *pdev)
+{
+	void __iomem *base;
+
+	if (!mmio_resource_enabled(pdev, 0))
+		return;
+
+	base = pci_ioremap_bar(pdev, 0);
+	if (base == NULL)
+		return;
+
+	__usb_handoff_xhci(pdev, base);
 
 	iounmap(base);
 }
@@ -473,7 +506,7 @@ static void __devinit quirk_usb_early_ha
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_OHCI)
 		quirk_usb_handoff_ohci(pdev);
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_EHCI)
-		quirk_usb_disable_ehci(pdev);
+		quirk_usb_handoff_ehci(pdev);
 	else if (pdev->class == PCI_CLASS_SERIAL_USB_XHCI)
 		quirk_usb_handoff_xhci(pdev);
 }

Index: linux-2.6/drivers/usb/host/pci-quirks.h
===================================================================

drivers/usb/host/pci-quirks.h +6 -1
@@ -3,5 +3,11 @@
 
 void uhci_reset_hc(struct pci_dev *pdev, unsigned long base);
 int uhci_check_and_reset_hc(struct pci_dev *pdev, unsigned long base);
+void __usb_handoff_ohci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_ehci(struct pci_dev *pdev, void __iomem *base);
+void __usb_handoff_xhci(struct pci_dev *pdev, void __iomem *base);
+
+extern void (*usb_handoff_udelay)(unsigned long);
+extern void (*usb_handoff_msleep)(unsigned long);
 
 #endif  /*  __LINUX_USB_PCI_QUIRKS_H  */
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#7 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

In reply to Yinghai Lu
Quoted 3 lines · Yinghai Lu

for early access, can not use udelay yet, it will take some one. Also msleep will cause crash, because it needs scheduler there.

Right, and that's for such special cases (hopefully rare) that we have system_state... Much better than your function pointers I reckon.

We could even wrap it into a safe_delay() function or whatever (in fact why not make msleep() itself safe ? It's not like it was timing critical code :-)

Cheers,
Ben.

author
#8 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

Quoted 7 lines · Benjamin Herrenschmidt
Quoted 3 lines · Yinghai Lu

for early access, can not use udelay yet, it will take some one. Also msleep will cause crash, because it needs scheduler there.

Right, and that's for such special cases (hopefully rare) that we have system_state... Much better than your function pointers I reckon.

system_state does not work.
it only have BOOTING and RUNNING ...
RUNNING is set in init/main.c::init_post(). so early_quirk and pci_quirk all with BOOTING stage...

slab_is_available() could be used, but looks alike abuse.

Quoted 4 lines

We could even wrap it into a safe_delay() function or whatever (in fact why not make msleep() itself safe ? It's not like it was timing critical code :-)

like
void safe_udelay(unsigned long usecs)

{
	if (slab_is_available())
		udelay(usecs)
	else
		early_udelay(usecs);
}

or wonder if you are happy with

void __weak safe_udelay(unsigned long usecs)

{
	udelay(usecs);
}

and will have x86 have it's own safe_udelay...

Yinghai

#9 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

Quoted 5 lines · Yinghai Lu

system_state does not work.
it only have BOOTING and RUNNING ...
RUNNING is set in init/main.c::init_post(). so early_quirk and pci_quirk all with BOOTING stage...

Or we can make msleep() itself safe...

Ben.

Quoted 26 lines

slab_is_available() could be used, but looks alike abuse.

Quoted 2 lines

We could even wrap it into a safe_delay() function or whatever (in

fact

Quoted 1 line

why not make msleep() itself safe ? It's not like it was timing

critical

Quoted 1 line

code :-)

like
void safe_udelay(unsigned long usecs)

{
        if (slab_is_available())
                udelay(usecs)
        else
                early_udelay(usecs);
}

or wonder if you are happy with

void __weak safe_udelay(unsigned long usecs)

{
        udelay(usecs);
}

and will have x86 have it's own safe_udelay...

author
#10 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

Quoted 8 lines · Benjamin Herrenschmidt
Quoted 5 lines · Yinghai Lu

system_state does not work.
it only have BOOTING and RUNNING ...
RUNNING is set in init/main.c::init_post(). so early_quirk and pci_quirk all with BOOTING stage...

Or we can make msleep() itself safe...

still think the function pointer is more clean and safe.

Yinghai

#11 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

Quoted 4 lines · Yinghai Lu
Quoted 2 lines · Benjamin Herrenschmidt
Quoted 1 line · Yinghai Lu
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

system_state does not work.
it only have BOOTING and RUNNING ...
RUNNING is set in init/main.c::init_post(). so early_quirk and pci_quirk all with BOOTING stage...

Or we can make msleep() itself safe...

still think the function pointer is more clean and safe.

I do not.

#12 lore raw

Re: [PATCH -v3 1/4] pci, usb: Make usb handoff func all take base remapping

Quoted 5 lines · Greg KH
Quoted 4 lines · Yinghai Lu
Quoted 2 lines · Benjamin Herrenschmidt
Quoted 1 line · Yinghai Lu
Quoted 8 lines
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines

system_state does not work.
it only have BOOTING and RUNNING ...
RUNNING is set in init/main.c::init_post(). so early_quirk and pci_quirk all with BOOTING stage...

Or we can make msleep() itself safe...

still think the function pointer is more clean and safe.

You could use the pvops interface. Similar to how it patches the SMP locking code if it finds to be running under UP.

But that might be way to complex for this issue.

author
#13 lore raw

RFC PATCH 1/1 x86: Add safe_udelay() and safe_msleep()

In reply to Greg KH

We need to use those function in early-quirk stage with code that is shared with later stage.

for x86, normal udelay() will need to wait per_cpu(cpu_info) is allocated... that i after smp_prepare_cpus(), because it need to use percpu.loops_per_jiffy.

Also msleep() will need to wait schedular is ready.

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

This patch will set safe_udelay to early in x86 early arch code, and then init/main.c will set them back.

---
 arch/x86/include/asm/delay.h |    3 +++
 arch/x86/kernel/setup.c      |    4 ++++
 arch/x86/lib/delay.c         |   28 ++++++++++++++++++++++++++++
 include/linux/delay.h        |    2 ++
 init/main.c                  |    6 ++++++
 5 files changed, 43 insertions(+)
arch/x86/include/asm/delay.h +3 -0
Index: linux-2.6/arch/x86/include/asm/delay.h
===================================================================
@@ -28,4 +28,7 @@ extern void __delay(unsigned long loops)
 
 void use_tsc_delay(void);
 
+extern void __early_udelay(unsigned long usecs);
+extern void __early_msleep(unsigned int msecs);
+
 #endif /* _ASM_X86_DELAY_H */
arch/x86/kernel/setup.c +4 -0
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
@@ -853,6 +853,10 @@ void __init setup_arch(char **cmdline_p)
 
 	dmi_scan_machine();
 
+	/* for early using */
+	safe_udelay = __early_udelay;
+	safe_msleep = __early_msleep;
+
 	/*
 	 * VMware detection requires dmi to be available, so this
 	 * needs to be done after dmi_scan_machine, for the BP.
arch/x86/lib/delay.c +28 -0
Index: linux-2.6/arch/x86/lib/delay.c
===================================================================
@@ -138,3 +138,31 @@ void __ndelay(unsigned long nsecs)
 	__const_udelay(nsecs * 0x00005); /* 2**32 / 1000000000 (rounded up) */
 }
 EXPORT_SYMBOL(__ndelay);
+
+/* before cpu_info.loops_per_jiffy get set */
+static inline void __early_const_udelay(unsigned long xloops)
+{
+	int d0;
+
+	xloops *= 4;
+	asm("mull %%edx"
+		: "=d" (xloops), "=&a" (d0)
+		: "1" (xloops), "0"
+		(loops_per_jiffy * (HZ/4)));
+
+	delay_loop(++xloops);
+}
+
+/* usecs need to < 2000 */
+void __init __early_udelay(unsigned long usecs)
+{
+	/* 2**32 / 1000000 (rounded up) */
+	__early_const_udelay(usecs * 0x000010c7);
+}
+
+/* before schedular is there */
+void __init __early_msleep(unsigned int msecs)
+{
+	while (msecs--)
+		__early_udelay(1000);
+}
include/linux/delay.h +2 -0
Index: linux-2.6/include/linux/delay.h
===================================================================
@@ -52,4 +52,6 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+extern void (*safe_udelay)(unsigned long);
+extern void (*safe_msleep)(unsigned int);
 #endif /* defined(_LINUX_DELAY_H) */
init/main.c +6 -0
Index: linux-2.6/init/main.c
===================================================================
@@ -240,6 +240,9 @@ unsigned long loops_per_jiffy = (1<<12);
 
 EXPORT_SYMBOL(loops_per_jiffy);
 
+void (*safe_udelay)(unsigned long) = __udelay;
+void (*safe_msleep)(unsigned int) = msleep;
+
 static int __init debug_kernel(char *str)
 {
 	console_loglevel = 10;
@@ -879,6 +882,9 @@ static int __init kernel_init(void * unu
 	cad_pid = task_pid(current);
 
 	smp_prepare_cpus(setup_max_cpus);
+	/* set them back, x86 use it for early delay*/
+	safe_udelay = __udelay;
+	safe_msleep = msleep;
 
 	do_pre_smp_initcalls();
 	lockup_detector_init();
#14 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 6 lines · Yinghai Lu

We need to use those function in early-quirk stage with code that is shared with later stage.

for x86, normal udelay() will need to wait per_cpu(cpu_info) is allocated... that i after smp_prepare_cpus(), because it need to use percpu.loops_per_jiffy.

Quoted 7 lines

Also msleep() will need to wait schedular is ready.

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

This patch will set safe_udelay to early in x86 early arch code, and then init/main.c will set them back.

I still think it's better to just make msleep() work with and without scheduler and avoid having to bother with a new API

Cheers,
Ben.

Quoted 108 lines
---
 arch/x86/include/asm/delay.h |    3 +++
 arch/x86/kernel/setup.c      |    4 ++++
 arch/x86/lib/delay.c         |   28 ++++++++++++++++++++++++++++
 include/linux/delay.h        |    2 ++
 init/main.c                  |    6 ++++++
 5 files changed, 43 insertions(+)

Index: linux-2.6/arch/x86/include/asm/delay.h
===================================================================

arch/x86/include/asm/delay.h +3 -0
@@ -28,4 +28,7 @@ extern void __delay(unsigned long loops)
 
 void use_tsc_delay(void);
 
+extern void __early_udelay(unsigned long usecs);
+extern void __early_msleep(unsigned int msecs);
+
 #endif /* _ASM_X86_DELAY_H */

Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================

arch/x86/kernel/setup.c +4 -0
@@ -853,6 +853,10 @@ void __init setup_arch(char **cmdline_p)
 
 	dmi_scan_machine();
 
+	/* for early using */
+	safe_udelay = __early_udelay;
+	safe_msleep = __early_msleep;
+
 	/*
 	 * VMware detection requires dmi to be available, so this
 	 * needs to be done after dmi_scan_machine, for the BP.

Index: linux-2.6/arch/x86/lib/delay.c
===================================================================

arch/x86/lib/delay.c +28 -0
@@ -138,3 +138,31 @@ void __ndelay(unsigned long nsecs)
 	__const_udelay(nsecs * 0x00005); /* 2**32 / 1000000000 (rounded up) */
 }
 EXPORT_SYMBOL(__ndelay);
+
+/* before cpu_info.loops_per_jiffy get set */
+static inline void __early_const_udelay(unsigned long xloops)
+{
+	int d0;
+
+	xloops *= 4;
+	asm("mull %%edx"
+		: "=d" (xloops), "=&a" (d0)
+		: "1" (xloops), "0"
+		(loops_per_jiffy * (HZ/4)));
+
+	delay_loop(++xloops);
+}
+
+/* usecs need to < 2000 */
+void __init __early_udelay(unsigned long usecs)
+{
+	/* 2**32 / 1000000 (rounded up) */
+	__early_const_udelay(usecs * 0x000010c7);
+}
+
+/* before schedular is there */
+void __init __early_msleep(unsigned int msecs)
+{
+	while (msecs--)
+		__early_udelay(1000);
+}

Index: linux-2.6/include/linux/delay.h
===================================================================

include/linux/delay.h +2 -0
@@ -52,4 +52,6 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+extern void (*safe_udelay)(unsigned long);
+extern void (*safe_msleep)(unsigned int);
 #endif /* defined(_LINUX_DELAY_H) */

Index: linux-2.6/init/main.c
===================================================================

init/main.c +6 -0
@@ -240,6 +240,9 @@ unsigned long loops_per_jiffy = (1<<12);
 
 EXPORT_SYMBOL(loops_per_jiffy);
 
+void (*safe_udelay)(unsigned long) = __udelay;
+void (*safe_msleep)(unsigned int) = msleep;
+
 static int __init debug_kernel(char *str)
 {
 	console_loglevel = 10;
@@ -879,6 +882,9 @@ static int __init kernel_init(void * unu
 	cad_pid = task_pid(current);
 
 	smp_prepare_cpus(setup_max_cpus);
+	/* set them back, x86 use it for early delay*/
+	safe_udelay = __udelay;
+	safe_msleep = msleep;
 
 	do_pre_smp_initcalls();
 	lockup_detector_init();
#15 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 9 lines · Benjamin Herrenschmidt
Quoted 8 lines · Yinghai Lu

We need to use those function in early-quirk stage with code that is shared with later stage.

for x86, normal udelay() will need to wait per_cpu(cpu_info) is allocated... that i after smp_prepare_cpus(), because it need to use percpu.loops_per_jiffy.

Also msleep() will need to wait schedular is ready.

Quoted 8 lines
Quoted 5 lines

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

This patch will set safe_udelay to early in x86 early arch code, and then init/main.c will set them back.

I still think it's better to just make msleep() work with and without scheduler and avoid having to bother with a new API

I agree, this patch just makes things really complicated for no reason.

Again, I still fail to see why you are doing this patch series in the first place...

thanks,

greg k-h

author
#16 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Benjamin Herrenschmidt
Quoted 9 lines · Benjamin Herrenschmidt
Quoted 8 lines · Yinghai Lu

We need to use those function in early-quirk stage with code that is shared with later stage.

for x86, normal udelay() will need to wait per_cpu(cpu_info) is allocated... that i after smp_prepare_cpus(), because it need to use percpu.loops_per_jiffy.

Also msleep() will need to wait schedular is ready.

Quoted 8 lines
Quoted 5 lines

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

This patch will set safe_udelay to early in x86 early arch code, and then init/main.c will set them back.

I still think it's better to just make msleep() work with and without scheduler and avoid having to bother with a new API

please check if you are happy with this one.

[PATCH] x86: Make udelay() and msleep() can be used early

We need to use those functions in early-quirk stage with code that is shared with later stage.

For x86, normal udelay need to use percpu.loops_per_jiffy
will need to wait per_cpu(cpu_info) is allocated and copied from boot_cpu_data, after it is in smp_prepare_cpus() boot_cpu_data.loops_per_jiffy get set in identify_boot_cpu from check_bugs.

Also msleep() will need to wait schedular is ready.

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

-v2: use function pointer and keep old udelay() and msleep() API as requested from BenH

---
 arch/x86/lib/delay.c  |   28 +++++++++++++++++++++++++++-
 include/linux/delay.h |    2 ++
 init/main.c           |    3 +++
 kernel/timer.c        |   28 +++++++++++++++++++++++-----
 4 files changed, 55 insertions(+), 6 deletions(-)
arch/x86/lib/delay.c +27 -1
Index: linux-2.6/arch/x86/lib/delay.c
===================================================================
@@ -113,7 +113,7 @@ void __delay(unsigned long loops)
 }
 EXPORT_SYMBOL(__delay);
 
-inline void __const_udelay(unsigned long xloops)
+static void __normal_const_udelay(unsigned long xloops)
 {
 	int d0;
 
@@ -125,8 +125,34 @@ inline void __const_udelay(unsigned long
 
 	__delay(++xloops);
 }
+
+/* before percpu cpu_info.loops_per_jiffy is set */
+static void __init __early_const_udelay(unsigned long xloops)
+{
+	int d0;
+
+	xloops *= 4;
+	asm("mull %%edx"
+		: "=d" (xloops), "=&a" (d0)
+		: "1" (xloops), "0"
+		(loops_per_jiffy * (HZ/4)));
+
+	delay_loop(++xloops);
+}
+
+static void (*__const_udelay_fn)(unsigned long) = __early_const_udelay;
+
+void __const_udelay(unsigned long usecs)
+{
+	__const_udelay_fn(usecs);
+}
 EXPORT_SYMBOL(__const_udelay);
 
+void __init use_normal_delay(void)
+{
+	__const_udelay_fn = __normal_const_udelay;
+}
+
 void __udelay(unsigned long usecs)
 {
 	__const_udelay(usecs * 0x000010c7); /* 2**32 / 1000000 (rounded up) */
include/linux/delay.h +2 -0
Index: linux-2.6/include/linux/delay.h
===================================================================
@@ -52,4 +52,6 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+extern void use_normal_delay(void);
+extern void use_normal_sleep(void);
 #endif /* defined(_LINUX_DELAY_H) */
init/main.c +3 -0
Index: linux-2.6/init/main.c
===================================================================
@@ -879,6 +879,9 @@ static int __init kernel_init(void * unu
 	cad_pid = task_pid(current);
 
 	smp_prepare_cpus(setup_max_cpus);
+	/* set them back, x86 use it for early delay*/
+	use_normal_delay();
+	use_normal_sleep();
 
 	do_pre_smp_initcalls();
 	lockup_detector_init();
kernel/timer.c +23 -5
Index: linux-2.6/kernel/timer.c
===================================================================
@@ -1723,11 +1723,7 @@ void __init init_timers(void)
 	open_softirq(TIMER_SOFTIRQ, run_timer_softirq);
 }
 
-/**
- * msleep - sleep safely even with waitqueue interruptions
- * @msecs: Time in milliseconds to sleep for
- */
-void msleep(unsigned int msecs)
+static void __normal_msleep(unsigned int msecs)
 {
 	unsigned long timeout = msecs_to_jiffies(msecs) + 1;
 
@@ -1735,6 +1731,28 @@ void msleep(unsigned int msecs)
 		timeout = schedule_timeout_uninterruptible(timeout);
 }
 
+static void __init __early_msleep(unsigned int msecs)
+{
+	mdelay(msecs);
+}
+
+static void (*msleep_fn)(unsigned int) = __early_msleep;
+
+void __init use_normal_sleep(void)
+{
+	msleep_fn = __normal_msleep;
+}
+
+void __init __weak use_normal_delay(void) { }
+
+/**
+ * msleep - sleep safely even with waitqueue interruptions
+ * @msecs: Time in milliseconds to sleep for
+ */
+void msleep(unsigned int msecs)
+{
+	msleep_fn(msecs);
+}
 EXPORT_SYMBOL(msleep);
 
 /**
#17 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 111 lines · Yinghai Lu
Quoted 9 lines · Benjamin Herrenschmidt
Quoted 8 lines · Yinghai Lu

We need to use those function in early-quirk stage with code that is shared with later stage.

for x86, normal udelay() will need to wait per_cpu(cpu_info) is allocated... that i after smp_prepare_cpus(), because it need to use percpu.loops_per_jiffy.

Also msleep() will need to wait schedular is ready.

Quoted 8 lines
Quoted 5 lines

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

This patch will set safe_udelay to early in x86 early arch code, and then init/main.c will set them back.

I still think it's better to just make msleep() work with and without scheduler and avoid having to bother with a new API

please check if you are happy with this one.

[PATCH] x86: Make udelay() and msleep() can be used early

We need to use those functions in early-quirk stage with code that is shared with later stage.

For x86, normal udelay need to use percpu.loops_per_jiffy
will need to wait per_cpu(cpu_info) is allocated and copied from boot_cpu_data, after it is in smp_prepare_cpus() boot_cpu_data.loops_per_jiffy get set in identify_boot_cpu from check_bugs.

Also msleep() will need to wait schedular is ready.

Try to have one early version udelay that use loops_per_jiffy directly. and early msleep is just early delay.

-v2: use function pointer and keep old udelay() and msleep() API as requested from BenH

---
 arch/x86/lib/delay.c  |   28 +++++++++++++++++++++++++++-
 include/linux/delay.h |    2 ++
 init/main.c           |    3 +++
 kernel/timer.c        |   28 +++++++++++++++++++++++-----
 4 files changed, 55 insertions(+), 6 deletions(-)

Index: linux-2.6/arch/x86/lib/delay.c
===================================================================

arch/x86/lib/delay.c +27 -1
@@ -113,7 +113,7 @@ void __delay(unsigned long loops)
 }
 EXPORT_SYMBOL(__delay);
 
-inline void __const_udelay(unsigned long xloops)
+static void __normal_const_udelay(unsigned long xloops)
 {
 	int d0;
 
@@ -125,8 +125,34 @@ inline void __const_udelay(unsigned long
 
 	__delay(++xloops);
 }
+
+/* before percpu cpu_info.loops_per_jiffy is set */
+static void __init __early_const_udelay(unsigned long xloops)
+{
+	int d0;
+
+	xloops *= 4;
+	asm("mull %%edx"
+		: "=d" (xloops), "=&a" (d0)
+		: "1" (xloops), "0"
+		(loops_per_jiffy * (HZ/4)));
+
+	delay_loop(++xloops);
+}
+
+static void (*__const_udelay_fn)(unsigned long) = __early_const_udelay;
+
+void __const_udelay(unsigned long usecs)
+{
+	__const_udelay_fn(usecs);
+}
 EXPORT_SYMBOL(__const_udelay);
 
+void __init use_normal_delay(void)
+{
+	__const_udelay_fn = __normal_const_udelay;
+}
+
 void __udelay(unsigned long usecs)
 {
 	__const_udelay(usecs * 0x000010c7); /* 2**32 / 1000000 (rounded up) */

Index: linux-2.6/include/linux/delay.h
===================================================================

include/linux/delay.h +2 -0
@@ -52,4 +52,6 @@ static inline void ssleep(unsigned int s
 	msleep(seconds * 1000);
 }
 
+extern void use_normal_delay(void);
+extern void use_normal_sleep(void);
 #endif /* defined(_LINUX_DELAY_H) */

Index: linux-2.6/init/main.c
===================================================================

init/main.c static int __init kernel_init(void * unu · line 884

Quoted 6 lines
init/main.c +3 -0
@@ -879,6 +879,9 @@ static int __init kernel_init(void * unu
 	cad_pid = task_pid(current);
 
 	smp_prepare_cpus(setup_max_cpus);
+	/* set them back, x86 use it for early delay*/
+	use_normal_delay();
+	use_normal_sleep();

Ick, I really don't like this.

And, most importantly, I'm still not sure it's needed at all, as I don't agree with your previous patches that you say this one is needed for.

So, how about we work on the original root problem here before worrying about changing the main usleep logic for the whole kernel?

thanks,

greg k-h

#18 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 18 lines · Greg KH
Quoted 8 lines · Yinghai Lu
include/linux/delay.h +2 -0
+extern void use_normal_delay(void);
+extern void use_normal_sleep(void);
 #endif /* defined(_LINUX_DELAY_H) */

Index: linux-2.6/init/main.c
===================================================================

init/main.c static int __init kernel_init(void * unu · line 884

Quoted 6 lines
init/main.c +3 -0
@@ -879,6 +879,9 @@ static int __init kernel_init(void * unu
 	cad_pid = task_pid(current);
 
 	smp_prepare_cpus(setup_max_cpus);
+	/* set them back, x86 use it for early delay*/
+	use_normal_delay();
+	use_normal_sleep();

Ick, I really don't like this.

Quoted 5 lines

And, most importantly, I'm still not sure it's needed at all, as I don't agree with your previous patches that you say this one is needed for.

So, how about we work on the original root problem here before worrying about changing the main usleep logic for the whole kernel?

What the hell is the root problem ?

Thanks,

tglx

#19 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 19 lines · Thomas Gleixner
Quoted 18 lines · Greg KH
Quoted 8 lines · Yinghai Lu
include/linux/delay.h +2 -0
+extern void use_normal_delay(void);
+extern void use_normal_sleep(void);
 #endif /* defined(_LINUX_DELAY_H) */

Index: linux-2.6/init/main.c
===================================================================

init/main.c static int __init kernel_init(void * unu · line 884

Quoted 6 lines
init/main.c +3 -0
@@ -879,6 +879,9 @@ static int __init kernel_init(void * unu
 	cad_pid = task_pid(current);
 
 	smp_prepare_cpus(setup_max_cpus);
+	/* set them back, x86 use it for early delay*/
+	use_normal_delay();
+	use_normal_sleep();

Ick, I really don't like this.

Quoted 7 lines
Quoted 5 lines

And, most importantly, I'm still not sure it's needed at all, as I don't agree with your previous patches that you say this one is needed for.

So, how about we work on the original root problem here before worrying about changing the main usleep logic for the whole kernel?

What the hell is the root problem ?

I have yet to figure that out, even after reading the patches provided :(

confused,

greg k-h

author
#20 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Thomas Gleixner
Quoted 2 lines

What the hell is the root problem ?

for patch

x86: usb handoff in early_quirk

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

According to Thomas:

| http://lkml.indiana.edu/hypermail/linux/kernel/0703.2/0420.html
| 
| The wrong calibration values are probably caused by SMM code trying to
| emulate a PS/2 keyboard from a (maybe connected or not) USB keyboard.
| This prohibits the accurate delivery of PIT interrupts, which are used
| to calibrate the local APIC timer. Unfortunately we have no way to
| disable this BIOS misfeature in the early boot process.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

reuse code from drivers/usb/host/pci-quirks.c

but pci-quirks.c is using udelay and msleep ...

And BenH does't want
1. if (early)...
2. include .c
3. new API new about safe_udelay/safe_msleep...

just want to keep the old udelay/mdelay.

Thanks

Yinghai

#21 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 29 lines · Yinghai Lu
Quoted 2 lines

What the hell is the root problem ?

for patch

x86: usb handoff in early_quirk

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

According to Thomas:

| http://lkml.indiana.edu/hypermail/linux/kernel/0703.2/0420.html
| 
| The wrong calibration values are probably caused by SMM code trying to
| emulate a PS/2 keyboard from a (maybe connected or not) USB keyboard.
| This prohibits the accurate delivery of PIT interrupts, which are used
| to calibrate the local APIC timer. Unfortunately we have no way to
| disable this BIOS misfeature in the early boot process.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

reuse code from drivers/usb/host/pci-quirks.c

Quoted 8 lines

but pci-quirks.c is using udelay and msleep ...

And BenH does't want
1. if (early)...
2. include .c
3. new API new about safe_udelay/safe_msleep...

just want to keep the old udelay/mdelay.

NAK. That's the total wrong thing to do.

Do we have any indication that we miscalibrate? If no, then were is the point of this ? If yes, then I it's way easier to fix that than doing all this ugly churn.

We deal with that SMI crap in the TSC calibration as well and I'd rather see a combined TSC/APIC calibration than all this fugliness.

Thanks,

tglx

#23 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Yinghai Lu
Quoted 5 lines · Yinghai Lu
Quoted 2 lines

What the hell is the root problem ?

for patch

Quoted 7 lines

x86: usb handoff in early_quirk

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

But is that causing a real problem? You never answered that question when I asked it in the past, which makes me think that all of this work is for no good reason.

thanks,

greg k-h

author
#24 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

there are some udelay() in drivers/usb/early/ehci-dbgp.c

that is for early USB debug port console support.

We should replace them with early version udelay()

Thanks

Yinghai

#25 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

Quoted 5 lines · Yinghai Lu

there are some udelay() in drivers/usb/early/ehci-dbgp.c

that is for early USB debug port console support.

We should replace them with early version udelay()

Or have a reasonable default for lpj ...

Ben.

#28 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Yinghai Lu
Quoted 5 lines · Yinghai Lu

there are some udelay() in drivers/usb/early/ehci-dbgp.c

that is for early USB debug port console support.

We should replace them with early version udelay()

Again, I fail to see why. They work fine today, right?

You _really_ need to prove what you are trying to do here, with lots of details and descriptions of what the problem is, as it's getting a bit annoying...

thanks,

greg k-h

author
#26 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Benjamin Herrenschmidt
Quoted 8 lines · Benjamin Herrenschmidt
Quoted 5 lines · Yinghai Lu

there are some udelay() in drivers/usb/early/ehci-dbgp.c

that is for early USB debug port console support.

We should replace them with early version udelay()

Or have a reasonable default for lpj ...

x86 udelay is using percpu cpu_data.loops_per_jiffy, and it is not set yet.

when x86 boot with earlyprintk=dbgp, that early console is loaded quite before setup_percpu_areas()

maybe something like this could workaround it, if cpu_data(0) can be used before setup_percpu_areas().

arch/x86/kernel/setup.c +1 -0
index d3cfe26..333694a 100644
@@ -734,6 +734,7 @@ void __init setup_arch(char **cmdline_p)
 
        early_trap_init();
        early_cpu_init();
+       cpu_data(0).loops_per_jiffy = loops_per_jiffy;
        early_ioremap_init();
 
        setup_olpc_ofw_pgd();
author
#29 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Greg KH
Quoted 8 lines · Greg KH
Quoted 5 lines · Yinghai Lu

there are some udelay() in drivers/usb/early/ehci-dbgp.c

that is for early USB debug port console support.

We should replace them with early version udelay()

Again, I fail to see why. They work fine today, right?

did not test that for a while.

from code review, we should not use udelay() that early.

in arch/x86/lib/delay.c we have
inline void __const_udelay(unsigned long xloops)

{
        int d0;

        xloops *= 4;
        asm("mull %%edx"
                :"=d" (xloops), "=&a" (d0)
                :"1" (xloops), "0"
                (this_cpu_read(cpu_info.loops_per_jiffy) * (HZ/4)));

        __delay(++xloops);
}

EXPORT_SYMBOL(__const_udelay);

that percpu cpu_info.loops_per_jiffy only have value after smp_prepare_cpus() is called.

Quoted 4 lines

You _really_ need to prove what you are trying to do here, with lots of details and descriptions of what the problem is, as it's getting a bit annoying...

sorry for that.

Yinghai

#27 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Yinghai Lu
Quoted 13 lines · Yinghai Lu
Quoted 1 line

Or have a reasonable default for lpj ...

x86 udelay is using percpu cpu_data.loops_per_jiffy, and it is not set yet.

when x86 boot with earlyprintk=dbgp, that early console is loaded quite before setup_percpu_areas()

maybe something like this could workaround it, if cpu_data(0) can be used before setup_percpu_areas().

arch/x86/kernel/setup.c +0 -0
index d3cfe26..333694a 100644

arch/x86/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 740

Quoted 7 lines
arch/x86/kernel/setup.c +1 -0
@@ -734,6 +734,7 @@ void __init setup_arch(char **cmdline_p)
 
        early_trap_init();
        early_cpu_init();
+       cpu_data(0).loops_per_jiffy = loops_per_jiffy;
        early_ioremap_init();
 
        setup_olpc_ofw_pgd();

That could work.

Acked-by H. Peter Anvin
#30 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Yinghai Lu
Quoted 4 lines · Yinghai Lu

from code review, we should not use udelay() that early.

in arch/x86/lib/delay.c we have
inline void __const_udelay(unsigned long xloops)

Quoted 8 lines
{
        int d0;

        xloops *= 4;
        asm("mull %%edx"
                :"=d" (xloops), "=&a" (d0)
                :"1" (xloops), "0"
                (this_cpu_read(cpu_info.loops_per_jiffy) * (HZ/4)));

^^^^ this line was recently changed through the commit included at the end of the message. Solution would be to revert the commit or set the loops_per_jiffy in the early per cpu segment (as done in the last patch I saw from you).

commit 357089fca91f639dd005ae0721f5f932b4f276ab

Author: Christoph Lameter <[email protected]>
Date:   Thu Dec 16 12:14:43 2010 -0600

x86: udelay: Use this_cpu_read to avoid address calculation

The code will use a segment prefix instead of doing the lookup and calculation.

arch/x86/lib/delay.c +0 -1
index ff485d3..fc45ba8 100644
@@ -121,7 +121,7 @@ inline void __const_udelay(unsigned long xloops)
        asm("mull %%edx"
                :"=d" (xloops), "=&a" (d0)
                :"1" (xloops), "0"
-               (cpu_data(raw_smp_processor_id()).loops_per_jiffy *

(HZ/4)));
+ (this_cpu_read(cpu_info.loops_per_jiffy) * (HZ/4)));

        __delay(++xloops);

}

author
#31 lore raw

PATCH 1/1 x86: set percpu cpu0 lpj to default

We need to use udelay() in early stage.

For x86, normally udelay need to use percpu.loops_per_jiffy will need to wait per_cpu(cpu_info) is allocated and copied from boot_cpu_data, after it is in smp_prepare_cpus() boot_cpu_data.loops_per_jiffy get set in identify_boot_cpu from check_bugs.

But cpu_data(0) can be referred early, that is in per cpu load data.

Try to set it with default vaule instead leave it as zero.

BTW: it it is some small...only 4096

Now: only user for udelay is ehci dgp early console.

---
 arch/x86/kernel/setup.c |    1 +
 1 file changed, 1 insertion(+)
arch/x86/kernel/setup.c +1 -0
Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
@@ -734,6 +734,7 @@ void __init setup_arch(char **cmdline_p)
 
 	early_trap_init();
 	early_cpu_init();
+	cpu_data(0).loops_per_jiffy = loops_per_jiffy;
 	early_ioremap_init();
 
 	setup_olpc_ofw_pgd();
#32 lore raw

Re: [PATCH] x86: set percpu cpu0 lpj to default

I think Tejun needs to look at this. There may be a better spot for this.

#35 lore raw

Re: [PATCH] x86: set percpu cpu0 lpj to default

In reply to Yinghai Lu
Quoted 2 lines · Yinghai Lu

We need to use udelay() in early stage.

I still don't agree with this.

Again, _please_ post your real patches that require this, after explaining exactly _why_ this is needed at all, and what real problem it is solving.

You seem to be totally ignoring this request I've made numerous times, which isn't appreciated at all. So I'm guessing that we should also ignore your patches as well?

thanks,

greg k-h

author
#36 lore raw

Re: [PATCH] x86: set percpu cpu0 lpj to default

Quoted 6 lines · Greg KH
Quoted 2 lines · Yinghai Lu

We need to use udelay() in early stage.

I still don't agree with this.

Quoted 7 lines

Again, _please_ post your real patches that require this, after explaining exactly _why_ this is needed at all, and what real problem it is solving.

You seem to be totally ignoring this request I've made numerous times, which isn't appreciated at all. So I'm guessing that we should also ignore your patches as well?

Please forget about those patches about disabling USB legacy early now.

the one is because

echi debug port early serial console in drivers/usb/early/ehci-dbgp.c

is using udelay() in early stage.

and now udelay is relying to use loops_per_jiffy in per cpu area that not set yet.

Yinghai

author
#22 lore raw

Re: [RFC PATCH] x86: Add safe_udelay() and safe_msleep()

In reply to Thomas Gleixner
Quoted 40 lines · Thomas Gleixner
Quoted 29 lines · Yinghai Lu
Quoted 2 lines

What the hell is the root problem ?

for patch

x86: usb handoff in early_quirk

some systems keep getting
APIC calibration not consistent with PM-Timer: 139ms instead of 100ms APIC delta adjusted to PM-Timer: 831249 (1163736)

USB legacy SMI handler is not disabled at that time.

According to Thomas:

| http://lkml.indiana.edu/hypermail/linux/kernel/0703.2/0420.html
| 
| The wrong calibration values are probably caused by SMM code trying to
| emulate a PS/2 keyboard from a (maybe connected or not) USB keyboard.
| This prohibits the accurate delivery of PIT interrupts, which are used
| to calibrate the local APIC timer. Unfortunately we have no way to
| disable this BIOS misfeature in the early boot process.

Try to disable USB legacy support early with this patch.
So later APIC Timer calibration don't get messed up by USB legacy support SMI handler. After this patch, that warning never show up for 100 reboot tests.

reuse code from drivers/usb/host/pci-quirks.c

Quoted 8 lines

but pci-quirks.c is using udelay and msleep ...

And BenH does't want
1. if (early)...
2. include .c
3. new API new about safe_udelay/safe_msleep...

just want to keep the old udelay/mdelay.

Quoted 8 lines

NAK. That's the total wrong thing to do.

Do we have any indication that we miscalibrate? If no, then were is the point of this ? If yes, then I it's way easier to fix that than doing all this ugly churn.

We deal with that SMI crap in the TSC calibration as well and I'd rather see a combined TSC/APIC calibration than all this fugliness.

maybe disabling USB legacy support early patches could help this one?

http://ubuntuforums.org/archive/index.php/t-1289119.html

| Recently changed motherboard and cpu to
| gigabyte ga-ma790xt-ud4p and amd phenom II x4 965.
|
| Sometimes (and I mean sometimes) ubuntu 9.04 does not recognize
| all cpu cores, but 1 (sometimes :) even 2 or 3).
| I have tried Karmic(9.10), and at first it recognized all cores, but now it
| is back to normal (random not recognizing).
...
| sgb   October 16th, 2009, 01:03 PM
| Thanks everyone,
| I have resolved the issue.
| Unbelievable as it is: four port USB hub was responsible
| for this random behavior. As soon as it is removed,
| everything is working as it should.
| I am amazed...
| But it's working now. :)
| If anyone has any theory how windows bypassed it,
| and why linux *sometimes* did, and sometimes did not,
| I would really like to hear... :confused:
| Thanks again for Your effort.
#33 lore raw

Re: [PATCH] x86: set percpu cpu0 lpj to default

In reply to Christoph Lameter

Hello,

It doesn't seem like Christoph's recent patches had anything to do with it. The problem is the default loops_per_jiffy has initial value of 1 << 12 but the per cpu one used on x86 starts as zero triggering warning if delay is used before calibration.

So, all that's necessary is just to initialize it with the same constant when defining the per cpu variable like the following. It probably would be better to define a named constant for it tho.

Thanks.

arch/x86/kernel/smpboot.c +4 -1
index 763df77..1898c70 100644
@@ -131,7 +131,10 @@ DEFINE_PER_CPU(cpumask_var_t, cpu_core_map);
 EXPORT_PER_CPU_SYMBOL(cpu_core_map);
 
 /* Per CPU bogomips and other parameters */
-DEFINE_PER_CPU_SHARED_ALIGNED(struct cpuinfo_x86, cpu_info);
+DEFINE_PER_CPU_SHARED_ALIGNED(struct cpuinfo_x86, cpu_info) =
+{
+	.loops_per_jiffy	= 1 << 12,
+};
 EXPORT_PER_CPU_SYMBOL(cpu_info);
 
 atomic_t init_deasserted;
Signature · 1 line
tejun
author
#34 lore raw

Re: [PATCH] x86: set percpu cpu0 lpj to default

Quoted 20 lines · Tejun Heo

Hello,

It doesn't seem like Christoph's recent patches had anything to do with it.  The problem is the default loops_per_jiffy has initial value of 1 << 12 but the per cpu one used on x86 starts as zero triggering warning if delay is used before calibration.

So, all that's necessary is just to initialize it with the same constant when defining the per cpu variable like the following.  It probably would be better to define a named constant for it tho.

Thanks.

arch/x86/kernel/smpboot.c +0 -0
index 763df77..1898c70 100644
@@ -131,7 +131,10 @@ DEFINE_PER_CPU(cpumask_var_t, cpu_core_map);
  EXPORT_PER_CPU_SYMBOL(cpu_core_map);

arch/x86/kernel/smpboot.c DEFINE_PER_CPU(cpumask_var_t, cpu_core_map); · line 140

Quoted 10 lines
arch/x86/kernel/smpboot.c +4 -1
 
  /* Per CPU bogomips and other parameters */
-DEFINE_PER_CPU_SHARED_ALIGNED(struct cpuinfo_x86, cpu_info);
+DEFINE_PER_CPU_SHARED_ALIGNED(struct cpuinfo_x86, cpu_info) =
+{
+       .loops_per_jiffy        = 1 << 12,
+};
  EXPORT_PER_CPU_SYMBOL(cpu_info);
 
  atomic_t init_deasserted;

yes, it should work.

Thanks

Yinghai