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

[Lu Baolu]

Hi Greg,

On 06/18/2016 08:58 AM, Greg Kroah-Hartman wrote:
> On Thu, Jun 16, 2016 at 08:27:41AM +0800, Lu Baolu wrote:
>> Hi Greg,
>>
>> On 06/09/2016 10:39 AM, Lu Baolu wrote:
>>> 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. :)
>> A gentle ping. :)
> For what?  This patchset is long gone from my queue for the other
> various things that came up with it, what can I do with it now?

I see now. Thanks for your reply.

Best regards,
Lu Baolu

>
>> This port mux is not a PCI device. It only leverages the vendor
>> specific capability defined in xhci specification for enumeration.
> It's still crap :)
>
> I don't know, and don't really remember the patch anymore anyway,
> remember, I have the sort-term memory of a squirrel, you need repost
> patches, with a proper changelog for me to be able to do anything...
>
> 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 v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Greg Kroah-Hartman]

On Thu, Jun 16, 2016 at 08:27:41AM +0800, Lu Baolu wrote:
> Hi Greg,
> 
> On 06/09/2016 10:39 AM, Lu Baolu wrote:
> > 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. :)
> 
> A gentle ping. :)

For what?  This patchset is long gone from my queue for the other
various things that came up with it, what can I do with it now?

> This port mux is not a PCI device. It only leverages the vendor
> specific capability defined in xhci specification for enumeration.

It's still crap :)

I don't know, and don't really remember the patch anymore anyway,
remember, I have the sort-term memory of a squirrel, you need repost
patches, with a proper changelog for me to be able to do anything...

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 v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Lu Baolu]

Hi Greg,

On 06/09/2016 10:39 AM, Lu Baolu wrote:
> 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. :)

A gentle ping. :)

This port mux is not a PCI device. It only leverages the vendor
specific capability defined in xhci specification for enumeration.

Best regards,
Lu Baolu
--
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 v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Lu Baolu]

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
--
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 v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Lu Baolu]

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
--
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 v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Greg Kroah-Hartman]

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
--
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 v10 6/7] usb: pci-quirks: add Intel USB drcfg mux device

[Lu Baolu]

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 
Reviewed-by: Felipe Balbi 
---
 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 
 #include 
 #include 
+#include 
+
 #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_XHCI0x22b5
+
 /*
  * 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 = _pdev->dev;
+   platform_device_add_properties(plat_dev, pentry);
+   ret = platform_device_add(plat_dev);
+   if (ret) {
+   dev_warn(_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_ROUTE5
 /* IDs 6-9 reserved */
 #define XHCI_EXT_CAPS_DEBUG10
+/* Vendor defined 192-255 */
+#define XHCI_EXT_CAPS_INTEL_USB_MUX192
 /* 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

--
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