Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

2017-09-19 Thread Mathias Nyman 

Hi,

sorry about the long delay

On 07.09.2017 18:49, Hans de Goede wrote:

Hi,

On 07-09-17 15:14, Mathias Nyman wrote:

On 05.09.2017 19:42, Hans de Goede wrote:

The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.

Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.


I think it would be better to have one place where we add handlers for
vendor specific extended capabilities.

Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
there's a xhci-ext-caps.h header already

We could walk through the capability list once and add the needed handlers.
Something like:

+int xhci_ext_cap_init(void __iomem *base)


This will need to take a struct xhci_hcd *xhci param instead
as some of the ext_cap handling (including the cht mux code)
will need access to this.



yes, sample code added in second patch for reference/testing.



So I see 2 options here (without making this function PCI specific)
1) Add an u32 product_id field to struct xhci_hcd; or
2) Use a quirk flag as my current code is doing.

I'm fine with doing this either way, please let me know your preference.


Lets go with the quirk for now, I'll sort that out later



Can you do a "git format-patch" of that and send it to me? If you
can give me that + your preference for how to check if we're
dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
with your suggestions applied.


Ended up modifying xhci_find_next_ext_cap() using id = 0 for
the next capability in list. Patch attached,

Second patch is just for reference how to use it.

Thanks
-Mathias


>From d5f26c1595f211ea7d46fca91551f70d1207330d Mon Sep 17 00:00:00 2001
From: Mathias Nyman 
Date: Tue, 19 Sep 2017 14:54:58 +0300
Subject: [PATCH 1/2] xhci: Add option to get next extended capability in list
 by passing id = 0

Modify xhci_find_next_ext_cap(base, offset, id) to return the next
capability offset if 0 is passed for id. Otherwise it will behave as
previously and return the offset of the next capability with matching id

capability id 0 is not used by xhci (reserved)

This is useful when we want to loop through all capabilities.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ext-caps.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index 28deea5..c1b4042 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -96,7 +96,8 @@
  * @base	PCI MMIO registers base address.
  * @start	address at which to start looking, (0 or HCC_PARAMS to start at
  *		beginning of list)
- * @id		Extended capability ID to search for.
+ * @id		Extended capability ID to search for, or 0 for the next
+ *		capability
  *
  * Returns the offset of the next matching extended capability structure.
  * Some capabilities can occur several times, e.g., the XHCI_EXT_CAPS_PROTOCOL,
@@ -122,7 +123,7 @@ static inline int xhci_find_next_ext_cap(void __iomem *base, u32 start, int id)
 		val = readl(base + offset);
 		if (val == ~0)
 			return 0;
-		if (XHCI_EXT_CAPS_ID(val) == id && offset != start)
+		if (offset != start && (id == 0 || XHCI_EXT_CAPS_ID(val) == id))
 			return offset;
 
 		next = XHCI_EXT_CAPS_NEXT(val);
-- 
1.9.1

>From da44e961605d382829f90fdcfb90b61fa5ca9590 Mon Sep 17 00:00:00 2001
From: Mathias Nyman 
Date: Tue, 19 Sep 2017 14:58:40 +0300
Subject: [PATCH 2/2] xhci: test xhci_find_next_ext_cap with 0 id

NOT for UPSTREAM, just testing/reference code

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ext-caps.h |  3 +++
 drivers/usb/host/xhci.c  | 30 ++
 2 files changed, 33 insertions(+)

diff --git a/drivers/usb/host/xhci-ext-caps.h b/drivers/usb/host/xhci-ext-caps.h
index c1b4042..7deb9e7 100644
--- a/drivers/usb/host/xhci-ext-caps.h
+++ b/drivers/usb/host/xhci-ext-caps.h
@@ -51,6 +51,9 @@
 #define XHCI_EXT_CAPS_ROUTE	5
 /* IDs 6-9 reserved */
 #define XHCI_EXT_CAPS_DEBUG	10
+/* IDs 192 - 255 vendor specific extensions */
+#define XHCI_EXT_CAPS_VENDOR_INTEL 192
+
 /* USB Legacy Support Capability - section 7.1.1 */
 #define XHCI_HC_BIOS_OWNED	(1 << 16)
 #define XHCI_HC_OS_OWNED	(1 << 24)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 870093a..99c804a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -4772,6 +4772,34 @@ static int xhci_get_frame(struct usb_hcd *hcd)
 	return readl(&xhci->run_regs->microframe_index) >> 3;
 }
 
