Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1411740 > unrolled thread

[PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-02 03:40 +0200
    Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-08 06:50 +0200
      Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-08 10:00 +0200
        Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-06-08 17:50 +0200
          Re: [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device Lu Baolu <baolu.lu@linux.intel.com> - 2016-06-09 04:40 +0200

#1411740 — [PATCH v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

In some Intel platforms, a single usb port is shared between USB host
and device controllers. The shared port is under control of a switch
which is defined in the Intel vendor defined extended capability for
xHCI.

This patch adds the support to detect and create the platform device
for the port mux switch.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@kernel.org>
---
 drivers/usb/host/pci-quirks.c    | 42 +++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/xhci-ext-caps.h |  2 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index 35af362..7e3194f 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -16,6 +16,8 @@
 #include <linux/export.h>
 #include <linux/acpi.h>
 #include <linux/dmi.h>
+#include <linux/platform_device.h>
+
 #include "pci-quirks.h"
 #include "xhci-ext-caps.h"
 
@@ -78,6 +80,9 @@
 #define USB_INTEL_USB3_PSSEN   0xD8
 #define USB_INTEL_USB3PRM      0xDC
 
+#define DEVICE_ID_INTEL_BROXTON_P_XHCI		0x5aa8
+#define DEVICE_ID_INTEL_CHERRYVIEW_XHCI		0x22b5
+
 /*
  * amd_chipset_gen values represent AMD different chipset generations
  */
@@ -956,6 +961,36 @@ void usb_disable_xhci_ports(struct pci_dev *xhci_pdev)
 }
 EXPORT_SYMBOL_GPL(usb_disable_xhci_ports);
 
+static void create_intel_usb_mux_device(struct pci_dev *xhci_pdev,
+					void __iomem *base)
+{
+	int ret;
+	struct platform_device *plat_dev;
+	struct property_entry pentry[] = {
+		PROPERTY_ENTRY_U64("reg-start",
+				   pci_resource_start(xhci_pdev, 0) + 0x80d8),
+		PROPERTY_ENTRY_U64("reg-size", 8),
+		{ },
+	};
+
+	if (!xhci_find_next_ext_cap(base, 0, XHCI_EXT_CAPS_INTEL_USB_MUX))
+		return;
+
+	plat_dev = platform_device_alloc("intel-mux-drcfg",
+					 PLATFORM_DEVID_NONE);
+	if (!plat_dev)
+		return;
+
+	plat_dev->dev.parent = &xhci_pdev->dev;
+	platform_device_add_properties(plat_dev, pentry);
+	ret = platform_device_add(plat_dev);
+	if (ret) {
+		dev_warn(&xhci_pdev->dev,
+			 "failed to create mux device with error %d", ret);
+		platform_device_put(plat_dev);
+	}
+}
+
 /**
  * PCI Quirks for xHCI.
  *
@@ -1022,9 +1057,14 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
 	writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
 
 hc_init:
-	if (pdev->vendor == PCI_VENDOR_ID_INTEL)
+	if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
 		usb_enable_intel_xhci_ports(pdev);
 
+		if (pdev->device == DEVICE_ID_INTEL_BROXTON_P_XHCI ||
+		    pdev->device == DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
+			create_intel_usb_mux_device(pdev, base);
+	}
+
 	op_reg_base = base + XHCI_HC_LENGTH(readl(base));
 
 	/* Wait for the host controller to be ready before writing any
diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index e0244fb..e368ccb 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -51,6 +51,8 @@
 #define XHCI_EXT_CAPS_ROUTE	5
 /* IDs 6-9 reserved */
 #define XHCI_EXT_CAPS_DEBUG	10
+/* Vendor defined 192-255 */
+#define XHCI_EXT_CAPS_INTEL_USB_MUX	192
 /* USB Legacy Support Capability - section 7.1.1 */
 #define XHCI_HC_BIOS_OWNED	(1 << 16)
 #define XHCI_HC_OS_OWNED	(1 << 24)
-- 
2.1.4

[toc] | [next] | [standalone]

#1416865

On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
> In some Intel platforms, a single usb port is shared between USB host
> and device controllers. The shared port is under control of a switch
> which is defined in the Intel vendor defined extended capability for
> xHCI.
> 
> This patch adds the support to detect and create the platform device
> for the port mux switch.

Why do you need a platform device for this?  You do nothing with this
device, why create it at all?

And why is it a platform device, isn't is really a PCI device?  Why
would you ever find a "platform" device below a PCI device?  Don't abuse
platform devices for things that aren't.  It makes me want to delete
that whole interface more and more...

greg k-h

[toc] | [prev] | [next] | [standalone]

#1416999

Hi Greg,

On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
>> In some Intel platforms, a single usb port is shared between USB host
>> and device controllers. The shared port is under control of a switch
>> which is defined in the Intel vendor defined extended capability for
>> xHCI.
>>
>> This patch adds the support to detect and create the platform device
>> for the port mux switch.
> Why do you need a platform device for this?  You do nothing with this
> device, why create it at all?

In this patch series, I have a generic framework for port mux devices
and two port mux drivers sitting on top the generic code.

In this patch, I create a platform device for the real mux device in
Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux
into the generic framework and handle the power management
things in driver's pm entries (otherwise, the system can't be waken
up from system suspend).

