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 <mathias.ny...@linux.intel.com>
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 <mathias.ny...@linux.intel.com>
---
 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 <mathias.ny...@linux.intel.com>
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 <mathias.ny...@linux.intel.com>
---
 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;
+	u32 cap_offset;
+	u32 val;
+
+	base = &xhci->cap_regs->hc_capbase;
+	cap_offset = xhci_find_next_ext_cap(base, 0, 0);
+
+	do {
+		val = readl(base + cap_offset);
+
+		switch (XHCI_EXT_CAPS_ID(val)) {
+		case XHCI_EXT_CAPS_VENDOR_INTEL:
+			printk(KERN_ERR "TEST EXT_CAPS_VENDOR_INTEL\n");
+			break;
+		default:
+			break;
+		}
+
+		printk(KERN_ERR "TEST EXT_CAP id %d\n", XHCI_EXT_CAPS_ID(val));
+
+		cap_offset = xhci_find_next_ext_cap(base, cap_offset, 0);
+	} while (cap_offset);
+
+	return 0;
+}
+
 int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 {
 	struct xhci_hcd		*xhci;
@@ -4886,6 +4914,8 @@ int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks)
 		dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	}
 
+	xhci_ext_cap_init(xhci);
+
 	xhci_dbg(xhci, "Calling HCD init\n");
 	/* Initialize HCD and host controller data structures. */
 	retval = xhci_init(hcd);
-- 
1.9.1

Reply via email to