+int xhci_ext_cap_init(struct xhci_hcd *xhci)
+{
+	void __iomem *base;
+

Re: [PATCH v3 07/14] xhci: Add Intel cherrytrail extended cap / otg phy mux handling

2017-10-05 Thread Mathias Nyman 

On 22.09.2017 21:37, Hans de Goede wrote:

The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.

Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.

Note this commit adds a call to the new xhci_ext_cap_init() function
to xhci_pci_probe(), it is added here because xhci_ext_cap_init() must
be called only once. If in the future we also want to handle ext-caps
on non pci XHCI HCDs from xhci_ext_cap_init() a call to it should also
be added to other bus probe paths.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Check xHCI controller PCI device-id instead of only checking for the
  Intel Extended capability ID, as the Extended capability ID is used on
  other model Intel xHCI controllers too

Changes in v3:
-Add a new generic xhci_ext_cap_init() function and handle the new
  XHCI_INTEL_CHT_USB_MUX quirk there.
---


Acked-by: Mathias Nyman 

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

2017-09-07 Thread Mathias Nyman 

On 05.09.2017 19:42, Hans de Goede wrote:

The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.

Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.

Signed-off-by: Hans de Goede 
---
Changes in v2:
-Check xHCI controller PCI device-id instead of only checking for the
  Intel Extended capability ID, as the Extended capability ID is used on
  other model Intel xHCI controllers too
---
  drivers/usb/host/Makefile|  2 +-
  drivers/usb/host/xhci-intel-quirks.c | 85 
  drivers/usb/host/xhci-pci.c  |  4 ++
  drivers/usb/host/xhci.h  |  2 +
  4 files changed, 92 insertions(+), 1 deletion(-)
  create mode 100644 drivers/usb/host/xhci-intel-quirks.c

diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index cf2691fffcc0..441edf82eb1c 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -63,7 +63,7 @@ obj-$(CONFIG_USB_OHCI_HCD_DAVINCI)+= ohci-da8xx.o
  obj-$(CONFIG_USB_UHCI_HCD)+= uhci-hcd.o
  obj-$(CONFIG_USB_FHCI_HCD)+= fhci.o
  obj-$(CONFIG_USB_XHCI_HCD)+= xhci-hcd.o
-obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o
+obj-$(CONFIG_USB_XHCI_PCI) += xhci-pci.o xhci-intel-quirks.o
  obj-$(CONFIG_USB_XHCI_PLATFORM) += xhci-plat-hcd.o
  obj-$(CONFIG_USB_XHCI_MTK)+= xhci-mtk.o
  obj-$(CONFIG_USB_XHCI_TEGRA)  += xhci-tegra.o
diff --git a/drivers/usb/host/xhci-intel-quirks.c 
b/drivers/usb/host/xhci-intel-quirks.c
new file mode 100644


I think it would be better to have one place where we add handlers for
vendor specific extended capabilities.

Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
there's a xhci-ext-caps.h header already

We could walk through the capability list once and add the needed handlers.
Something like:

+int xhci_ext_cap_init(void __iomem *base)
+{
+u32 val;
+   u32 cap_offset;
+
+cap_offset = xhci_next_ext_cap(base, 0);
+
+while (cap_offset) {
+val = readl(base + cap_offset);
+
+switch (XHCI_EXT_CAPS_ID(val)) {
+case XHCI_EXT_CAPS_VENDOR_INTEL:
+/* check hw/id/something, and call what's needed */
+break;
+case XHCI_EXT_CAPS_VENDOR_XYZ:
+/* do something */
+break;
+default:
+break;
+}
+
+   printk(KERN_ERR "MATTU EXT_CAP id %d\n", XHCI_EXT_CAPS_ID(val));
+
+   cap_offset = xhci_next_ext_cap(base, cap_offset);
+}
+
+return 0;
+}

xhci_next_ext_cap() doesn't exist anywhere else than my local sandbox branch 
yet.


+
+/* Extended capability IDs for Intel Vendor Defined */
+#define XHCI_EXT_CAPS_INTEL_HOST_CAP   192


XHCI_EXT_CAPS_VENDOR_INTEL
and should be in xhci-ext-caps.h


+
+static void xhci_intel_unregister_pdev(void *arg)
+{
+   platform_device_unregister(arg);
+}
+
+int xhci_create_intel_cht_mux_pdev(struct xhci_hcd *xhci)
+{
+   struct usb_hcd *hcd = xhci_to_hcd(xhci);
+   struct device *dev = hcd->self.controller;
+   struct platform_device *pdev;
+   struct resource res = { 0, };
+   int ret, ext_offset;
+
+   ext_offset = xhci_find_next_ext_cap(&xhci->cap_regs->hc_capbase, 0,
+   XHCI_EXT_CAPS_INTEL_HOST_CAP);
+   if (!ext_offset) {
+   xhci_err(xhci, "couldn't find Intel ext caps\n");
+   return -ENODEV;
+   }
+
+   pdev = platform_device_alloc("intel_cht_usb_mux", PLATFORM_DEVID_NONE);
+   if (!pdev) {
+   xhci_err(xhci, "couldn't allocate intel_cht_usb_mux pdev\n");
+   return -ENOMEM;
+   }
+
+   res.start = hcd->rsrc_start + ext_offset;
+   res.end   = res.start + 0x3ff;
+   res.name  = "intel_cht_usb_mux";
+   res.flags = IORESOURCE_MEM;
+
+   ret = platform_device_add_resources(pdev, &res, 1);
+   if (ret) {
+   dev_err(dev, "couldn't add resources to intel_cht_usb_mux 
pdev\n");
+   platform_device_put(pdev);
+   return ret;
+   }
+
+   pdev->dev.parent = dev;
+
+   ret = platform_device_add(pdev);
+   if (ret) {
+   dev_err(dev, "couldn't register intel_cht_usb_mux pdev\n");
+   platform_device_put(pdev);
+   return ret;
+   }
+
+   ret = devm_add_action_or_reset(dev, xhci_intel_unregister_pdev, pdev);
+   if (ret) {
+   dev_err(dev, "couldn't add unregister action for intel_cht_usb_mux 
pdev\n");
+