> And why is it a platform device, isn't is really a PCI device?  Why
> would you ever find a "platform" device below a PCI device?  Don't abuse
> platform devices for things that aren't.  It makes me want to delete
> that whole interface more and more...

Port mux devices are physical devices in Intel Cherry Trail and Broxton
SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI
space. OS kernel can enumerate it by looking up the xhci extended
capability list with a vendor specific capability ID.

Best regards,
Lu Baolu

[toc] | [prev] | [next] | [standalone]

#1417605

On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
> Hi Greg,
> 
> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
> > On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
> >> In some Intel platforms, a single usb port is shared between USB host
> >> and device controllers. The shared port is under control of a switch
> >> which is defined in the Intel vendor defined extended capability for
> >> xHCI.
> >>
> >> This patch adds the support to detect and create the platform device
> >> for the port mux switch.
> > Why do you need a platform device for this?  You do nothing with this
> > device, why create it at all?
> 
> In this patch series, I have a generic framework for port mux devices
> and two port mux drivers sitting on top the generic code.
> 
> In this patch, I create a platform device for the real mux device in
> Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux
> into the generic framework and handle the power management
> things in driver's pm entries (otherwise, the system can't be waken
> up from system suspend).
> 
> > And why is it a platform device, isn't is really a PCI device?  Why
> > would you ever find a "platform" device below a PCI device?  Don't abuse
> > platform devices for things that aren't.  It makes me want to delete
> > that whole interface more and more...
> 
> Port mux devices are physical devices in Intel Cherry Trail and Broxton
> SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI
> space. OS kernel can enumerate it by looking up the xhci extended
> capability list with a vendor specific capability ID.

A physical device that maps registers into PCI space seems like a PCI
device of some type to me :)

Again, I hate platform devices for obvious reasons like this...

greg k-h

[toc] | [prev] | [next] | [standalone]

#1418020

Hi Greg,

On 06/08/2016 11:45 PM, Greg Kroah-Hartman wrote:
> On Wed, Jun 08, 2016 at 03:56:04PM +0800, Lu Baolu wrote:
>> Hi Greg,
>>
>> On 06/08/2016 12:45 PM, Greg Kroah-Hartman wrote:
>>> On Thu, Jun 02, 2016 at 09:37:28AM +0800, Lu Baolu wrote:
>>>> In some Intel platforms, a single usb port is shared between USB host
>>>> and device controllers. The shared port is under control of a switch
>>>> which is defined in the Intel vendor defined extended capability for
>>>> xHCI.
>>>>
>>>> This patch adds the support to detect and create the platform device
>>>> for the port mux switch.
>>> Why do you need a platform device for this?  You do nothing with this
>>> device, why create it at all?
>> In this patch series, I have a generic framework for port mux devices
>> and two port mux drivers sitting on top the generic code.
>>
>> In this patch, I create a platform device for the real mux device in
>> Intel Cherry Trail or Broxton SOCs. In it's driver, I registered a mux
>> into the generic framework and handle the power management
>> things in driver's pm entries (otherwise, the system can't be waken
>> up from system suspend).:)
>>
>>> And why is it a platform device, isn't is really a PCI device?  Why
>>> would you ever find a "platform" device below a PCI device?  Don't abuse
>>> platform devices for things that aren't.  It makes me want to delete
>>> that whole interface more and more...
>> Port mux devices are physical devices in Intel Cherry Trail and Broxton
>> SOCs. It doesn't sit on any PCIe bus. But it maps its registers in xHCI
>> space. OS kernel can enumerate it by looking up the xhci extended
>> capability list with a vendor specific capability ID.
> A physical device that maps registers into PCI space seems like a PCI
> device of some type to me :)
>
> Again, I hate platform devices for obvious reasons like this...
>

It's not PCI configure space, but xhci's io memory. XHCI spec reserves
a range in its extended capability list for vendor specific things. Intel's
platform leverages this for the port mux device register mapping.
It looks odd though. :)

Best regards,
Lu Baolu

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web