[PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
From: Bryan O'Donoghue
This patch is to enable USB host controller for Intel Quark X1000. Add pci
quirks
to adjust the packet buffer in/out threshold value, and ensure EHCI packet
buffer
i/o threshold value is reconfigured to half.
Signed-off-by: Bryan O'Donoghue
Signed-off-by: Alvin (Weike) Chen
---
drivers/usb/host/ehci-pci.c |4
drivers/usb/host/pci-quirks.c | 42 +
drivers/usb/host/pci-quirks.h |2 ++
3 files changed, 48 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..33cfa23 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -50,6 +50,10 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, "MWI active\n");
+ /* Reset the threshold limit */
+ if(unlikely(usb_is_intel_qrk(pdev)))
+ usb_set_qrk_bulk_thresh(pdev);
+
return 0;
}
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 00661d3..1ea8803 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -823,6 +823,48 @@ static int handshake(void __iomem *ptr, u32 mask, u32 done,
return -ETIMEDOUT;
}
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+bool usb_is_intel_qrk(struct pci_dev *pdev)
+{
+ return pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+
+}
+EXPORT_SYMBOL_GPL(usb_is_intel_qrk);
+
+#define EHCI_INSNREG01 0x84
+#define EHCI_INSNREG01_THRESH 0x007F007F /* Threshold value */
+void usb_set_qrk_bulk_thresh(struct pci_dev *pdev)
+{
+ void __iomem *base, *op_reg_base;
+ u8 cap_length;
+ u32 val;
+
+ 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;
+
+ val = readl(op_reg_base + EHCI_INSNREG01);
+ dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
+
+ val = EHCI_INSNREG01_THRESH;
+
+ writel(val, op_reg_base + EHCI_INSNREG01);
+
+ val = readl(op_reg_base + EHCI_INSNREG01);
+ dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
+
+ iounmap(base);
+
+}
+EXPORT_SYMBOL_GPL(usb_set_qrk_bulk_thresh);
+
/*
* Intel's Panther Point chipset has two host controllers (EHCI and xHCI) that
* share some number of ports. These ports can be switched between either
diff --git a/drivers/usb/host/pci-quirks.h b/drivers/usb/host/pci-quirks.h
index c622ddf..617c22b 100644
--- a/drivers/usb/host/pci-quirks.h
+++ b/drivers/usb/host/pci-quirks.h
@@ -14,6 +14,8 @@ void usb_amd_quirk_pll_enable(void);
void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev);
void usb_disable_xhci_ports(struct pci_dev *xhci_pdev);
void sb800_prefetch(struct device *dev, int on);
+bool usb_is_intel_qrk(struct pci_dev *pdev);
+void usb_set_qrk_bulk_thresh(struct pci_dev *pdev);
#else
struct pci_dev;
static inline void usb_amd_quirk_pll_disable(void) {}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: ehci-pci: Add support for Intel Quark X1000 USB host controller
From: "Alvin (Weike) Chen" Hi, Intel Quark X1000 consists of one USB host controller which can be PCI enumerated. But the exsiting EHCI-PCI framework doesn't support it. Thus, we enable it to support Intel Quark X1000 USB host controller by adding pci quirks to configure buffer i/o threshold value to half. Bryan O'Donoghue (1): Quark USB host drivers/usb/host/ehci-pci.c |4 drivers/usb/host/pci-quirks.c | 42 + drivers/usb/host/pci-quirks.h |2 ++ 3 files changed, 48 insertions(+) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
See my comments below:
> > + /* Reset the threshold limit */
> > + if(unlikely(usb_is_intel_qrk(pdev)))
>
> Please run your patch thru scripts/checkpatch.pl -- space needed after
> *if*.
OK, I will improve it in the PATCH v2.
>
> > + usb_set_qrk_bulk_thresh(pdev);
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/usb/host/pci-quirks.c
> > b/drivers/usb/host/pci-quirks.c index 00661d3..1ea8803 100644
> > --- a/drivers/usb/host/pci-quirks.c
> > +++ b/drivers/usb/host/pci-quirks.c
> > @@ -823,6 +823,48 @@ static int handshake(void __iomem *ptr, u32 mask,
> u32 done,
> > return -ETIMEDOUT;
> > }
> >
> > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
> > +bool usb_is_intel_qrk(struct pci_dev *pdev) {
> > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > +
> > +}
> > +EXPORT_SYMBOL_GPL(usb_is_intel_qrk);
>
> Why not use DECLARE_PCI_FIXUP_FINAL() instead?
>
Alan Stern suggests me to move 'usb_is_intel_qrk' and 'usb_set_qrk_bulk_thresh'
to ehci-pci.c.
Since no other modules call these two functions, I will move them to ehci-pci.c
as static functions, so no necessary to export them out.
> > +
> > +#define EHCI_INSNREG01 0x84
> > +#define EHCI_INSNREG01_THRESH 0x007F007F /* Threshold value */
> > +void usb_set_qrk_bulk_thresh(struct pci_dev *pdev) {
> > + void __iomem *base, *op_reg_base;
> > + u8 cap_length;
> > + u32 val;
> > +
> > + 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;
> > +
> > + val = readl(op_reg_base + EHCI_INSNREG01);
> > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
> > +
> > + val = EHCI_INSNREG01_THRESH;
> > +
> > + writel(val, op_reg_base + EHCI_INSNREG01);
> > +
> > + val = readl(op_reg_base + EHCI_INSNREG01);
> > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
>
> I doubt these dev_printk() calls are really useful. But if the are, it's
> worth
> merging them into one call.
Actually, the dev_printk calls is not necessary, I will remove them in the
PATCH v2.
>
> WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
See my comments below: > -Original Message- > From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org] > Sent: Tuesday, June 24, 2014 9:25 PM > To: Chen, Alvin > Cc: Alan Stern; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Ong, > Boon Leong > Subject: Re: [PATCH] USB: ehci-pci: USB host controller support for Intel > Quark > X1000 > > On Tue, Jun 24, 2014 at 08:51:43AM -0700, Chen, Alvin wrote: > > From: Bryan O'Donoghue > > > > This patch is to enable USB host controller for Intel Quark X1000. Add > > pci quirks to adjust the packet buffer in/out threshold value, and > > ensure EHCI packet buffer i/o threshold value is reconfigured to half. > > > > Signed-off-by: Bryan O'Donoghue > > Signed-off-by: Alvin (Weike) Chen > > --- > > drivers/usb/host/ehci-pci.c |4 > > drivers/usb/host/pci-quirks.c | 42 > + > > drivers/usb/host/pci-quirks.h |2 ++ > > 3 files changed, 48 insertions(+) > > > > diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c > > index 3e86bf4..33cfa23 100644 > > --- a/drivers/usb/host/ehci-pci.c > > +++ b/drivers/usb/host/ehci-pci.c > > @@ -50,6 +50,10 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct > pci_dev *pdev) > > if (!retval) > > ehci_dbg(ehci, "MWI active\n"); > > > > + /* Reset the threshold limit */ > > + if(unlikely(usb_is_intel_qrk(pdev))) > > Why unlikely()? Have you measured a speed difference here? And this is a > _very_ slow path, what does that speed difference you measured help with? > Actually, 'unlikely' is not necessary, I will remove it in PATCH v2. > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
> -Original Message-
> From: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> Sent: Tuesday, June 24, 2014 9:25 PM
> To: Chen, Alvin
> Cc: Alan Stern; linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Ong,
> Boon Leong
> Subject: Re: [PATCH] USB: ehci-pci: USB host controller support for Intel
> Quark
> X1000
>
> On Tue, Jun 24, 2014 at 08:51:43AM -0700, Chen, Alvin wrote:
> > diff --git a/drivers/usb/host/pci-quirks.c
> > b/drivers/usb/host/pci-quirks.c index 00661d3..1ea8803 100644
> > --- a/drivers/usb/host/pci-quirks.c
> > +++ b/drivers/usb/host/pci-quirks.c
> > @@ -823,6 +823,48 @@ static int handshake(void __iomem *ptr, u32 mask,
> u32 done,
> > return -ETIMEDOUT;
> > }
> >
> > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
> > +bool usb_is_intel_qrk(struct pci_dev *pdev) {
> > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > +
> > +}
> > +EXPORT_SYMBOL_GPL(usb_is_intel_qrk);
>
> There is no lack of vowels available to you, please use them.
>
OK, I will change ' usb_is_intel_qrk ' to ' usb_is_intel_quark'.
> greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
> > This patch is to enable USB host controller for Intel Quark X1000. Add
> > pci quirks to adjust the packet buffer in/out threshold value, and
> > ensure EHCI packet buffer i/o threshold value is reconfigured to half.
>
> Please add more detailed description. For example, why is it necessary to
> reconfigure the threshold value?
>
This patch try to reconfigure the threshold value as maximal (0x7F DWord) as
possible since the default value is just 0x20 DWord, and I will update the
description.
> >
> > +
> > +#define EHCI_INSNREG01 0x84
> > +#define EHCI_INSNREG01_THRESH 0x007F007F /* Threshold value */
>
> What does this magic number mean?
> Would you make it more readable?
0x84 is the register offset, and the high word of '0x007F007F' is the out
threshold and the low word is the in threshold. I will use some micros to make
it more readable to avoid magic number in PATCH v2.
> > +void usb_set_qrk_bulk_thresh(struct pci_dev *pdev) {
> > + void __iomem *base, *op_reg_base;
> > + u8 cap_length;
> > + u32 val;
> > +
> > + 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;
> > +
> > + val = readl(op_reg_base + EHCI_INSNREG01);
> > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
> > +
> > + val = EHCI_INSNREG01_THRESH;
> > +
> > + writel(val, op_reg_base + EHCI_INSNREG01);
> > +
> > + val = readl(op_reg_base + EHCI_INSNREG01);
> > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val);
>
> As Alan Stern said, These INFO message are noisy.
> DEBUG level looks better.
These messages are not necessary, I will remove them.
> Best regards,
> Jingoo Han
>
> > +
> > --
> > 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
> > > > OK, I will change ' usb_is_intel_qrk ' to ' usb_is_intel_quark'. > > Or even usb_is_intel_quark_x1000() ? > OK, I will change the function name as your suggestion to make it more specific. > David > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: ehci-pci: USB host controller support for Intel Quark X1000
> > This patch is to enable USB host controller for Intel Quark X1000. Add >> pci quirks to adjust the packet buffer in/out threshold value, and > >ensure EHCI packet buffer i/o threshold value is reconfigured to half > > > What is the packet buffer in/out threshold value and why does it need to be > reconfigured to half? > I go through the hardware specification carefully again. The EHCI buffer packet depth can be 512 bytes, and the in/out threshold is programmable and can be programmed to 512 bytes (0x80 DWord) only when isochronous/interrupt transactions are not initiated by the host controller. The default threshold value for Quark X1000 is 0x20 DWord, so this patch try to program it as maximal as possible, and it is 0x7F Dword since the host controller initiates isochronous/interrupt transactions . I will update the description in PATCH v2. > > + val = readl(op_reg_base + EHCI_INSNREG01); > > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val); > > + > > + val = EHCI_INSNREG01_THRESH; > > + > > + writel(val, op_reg_base + EHCI_INSNREG01); > > + > > + val = readl(op_reg_base + EHCI_INSNREG01); > > + dev_printk(KERN_INFO, &pdev->dev, "INSNREG01 is 0x%08x\n", val); > > What good will these log messages do anybody? Is there any reason not to > make them debug messages? Or even leave them out entirely, since you > pretty much know beforehand what they're going to say? > These messages are not necessary, I will remove them. > > + > > + iounmap(base); > > + > > +} > > +EXPORT_SYMBOL_GPL(usb_set_qrk_bulk_thresh); > > None of this material belongs in pci-quirks.c. Please move it into > ehci-pci.c. > OK. I will move them to ehci-pci.c, since these two functions will not be called by other modules. > Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] USB: ehci-pci: USB host controller support for Intel Quark X1000
From: Bryan O'Donoghue
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host
controller, and the default value is 0x20 dwords. The in/out threshold can be
programmed
to 0x80 dwords, but only when isochronous/interrupt transactions are not
initiated by
the USB host controller. This patch is to reconfigure the packet buffer in/out
threshold
as maximal as possible, and it is 0x7F dwords since the USB host controller
initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue
Signed-off-by: Alvin (Weike) Chen
---
changelog v2:
*Change the function name from 'usb_is_intel_qrk' to 'usb_is_intel_quark_x1000'.
*Move the functions 'usb_is_intel_quark_x1000' and 'usb_set_qrk_bulk_thresh'
from 'pci-quirks.c' to 'ehci-pci.c'.
*Remove unnecessary kernel message in the function of 'usb_set_qrk_bulk_thresh'.
*Remove 'unlikely' in the functions of 'ehci_pci_reinit'.
*Add white space after 'if'.
*Update the descriptions to make it more clearly.
*Add Micros to avoid magic number.
drivers/usb/host/ehci-pci.c | 45 +++
1 file changed, 45 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..ca29f34 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,6 +35,47 @@ static const char hcd_name[] = "ehci-pci";
#define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool usb_is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+
+}
+
+/* Register offset of in/out threshold */
+#define EHCI_INSNREG01 0x84
+/* The maximal threshold is 0x80 Dword */
+#define EHCI_MAX_THRESHOLD 0X80
+/* Lower word is 'in' threshold, and higher word is 'out' threshold*/
+#define EHCI_INSNREG01_THRESH \
+ ((EHCI_MAX_THRESHOLD - 1)<<16 | (EHCI_MAX_THRESHOLD - 1))
+static void usb_set_qrk_bulk_thresh(struct pci_dev *pdev)
+{
+ void __iomem *base, *op_reg_base;
+ u8 cap_length;
+ u32 val;
+ u16 cmd;
+
+ if (!pci_resource_start(pdev, 0))
+ return;
+
+ if (pci_read_config_word(pdev, PCI_COMMAND, &cmd)
+ || !(cmd & PCI_COMMAND_MEMORY))
+ return;
+
+ base = pci_ioremap_bar(pdev, 0);
+ if (base == NULL)
+ return;
+
+ cap_length = readb(base);
+ op_reg_base = base + cap_length;
+
+ val = EHCI_INSNREG01_THRESH;
+ writel(val, op_reg_base + EHCI_INSNREG01);
+
+ iounmap(base);
+}
/* called after powerup, by probe or system-pm "wakeup" */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
@@ -50,6 +91,10 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, "MWI active\n");
+ /* Reset the threshold limit */
+ if (usb_is_intel_quark_x1000(pdev))
+ usb_set_qrk_bulk_thresh(pdev);
+
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] USB: ehci-pci: Add support for Intel Quark X1000 USB host controller
From: "Alvin (Weike) Chen" Hi, Intel Quark X1000 consists of one USB host controller which can be PCI enumerated. And the exsiting EHCI-PCI framework supports it with the default packet buffer in/out threshold. We reconfigure the in/out threshold as maximal as possible. Bryan O'Donoghue (1): USB: ehci-pci: USB host controller support for Intel Quark X1000 drivers/usb/host/ehci-pci.c | 43 +++ 1 file changed, 43 insertions(+) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] USB: ehci-pci: USB host controller support for Intel Quark X1000
> > > The EHCI packet buffer in/out threshold is programmable for Intel
> > > Quark X1000 USB host controller, and the default value is 0x20
> > > dwords. The in/out threshold can be programmed to 0x80 dwords, but
> > > only when isochronous/interrupt transactions are not initiated by
> > > the USB host controller. This patch is to reconfigure the packet
> > > buffer in/out threshold as maximal as possible, and it is 0x7F dwords
> > > since
> the USB host controller initiates isochronous/interrupt transactions.
> >
> > So, what is the reason to set the value as 0x80?
> > 1. The default value 0x20 makes some problems such as...
> > 2. To maximize the performance, ...
> > 3. Both
> > Please add the reason why 0x80 is necessary, as possible.
> >
> > Then, 0x7F means 508 bytes? 'Intel Quark X1000 USB host controller'
> > can support 0x80 (512bytes), however, in this case,
> > isochronous/interrupt transactions cannot be initiated by 'Intel Quark X1000
> USB host controller'.
> > Right?
> >
> > So, 0x79 (508bytes?) should be used, because the isochronous/interrupt
> > transactions can be initiated by 'Intel Quark X1000 USB host controller'.
> > Right?
> >
Yes, to maximize the performance, we set the threshold as maximal as possible.
Intel Quark X1000 can support 0x80 dwords (512Bytes), but 0x7F dwords (508
Bytes) should be used since the isochronous/interrupt
transactions can be initiated by Intel Quark X1000 USB host controller. I will
update the descriptions.
> > > /*--
> > > ---*/
> > > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC 0x0939
> > > +static inline bool usb_is_intel_quark_x1000(struct pci_dev *pdev) {
> > > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > > +
> >
> > Please don't add this unnecessary line.
OK, I will remove it.
> >
> > > +}
> > > +
> > > +/* Register offset of in/out threshold */
> > > +#define EHCI_INSNREG01 0x84
> > > +/* The maximal threshold is 0x80 Dword */
> > > +#define EHCI_MAX_THRESHOLD 0X80
> >
> > s/0X80/0x80
> >
> > 0x80 means 512 bytes. So, how about mentioning '0x80 means 512 bytes'
> > in this comment or the commit message?
> >
> > Maybe, how about the following?
> >
> > /* The maximal threshold value is 0x80, which means 512 bytes */
> > #define EHCI_THRESHOLD_512BYTES 0x80
> >
> > > +/* Lower word is 'in' threshold, and higher word is 'out'
> > > +threshold*/ #define EHCI_INSNREG01_THRESH \
> > > + ((EHCI_MAX_THRESHOLD - 1)<<16 | (EHCI_MAX_THRESHOLD - 1))
> >
> > Um, how about the following?
> >
> > /* Register offset of in/out threshold */
> > #define EHCI_INSNREG01 0x84
> > /* The maximal threshold value is 0x80, which means 512 bytes */
> > #define EHCI_THRESHOLD_512BYTES 0x80
> > #define EHCI_THRESHOLD_508BYTES 0x79
> > #define EHCI_THRESHOLD_OUT_SHIFT16
> > #define EHCI_THRESHOLD_IN_SHIFT 0
> >
> > ..
> >
> > /*
> > * In order to support the isochronous/interrupt transactions,
> > * 508 bytes should be used as max threshold values */
> > */
> > val = ((EHCI_THRESHOLD_512BYTES - 1) <<
> EHCI_THRESHOLD_OUT_SHIFT |
> > (EHCI_THRESHOLD_512BYTES - 1) << EHCI_THRESHOLD_IN_SHIFT);
> > writel(val, op_reg_base + EHCI_INSNREG01);
>
> Sorry, please refer to the following.
>
> /* Register offset of in/out threshold */
> #define EHCI_INSNREG010x84
> /* The maximal threshold value is 0x80, which means 512 bytes */
> #define EHCI_THRESHOLD_512BYTES 0x80
> #define EHCI_THRESHOLD_508BYTES 0x79
> #define EHCI_THRESHOLD_OUT_SHIFT 16
> #define EHCI_THRESHOLD_IN_SHIFT 0
>
> ..
>
> /*
>* In order to support the isochronous/interrupt transactions,
>* 508 bytes should be used as max threshold values */
>*/
> val = (EHCI_THRESHOLD_508BYTES <<
> EHCI_THRESHOLD_OUT_SHIFT |
> (EHCI_THRESHOLD_508BYTES << EHCI_THRESHOLD_IN_SHIFT);
> writel(val, op_reg_base + EHCI_INSNREG01);
>
I will improve the according to your suggestions.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] USB: ehci-pci: USB host controller support for Intel Quark X1000
> -Original Message- > From: David Laight [mailto:david.lai...@aculab.com] > Sent: Friday, June 27, 2014 4:08 PM > ... > > /* The maximal threshold value is 0x80, which means 512 bytes */ > > #define EHCI_THRESHOLD_512BYTES 0x80 > > #define EHCI_THRESHOLD_508BYTES 0x79 > > It would be better to define these using expressions. So: > #define EHCI_THRESHOLD_512BYTES (512u / 8u) > #define EHCI_THRESHOLD_508BYTES (508u / 8u) > Just to clarify, the threshold value set to register is in dword, so '0x80' means 0x80 dwords (512 bytes), and 508 bytes is not 0x79, it is 0x7F. > Then you might decide to use: > #define EHCI_THRESHOLD(size) ((size) / 8u) > > Then realise that the names are not generic EHCI, so need some driver-specific > prefix (for namespace reasons). > How about the following? #define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size) / 4u) /* threshold value is in dword*/ > And that the defines are probably limit values, and should be named as such. > > David > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2] USB: ehci-pci: USB host controller support for Intel Quark X1000
> > The EHCI packet buffer in/out threshold is programmable for Intel
> > Quark X1000 USB host controller, and the default value is 0x20 dwords.
> > The in/out threshold can be programmed to 0x80 dwords, but only when
> > isochronous/interrupt transactions are not initiated by the USB host
> > controller. This patch is to reconfigure the packet buffer in/out
> > threshold as maximal as possible, and it is 0x7F dwords since the USB host
> controller initiates isochronous/interrupt transactions.
>
> This is still incomplete. _Why_ do you want to increase the threshold?
> Does it fix a problem? Does it improve performance?
Try to set threshold as maximal as possible to maximize the performance. I will
update the descriptions.
> Also, the lines are too long. They should be wrapped before 80 columns.
Got you.
> > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
> > +static inline bool usb_is_intel_quark_x1000(struct pci_dev *pdev) {
> > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > +
> > +}
>
> The "usb_" prefix in the name isn't needed, because this is a static routine.
OK, I will remove the 'usb_' prefix.
> > +static void usb_set_qrk_bulk_thresh(struct pci_dev *pdev) {
> > + void __iomem *base, *op_reg_base;
> > + u8 cap_length;
> > + u32 val;
> > + u16 cmd;
> > +
> > + if (!pci_resource_start(pdev, 0))
> > + return;
> > +
> > + if (pci_read_config_word(pdev, PCI_COMMAND, &cmd)
> > + || !(cmd & PCI_COMMAND_MEMORY))
> > + return;
> > +
> > + base = pci_ioremap_bar(pdev, 0);
> > + if (base == NULL)
> > + return;
> > +
> > + cap_length = readb(base);
> > + op_reg_base = base + cap_length;
> > +
> > + val = EHCI_INSNREG01_THRESH;
> > + writel(val, op_reg_base + EHCI_INSNREG01);
> > +
> > + iounmap(base);
> > +}
>
> This is much more complicted that it needs to be. When this routine runs, the
> controller has already been memory-mapped. All you need to do is:
>
> ehci_writel(ehci, EHCI_INSNREG01_THRESH,
> ehci->regs->insnreg01_thresh);
>
> Since it's only one statement, you don't even need to put this in a separate
> function. It can go directly into ehci_pci_reinit().
OK, I will remove ' usb_set_qrk_bulk_thresh', and change the code as the
suggestions.
> Also, in include/linux/usb/ehci_defs.h, you'll have to add:
>
> #define insnreg01_thresh hostpc[0]
I will add it in ehci-pci.c instead of /linux/usb/ehci_defs.h, because it is
not a generic micro, but just used for Intel Quark X1000.
> with an explanatory comment, near the definition of the HOSTPC register.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
From: Bryan O'Donoghue
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host controller, and the default value is 0x20 dwords. The in/out threshold
can be programmed to 0x80 dwords (512 Bytes) to maximize the perfomrance,
but only when isochronous/interrupt transactions are not initiated by the USB
host controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible to maximize the performance, and 0x7F dwords
(508 Bytes) should be used because the USB host controller initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue
Signed-off-by: Alvin (Weike) Chen
---
changelog v3:
*Update the description to explain why set in/out threshold
as maximal as possible.
*Improve the threshold value micros more readable.
*Remove 'usb_set_qrk_bulk_thresh'.
*Set threshold value by 'ehci_writel' instead of 'usb_set_qrk_bulk_thresh'.
*Change the function name from 'usb_is_intel_quark_x1000'
to 'is_intel_quark_x1000'.
drivers/usb/host/ehci-pci.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..78f1622 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,11 +35,35 @@ static const char hcd_name[] = "ehci-pci";
#define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
+
+/*
+ * The offset of in/out threshold register is 0x84.
+ * And it is the register of 'hostpc'
+ * in memory-mapped EHCI controller.
+*/
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* The maximal ehci packet buffer size is 512 bytes */
+#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
+
+/* The threshold value set the register is in DWORD */
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
+#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
+
/* called after powerup, by probe or system-pm "wakeup" */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
{
int retval;
+ u32 val;
+ u32 thr;
/* we expect static quirk code to handle the "extended capabilities"
* (currently just BIOS handoff) allowed starting with EHCI 0.96
@@ -50,6 +74,22 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, "MWI active\n");
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+ * In order to support the isochronous/interrupt
+ * transactions, 508 bytes should be used as
+ * max threshold values to maximize the
+ * performance
+ */
+ thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
+ INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
+ );
+ val = thrintel_quark_x1000_insnreg01);
+ }
+
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3] USB: ehci-pci: Add support for Intel Quark X1000 USB
From: "Alvin (Weike) Chen" Hi, Intel Quark X1000 consists of one USB host controller which can be PCI enumerated. And the exsiting EHCI-PCI framework supports it with the default packet buffer in/out threshold. We reconfigure the in/out threshold as maximal as possible to maximize the performance. Bryan O'Donoghue (1): USB: ehci-pci: USB host controller support for Intel Quark X1000 drivers/usb/host/ehci-pci.c | 40 +++ 1 file changed, 40 insertions(+) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
> > /*
> > -*/
> > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
> > +static inline bool is_intel_quark_x1000(struct pci_dev *pdev) {
> > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > +}
>
> Why not just put this check inline into ehci_pci_reinit()?
Alan Stern said it is not a problem, I think so also since it just a inline
subroutine.
>
> > +
> > +/*
> > + * The offset of in/out threshold register is 0x84.
> > + * And it is the register of 'hostpc'
> > + * in memory-mapped EHCI controller.
> > +*/
>
> The preferred multi-line kernel style is this:
>
> /*
> * bla
> * bla
> */
I will improve it.
> > +#defineintel_quark_x1000_insnreg01 hostpc
> > +
> > +/* The maximal ehci packet buffer size is 512 bytes */
>
> s/ehci/EHCI/
>
> > +#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
> > +
> > +/* The threshold value set the register is in DWORD */
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
> > +
> >
>
> Too many empty lines...
>
> > /* called after powerup, by probe or system-pm "wakeup" */
> > static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
> > {
> > int retval;
> > + u32 val;
> > + u32 thr;
>
> Why not declare these where they are used?
All will be removed as Alan Stern's suggestion.
> > + /* Reset the threshold limit */
> > + if (is_intel_quark_x1000(pdev)) {
> > + /*
> > + * In order to support the isochronous/interrupt
> > + * transactions, 508 bytes should be used as
> > + * max threshold values to maximize the
> > + * performance
> > + */
>
> Same comment about the comment style...
>
> > + thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
> > + INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
> > + );
> > + val = thr< > + thr<
> Please surround << with spaces for consistency.
The above code will be removed as Alan Stern's suggestion.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v3] USB: ehci-pci: USB host controller support for Intel Quark X1000
> >
> > /*
> > -*/
> > +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
> > +static inline bool is_intel_quark_x1000(struct pci_dev *pdev) {
> > + return pdev->vendor == PCI_VENDOR_ID_INTEL &&
> > + pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
> > +}
>
> Whether to put this test directly into ehci_pci_reset() or leave it as a
> separate
> subroutine is up to you. I don't care either way.
I will just keep it.
> > +
> > +/*
> > + * The offset of in/out threshold register is 0x84.
> > + * And it is the register of 'hostpc'
> > + * in memory-mapped EHCI controller.
> > +*/
>
> 0x84 is the same as offset of the hostpc register in the Intel Moorestown
> controller. hostpc is not present in general EHCI controllers.
>
OK, I will improve the comments.
> > +#defineintel_quark_x1000_insnreg01 hostpc
> > +
> > +/* The maximal ehci packet buffer size is 512 bytes */
> > +#define INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE 512
> > +
> > +/* The threshold value set the register is in DWORD */
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD(size) ((size)/4u)
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD_OUT_SHIFT 16
> > +#define INTEL_QUARK_X1000_EHCI_THRESHOLD_IN_SHIFT 0
> > +
> > /* called after powerup, by probe or system-pm "wakeup" */ static
> > int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev) {
> > int retval;
> > + u32 val;
> > + u32 thr;
> >
> > /* we expect static quirk code to handle the "extended capabilities"
> > * (currently just BIOS handoff) allowed starting with EHCI 0.96 @@
> > -50,6 +74,22 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
> pci_dev *pdev)
> > if (!retval)
> > ehci_dbg(ehci, "MWI active\n");
> >
> > + /* Reset the threshold limit */
> > + if (is_intel_quark_x1000(pdev)) {
> > + /*
> > + * In order to support the isochronous/interrupt
> > + * transactions, 508 bytes should be used as
> > + * max threshold values to maximize the
> > + * performance
> > + */
> > + thr = INTEL_QUARK_X1000_EHCI_THRESHOLD(
> > + INTEL_QUARK_X1000_EHCI_MAX_PACKET_BUFFER_SIZE - 4
> > + );
> > + val = thr< > + thr< > + ehci_writel(ehci, val, ehci->regs->intel_quark_x1000_insnreg01);
>
> I saw what other people told you about the original patch version, and I
> disagree with them. It is not necessary to include a detailed calculation
> like
> this, it only makes the code harder to read. It will be better to have a
> single
> #define with a comment explaining it, like
> this:
>
> /* Maximum usable threshold value is 0x7f dwords for both IN and OUT */
> #define INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD 0x007f007f
>
> Then here, just use INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD instead of
> val. The comment can simply say:
>
> /*
>* For the Intel QUARK X1000, raise the I/O threshold to the
>* maximum usable value in order to improve performance.
>*/
>
I think so also. It is not necessary to make so complicated. I will adopt your
suggestions, it is more simple and clearly.
> Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] USB: ehci-pci: USB host controller support for Intel Quark X1000
From: Bryan O'Donoghue
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host controller, and the default value is 0x20 dwords. The in/out threshold
can be programmed to 0x80 dwords (512 Bytes) to maximize the perfomrance,
but only when isochronous/interrupt transactions are not initiated by the USB
host controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible to maximize the performance, and 0x7F dwords
(508 Bytes) should be used because the USB host controller initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue
Signed-off-by: Alvin (Weike) Chen
---
changelog v4:
* Just define the maximum threshold value, and clarify it in the comments.
* Improve the comments.
drivers/usb/host/ehci-pci.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..429434d 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,6 +35,21 @@ static const char hcd_name[] = "ehci-pci";
#define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
+
+/*
+ * 0x84 is the offset of in/out threshold register,
+ * and it is the same offset as the register of 'hostpc'.
+ */
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* Maximum usable threshold value is 0x7f dwords for both IN and OUT */
+#define INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD 0x007f007f
/* called after powerup, by probe or system-pm "wakeup" */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
@@ -50,6 +65,16 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, "MWI active\n");
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+* For the Intel QUARK X1000, raise the I/O threshold to the
+* maximum usable value in order to improve performance.
+*/
+ ehci_writel(ehci, INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD,
+ ehci->regs->intel_quark_x1000_insnreg01);
+ }
+
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4] USB: ehci-pci: Add support for Intel Quark X1000 USB
From: "Alvin (Weike) Chen" Hi, Intel Quark X1000 consists of one USB host controller which can be PCI enumerated. And the exsiting EHCI-PCI framework supports it with the default packet buffer in/out threshold. We reconfigure the in/out threshold as maximal as possible to maximize the performance. Bryan O'Donoghue (1): USB: ehci-pci: USB host controller support for Intel Quark X1000 drivers/usb/host/ehci-pci.c | 25 +++ 1 file changed, 25 insertions(+) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] USB: ehci-pci: USB host controller support for Intel Quark X1000
From: Bryan O'Donoghue
The EHCI packet buffer in/out threshold is programmable for Intel Quark X1000
USB host controller, and the default value is 0x20 dwords. The in/out threshold
can be programmed to 0x80 dwords (512 Bytes) to maximize the perfomrance,
but only when isochronous/interrupt transactions are not initiated by the USB
host controller. This patch is to reconfigure the packet buffer in/out
threshold as maximal as possible to maximize the performance, and 0x7F dwords
(508 Bytes) should be used because the USB host controller initiates
isochronous/interrupt transactions.
Signed-off-by: Bryan O'Donoghue
Signed-off-by: Alvin (Weike) Chen
Acked-by: Alan Stern
Reviewed-by: Jingoo Han
---
Changlog v5:
* Correct the wrong comment style.
drivers/usb/host/ehci-pci.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 3e86bf4..ca7b964 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -35,6 +35,21 @@ static const char hcd_name[] = "ehci-pci";
#define PCI_DEVICE_ID_INTEL_CE4100_USB 0x2e70
/*-*/
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC0x0939
+static inline bool is_intel_quark_x1000(struct pci_dev *pdev)
+{
+ return pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ pdev->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_SOC;
+}
+
+/*
+ * 0x84 is the offset of in/out threshold register,
+ * and it is the same offset as the register of 'hostpc'.
+ */
+#defineintel_quark_x1000_insnreg01 hostpc
+
+/* Maximum usable threshold value is 0x7f dwords for both IN and OUT */
+#define INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD 0x007f007f
/* called after powerup, by probe or system-pm "wakeup" */
static int ehci_pci_reinit(struct ehci_hcd *ehci, struct pci_dev *pdev)
@@ -50,6 +65,16 @@ static int ehci_pci_reinit(struct ehci_hcd *ehci, struct
pci_dev *pdev)
if (!retval)
ehci_dbg(ehci, "MWI active\n");
+ /* Reset the threshold limit */
+ if (is_intel_quark_x1000(pdev)) {
+ /*
+* For the Intel QUARK X1000, raise the I/O threshold to the
+* maximum usable value in order to improve performance.
+*/
+ ehci_writel(ehci, INTEL_QUARK_X1000_EHCI_MAX_THRESHOLD,
+ ehci->regs->intel_quark_x1000_insnreg01);
+ }
+
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: pch_udc: USB gadget device support for Intel Quark X1000
From: Bryan O'Donoghue
This patch is to enable the USB gadget device for Intel Quark X1000
Signed-off-by: Bryan O'Donoghue
Signed-off-by: Bing Niu
Signed-off-by: Alvin (Weike) Chen
---
drivers/usb/gadget/udc/Kconfig |3 ++-
drivers/usb/gadget/udc/pch_udc.c | 22 +++---
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 5151f94..34ebaa6 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -332,7 +332,7 @@ config USB_GOKU
gadget drivers to also be dynamically linked.
config USB_EG20T
- tristate "Intel EG20T PCH/LAPIS Semiconductor IOH(ML7213/ML7831) UDC"
+ tristate "Intel QUARK X1000/EG20T PCH/LAPIS Semiconductor
IOH(ML7213/ML7831) UDC"
depends on PCI
help
This is a USB device driver for EG20T PCH.
@@ -353,6 +353,7 @@ config USB_EG20T
ML7213/ML7831 is companion chip for Intel Atom E6xx series.
ML7213/ML7831 is completely compatible for Intel EG20T PCH.
+ This driver can be used with Intel's Quark X1000 SOC platform
#
# LAST -- dummy/emulated controller
#
diff --git a/drivers/usb/gadget/udc/pch_udc.c b/drivers/usb/gadget/udc/pch_udc.c
index eb8c3be..460d953 100644
--- a/drivers/usb/gadget/udc/pch_udc.c
+++ b/drivers/usb/gadget/udc/pch_udc.c
@@ -343,6 +343,7 @@ struct pch_vbus_gpio_data {
* @setup_data:Received setup data
* @phys_addr: of device memory
* @base_addr: for mapped device memory
+ * @bar: Indicates which PCI BAR for USB regs
* @irq: IRQ line for the device
* @cfg_data: current cfg, intf, and alt in use
* @vbus_gpio: GPIO informaton for detecting VBUS
@@ -370,14 +371,17 @@ struct pch_udc_dev {
struct usb_ctrlrequest setup_data;
unsigned long phys_addr;
void __iomem*base_addr;
+ unsignedbar;
unsignedirq;
struct pch_udc_cfg_data cfg_data;
struct pch_vbus_gpio_data vbus_gpio;
};
#define to_pch_udc(g) (container_of((g), struct pch_udc_dev, gadget))
+#define PCH_UDC_PCI_BAR_QUARK_X10000
#define PCH_UDC_PCI_BAR1
#define PCI_DEVICE_ID_INTEL_EG20T_UDC 0x8808
+#define PCI_DEVICE_ID_INTEL_QUARK_X1000_UDC0x0939
#define PCI_VENDOR_ID_ROHM 0x10DB
#define PCI_DEVICE_ID_ML7213_IOH_UDC 0x801D
#define PCI_DEVICE_ID_ML7831_IOH_UDC 0x8808
@@ -3076,7 +3080,7 @@ static void pch_udc_remove(struct pci_dev *pdev)
iounmap(dev->base_addr);
if (dev->mem_region)
release_mem_region(dev->phys_addr,
- pci_resource_len(pdev, PCH_UDC_PCI_BAR));
+ pci_resource_len(pdev, dev->bar));
if (dev->active)
pci_disable_device(pdev);
kfree(dev);
@@ -3144,9 +3148,15 @@ static int pch_udc_probe(struct pci_dev *pdev,
dev->active = 1;
pci_set_drvdata(pdev, dev);
+ /* Determine BAR based on PCI ID */
+ if (id->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_UDC)
+ dev->bar = PCH_UDC_PCI_BAR_QUARK_X1000;
+ else
+ dev->bar = PCH_UDC_PCI_BAR;
+
/* PCI resource allocation */
- resource = pci_resource_start(pdev, 1);
- len = pci_resource_len(pdev, 1);
+ resource = pci_resource_start(pdev, dev->bar);
+ len = pci_resource_len(pdev, dev->bar);
if (!request_mem_region(resource, len, KBUILD_MODNAME)) {
dev_err(&pdev->dev, "%s: pci device used already\n", __func__);
@@ -3212,6 +3222,12 @@ finished:
static const struct pci_device_id pch_udc_pcidev_id[] = {
{
+ PCI_DEVICE(PCI_VENDOR_ID_INTEL,
+ PCI_DEVICE_ID_INTEL_QUARK_X1000_UDC),
+ .class = (PCI_CLASS_SERIAL_USB << 8) | 0xfe,
+ .class_mask = 0x,
+ },
+ {
PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_EG20T_UDC),
.class = (PCI_CLASS_SERIAL_USB << 8) | 0xfe,
.class_mask = 0x,
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] USB: pch_udc: Add support for Intel Quark X1000 USB gadget device
From: "Alvin (Weike) Chen" Hi, Intel Quark X1000 consists of one USB gadget device which can be PCI enumerated. pch_udc layer doesn't support it. Thus, we add support for Intel Quark X1000 USB gadget device as well. Bryan O'Donoghue (1): USB: pch_udc: USB gadget device support for Intel Quark X1000 drivers/usb/gadget/udc/Kconfig |3 ++- drivers/usb/gadget/udc/pch_udc.c | 22 +++--- 2 files changed, 21 insertions(+), 4 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: pch_udc: USB gadget device support for Intel Quark X1000
Hi Felipe,
Any update for this patch? Just want to follow-up.
> -Original Message-
> From: Chen, Alvin
> Sent: Tuesday, August 05, 2014 1:23 AM
> To: Felipe Balbi
> Cc: linux-usb@vger.kernel.org; linux-ker...@vger.kernel.org; Ong, Boon
> Leong
> Subject: [PATCH] USB: pch_udc: USB gadget device support for Intel
> Quark
> X1000
>
> From: Bryan O'Donoghue
>
> This patch is to enable the USB gadget device for Intel Quark X1000
>
> Signed-off-by: Bryan O'Donoghue
> Signed-off-by: Bing Niu
> Signed-off-by: Alvin (Weike) Chen
> ---
> drivers/usb/gadget/udc/Kconfig |3 ++-
> drivers/usb/gadget/udc/pch_udc.c | 22 +++---
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/Kconfig
> b/drivers/usb/gadget/udc/Kconfig index 5151f94..34ebaa6 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -332,7 +332,7 @@ config USB_GOKU
> gadget drivers to also be dynamically linked.
>
> config USB_EG20T
> - tristate "Intel EG20T PCH/LAPIS Semiconductor IOH(ML7213/ML7831)
> UDC"
> + tristate "Intel QUARK X1000/EG20T PCH/LAPIS Semiconductor
> IOH(ML7213/ML7831) UDC"
> depends on PCI
> help
> This is a USB device driver for EG20T PCH.
> @@ -353,6 +353,7 @@ config USB_EG20T
> ML7213/ML7831 is companion chip for Intel Atom E6xx series.
> ML7213/ML7831 is completely compatible for Intel EG20T PCH.
>
> + This driver can be used with Intel's Quark X1000 SOC platform
> #
> # LAST -- dummy/emulated controller
> #
> diff --git a/drivers/usb/gadget/udc/pch_udc.c
> b/drivers/usb/gadget/udc/pch_udc.c
> index eb8c3be..460d953 100644
> --- a/drivers/usb/gadget/udc/pch_udc.c
> +++ b/drivers/usb/gadget/udc/pch_udc.c
> @@ -343,6 +343,7 @@ struct pch_vbus_gpio_data {
> * @setup_data: Received setup data
> * @phys_addr: of device memory
> * @base_addr: for mapped device memory
> + * @bar: Indicates which PCI BAR for USB regs
> * @irq: IRQ line for the device
> * @cfg_data:current cfg, intf, and alt in use
> * @vbus_gpio: GPIO informaton for detecting VBUS
> @@ -370,14 +371,17 @@ struct pch_udc_dev {
> struct usb_ctrlrequest setup_data;
> unsigned long phys_addr;
> void __iomem*base_addr;
> + unsignedbar;
> unsignedirq;
> struct pch_udc_cfg_data cfg_data;
> struct pch_vbus_gpio_data vbus_gpio;
> };
> #define to_pch_udc(g)(container_of((g), struct pch_udc_dev,
> gadget))
>
> +#define PCH_UDC_PCI_BAR_QUARK_X1000 0
> #define PCH_UDC_PCI_BAR 1
> #define PCI_DEVICE_ID_INTEL_EG20T_UDC0x8808
> +#define PCI_DEVICE_ID_INTEL_QUARK_X1000_UDC 0x0939
> #define PCI_VENDOR_ID_ROHM 0x10DB
> #define PCI_DEVICE_ID_ML7213_IOH_UDC 0x801D
> #define PCI_DEVICE_ID_ML7831_IOH_UDC 0x8808
> @@ -3076,7 +3080,7 @@ static void pch_udc_remove(struct pci_dev *pdev)
> iounmap(dev->base_addr);
> if (dev->mem_region)
> release_mem_region(dev->phys_addr,
> -pci_resource_len(pdev,
> PCH_UDC_PCI_BAR));
> +pci_resource_len(pdev, dev->bar));
> if (dev->active)
> pci_disable_device(pdev);
> kfree(dev);
> @@ -3144,9 +3148,15 @@ static int pch_udc_probe(struct pci_dev *pdev,
> dev->active = 1;
> pci_set_drvdata(pdev, dev);
>
> + /* Determine BAR based on PCI ID */
> + if (id->device == PCI_DEVICE_ID_INTEL_QUARK_X1000_UDC)
> + dev->bar = PCH_UDC_PCI_BAR_QUARK_X1000;
> + else
> + dev->bar = PCH_UDC_PCI_BAR;
> +
> /* PCI resource allocation */
> - resource = pci_resource_start(pdev, 1);
> - len = pci_resource_len(pdev, 1);
> + resource = pci_resource_start(pdev, dev->bar);
> + len = pci_resource_len(pdev, dev->bar);
>
> if (!request_mem_region(resource, len, KBUILD_MODNAME)) {
> dev_err(&pdev->dev, "%s: pci device used already\n", __func__);
> @@
> -3212,6 +3222,12 @@ finished:
>
> static const struct pci_device_id pch_udc_pcidev_id[] = {
> {
> + PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> +PCI_DEVICE_ID_INTEL_QUARK_X1000_UDC),
> + .class = (PCI_CLASS_SERIAL_USB << 8) | 0xfe,
> + .class_mask = 0x,
> + },
> + {
> PCI_DEVICE(PCI_VENDOR_ID_INTEL,
> PCI_DEVICE_ID_INTEL_EG20T_UDC),
> .class = (PCI_CLASS_SERIAL_USB << 8) | 0xfe,
> .class_mask = 0x,
> --
> 1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] USB: pch_udc: USB gadget device support for Intel Quark X1000
> Hi, > > On Mon, Aug 04, 2014 at 10:22:54AM -0700, Chen, Alvin wrote: > > From: Bryan O'Donoghue > > > > This patch is to enable the USB gadget device for Intel Quark X1000 > > > > Signed-off-by: Bryan O'Donoghue > > Signed-off-by: Bing Niu > > Signed-off-by: Alvin (Weike) Chen > > Can someone confirm to me this is not another incarnation of chipidea ? No, this is not another incarnation of chipidea. And its cover letter is as following: On Mon, Aug 04, 2014 at 11:00:07AM -0700, Chen, Alvin wrote: > From: "Alvin (Weike) Chen" > > Hi, > Intel Quark X1000 consists of one USB gadget device which can be PCI > enumerated. > pch_udc layer doesn't support it. Thus, we add support for Intel Quark > X1000 USB gadget device as well. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
