Skip to contentSkip to conversation

usb: xhci: allow imod-interval to be configurable

PATCH Superseded Adam Wallis 3 participants 6 messages Dec 1, 2017 – Dec 4, 2017

Subsystems (5)
Device tree bindings USB Arm MediaTek USB xHCI driver +1 more subsystems

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Adam Wallis

author
#0 lore raw

PATCH v3 1/1 usb: xhci: allow imod-interval to be configurable

The xHCI driver currently has the IMOD set to 160, which translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller") introduced a QUIRK for the MTK platform to adjust this interval to 20, which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is due to the fact that the MTK controller IMOD interval is 8 times as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch introduces the ability for vendors to set the IMOD_INTERVAL as is optimal for their platform. By using device_property_read_u32() on "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If no interval is specified, the default of 40,000ns (IMOD=160) will be used.

No bounds checking has been implemented due to the fact that a vendor may have violated the spec and would need to specify a value outside of the max 8,000 IRQs/second limit specified in the xHCI spec.

Signed-off-by Adam Wallis <[email protected]>

---
changes from v2:
* Added PCI default value [Mathias]
* Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
* Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
* Updated bindings Documentation to use proper units [Rob Herring]
* Added imod-interval description and example to MTK binding documentation
changes from v1:
* Removed device_property_read_u32() per suggestion from greg k-h
* Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
 Documentation/devicetree/bindings/usb/usb-xhci.txt          | 1 +
 drivers/usb/host/xhci-mtk.c                                 | 9 +++++++++
 drivers/usb/host/xhci-pci.c                                 | 3 +++
 drivers/usb/host/xhci-plat.c                                | 4 ++++
 drivers/usb/host/xhci.c                                     | 7 ++-----
 drivers/usb/host/xhci.h                                     | 2 ++
 7 files changed, 23 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +2 -0
index 3059596..45bbf18 100644
@@ -46,6 +46,7 @@ Optional properties:
  - pinctrl-names : a pinctrl state named "default" must be defined
  - pinctrl-0 : pin control group
 	See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns
 
 Example:
 usb30: usb at 11270000 {

@@ -66,6 +67,7 @@ usb30: usb at 11270000 {
 	usb3-lpm-capable;
 	mediatek,syscon-wakeup = <&pericfg>;
 	mediatek,wakeup-src = <1>;
+	imod-interval = <10000>;
 };
 
 2nd: dual-role mode with xHCI driver
Documentation/devicetree/bindings/usb/usb-xhci.txt +1 -0
index ae6e484..89b68f1 100644
@@ -29,6 +29,7 @@ Optional properties:
   - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable mechanism
+  - imod-interval: Default interval is 40000ns
 
 Example:
 	usb at f0931000 {
drivers/usb/host/xhci-mtk.c +9 -0
index b62a1d2..278ea3b 100644
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
 	xhci = hcd_to_xhci(hcd);
 	xhci->main_hcd = hcd;
+
+	/*
+	 * imod_interval is the interrupt modulation value in nanoseconds.
+	 * The increment interval is 8 times as much as that defined in
+	 * the xHCI spec on MTK's controller.
+	 */
+	xhci->imod_interval = 5000;
+	device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
 	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
 			dev_name(dev), hcd);
 	if (!xhci->shared_hcd) {
drivers/usb/host/xhci-pci.c +3 -0
index 7ef1274..efbe57b 100644
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
 	if (!xhci->sbrn)
 		pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
 	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
 	if (retval)
 		return retval;
drivers/usb/host/xhci-plat.c +4 -0
index 09f164f..b78be87 100644
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+	device_property_read_u32(sysdev, "imod-interval", &xhci->imod_interval);
+
 	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
 	if (IS_ERR(hcd->usb_phy)) {
 		ret = PTR_ERR(hcd->usb_phy);
drivers/usb/host/xhci.c +2 -5
index 2424d30..0b7755b 100644
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
 			"// Set the interrupt modulation register");
 	temp = readl(&xhci->ir_set->irq_control);
 	temp &= ~ER_IRQ_INTERVAL_MASK;
-	/*
-	 * the increment interval is 8 times as much as that defined
-	 * in xHCI spec on MTK's controller
-	 */
-	temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
+	temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
+
 	writel(temp, &xhci->ir_set->irq_control);
 
 	/* Set the HCD state before we enable the irqs */
drivers/usb/host/xhci.h +2 -0
index 99a014a..2a4177b 100644
@@ -1717,6 +1717,8 @@ struct xhci_hcd {
 	u8		max_interrupters;
 	u8		max_ports;
 	u8		isoc_threshold;
+	/* imod_interval in ns (I * 250ns) */
+	u32		imod_interval;
 	int		event_ring_max;
 	/* 4KB min, 128MB max */
 	int		page_size;
Signature · 3 lines
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
maintainer
#1 lore raw

PATCH v3 1/1 usb: xhci: allow imod-interval to be configurable

Quoted 46 lines · Adam Wallis

The xHCI driver currently has the IMOD set to 160, which translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller") introduced a QUIRK for the MTK platform to adjust this interval to 20, which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is due to the fact that the MTK controller IMOD interval is 8 times as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch introduces the ability for vendors to set the IMOD_INTERVAL as is optimal for their platform. By using device_property_read_u32() on "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If no interval is specified, the default of 40,000ns (IMOD=160) will be used.

No bounds checking has been implemented due to the fact that a vendor may have violated the spec and would need to specify a value outside of the max 8,000 IRQs/second limit specified in the xHCI spec.

Signed-off-by Adam Wallis <[email protected]>

---
changes from v2:
* Added PCI default value [Mathias]
* Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
* Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
* Updated bindings Documentation to use proper units [Rob Herring]
* Added imod-interval description and example to MTK binding documentation
changes from v1:
* Removed device_property_read_u32() per suggestion from greg k-h
* Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
 Documentation/devicetree/bindings/usb/usb-xhci.txt          | 1 +
 drivers/usb/host/xhci-mtk.c                                 | 9 +++++++++
 drivers/usb/host/xhci-pci.c                                 | 3 +++
 drivers/usb/host/xhci-plat.c                                | 4 ++++
 drivers/usb/host/xhci.c                                     | 7 ++-----
 drivers/usb/host/xhci.h                                     | 2 ++
 7 files changed, 23 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +0 -0
index 3059596..45bbf18 100644

Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt Optional properties: · line 49

Quoted 4 lines
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
@@ -46,6 +46,7 @@ Optional properties:
  - pinctrl-names : a pinctrl state named "default" must be defined
  - pinctrl-0 : pin control group
 	See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns

I think, as Rob suggested before, recommend to have a unit suffix appended to the property name. s/imod-interval/imod-interval-ns

Quoted 103 lines
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
 Example:
 usb30: usb at 11270000 {

@@ -66,6 +67,7 @@ usb30: usb at 11270000 {
 	usb3-lpm-capable;
 	mediatek,syscon-wakeup = <&pericfg>;
 	mediatek,wakeup-src = <1>;
+	imod-interval = <10000>;
 };
 
 2nd: dual-role mode with xHCI driver
Documentation/devicetree/bindings/usb/usb-xhci.txt +1 -0
index ae6e484..89b68f1 100644
@@ -29,6 +29,7 @@ Optional properties:
   - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable mechanism
+  - imod-interval: Default interval is 40000ns
 
 Example:
 	usb at f0931000 {
drivers/usb/host/xhci-mtk.c +9 -0
index b62a1d2..278ea3b 100644
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
 	xhci = hcd_to_xhci(hcd);
 	xhci->main_hcd = hcd;
+
+	/*
+	 * imod_interval is the interrupt modulation value in nanoseconds.
+	 * The increment interval is 8 times as much as that defined in
+	 * the xHCI spec on MTK's controller.
+	 */
+	xhci->imod_interval = 5000;
+	device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
 	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
 			dev_name(dev), hcd);
 	if (!xhci->shared_hcd) {
drivers/usb/host/xhci-pci.c +3 -0
index 7ef1274..efbe57b 100644
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
 	if (!xhci->sbrn)
 		pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
 	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
 	if (retval)
 		return retval;
drivers/usb/host/xhci-plat.c +4 -0
index 09f164f..b78be87 100644
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+	device_property_read_u32(sysdev, "imod-interval", &xhci->imod_interval);
+
 	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
 	if (IS_ERR(hcd->usb_phy)) {
 		ret = PTR_ERR(hcd->usb_phy);
drivers/usb/host/xhci.c +2 -5
index 2424d30..0b7755b 100644
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
 			"// Set the interrupt modulation register");
 	temp = readl(&xhci->ir_set->irq_control);
 	temp &= ~ER_IRQ_INTERVAL_MASK;
-	/*
-	 * the increment interval is 8 times as much as that defined
-	 * in xHCI spec on MTK's controller
-	 */
-	temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
+	temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
+
 	writel(temp, &xhci->ir_set->irq_control);
 
 	/* Set the HCD state before we enable the irqs */
drivers/usb/host/xhci.h +2 -0
index 99a014a..2a4177b 100644
@@ -1717,6 +1717,8 @@ struct xhci_hcd {
 	u8		max_interrupters;
 	u8		max_ports;
 	u8		isoc_threshold;
+	/* imod_interval in ns (I * 250ns) */
+	u32		imod_interval;
 	int		event_ring_max;
 	/* 4KB min, 128MB max */
 	int		page_size;
#2 lore raw

PATCH v3 1/1 usb: xhci: allow imod-interval to be configurable

Quoted 120 lines · Chunfeng Yun
Quoted 46 lines · Adam Wallis

The xHCI driver currently has the IMOD set to 160, which translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller") introduced a QUIRK for the MTK platform to adjust this interval to 20, which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is due to the fact that the MTK controller IMOD interval is 8 times as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch introduces the ability for vendors to set the IMOD_INTERVAL as is optimal for their platform. By using device_property_read_u32() on "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If no interval is specified, the default of 40,000ns (IMOD=160) will be used.

No bounds checking has been implemented due to the fact that a vendor may have violated the spec and would need to specify a value outside of the max 8,000 IRQs/second limit specified in the xHCI spec.

Signed-off-by Adam Wallis <[email protected]>

---
changes from v2:
* Added PCI default value [Mathias]
* Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
* Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
* Updated bindings Documentation to use proper units [Rob Herring]
* Added imod-interval description and example to MTK binding documentation
changes from v1:
* Removed device_property_read_u32() per suggestion from greg k-h
* Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
  Documentation/devicetree/bindings/usb/usb-xhci.txt          | 1 +
  drivers/usb/host/xhci-mtk.c                                 | 9 +++++++++
  drivers/usb/host/xhci-pci.c                                 | 3 +++
  drivers/usb/host/xhci-plat.c                                | 4 ++++
  drivers/usb/host/xhci.c                                     | 7 ++-----
  drivers/usb/host/xhci.h                                     | 2 ++
  7 files changed, 23 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +0 -0
index 3059596..45bbf18 100644

Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt Optional properties: · line 49

Quoted 4 lines
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
@@ -46,6 +46,7 @@ Optional properties:
   - pinctrl-names : a pinctrl state named "default" must be defined
   - pinctrl-0 : pin control group
  	See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns

I think, as Rob suggested before, recommend to have a unit suffix appended to the property name. s/imod-interval/imod-interval-ns

Quoted 65 lines
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
  Example:
  usb30: usb at 11270000 {

@@ -66,6 +67,7 @@ usb30: usb at 11270000 {
  	usb3-lpm-capable;
  	mediatek,syscon-wakeup = <&pericfg>;
  	mediatek,wakeup-src = <1>;
+	imod-interval = <10000>;
  };
 
  2nd: dual-role mode with xHCI driver
Documentation/devicetree/bindings/usb/usb-xhci.txt +1 -0
index ae6e484..89b68f1 100644
@@ -29,6 +29,7 @@ Optional properties:
    - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
    - usb3-lpm-capable: determines if platform is USB3 LPM capable
    - quirk-broken-port-ped: set if the controller has broken port disable mechanism
+  - imod-interval: Default interval is 40000ns
 
  Example:
  	usb at f0931000 {
drivers/usb/host/xhci-mtk.c +9 -0
index b62a1d2..278ea3b 100644
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
  	xhci = hcd_to_xhci(hcd);
  	xhci->main_hcd = hcd;
+
+	/*
+	 * imod_interval is the interrupt modulation value in nanoseconds.
+	 * The increment interval is 8 times as much as that defined in
+	 * the xHCI spec on MTK's controller.
+	 */
+	xhci->imod_interval = 5000;
+	device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
  	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
  			dev_name(dev), hcd);
  	if (!xhci->shared_hcd) {
drivers/usb/host/xhci-pci.c +3 -0
index 7ef1274..efbe57b 100644
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
  	if (!xhci->sbrn)
  		pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
  	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
  	if (retval)
  		return retval;
drivers/usb/host/xhci-plat.c +0 -0
index 09f164f..b78be87 100644
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
  	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
  		xhci->quirks |= XHCI_BROKEN_PORT_PED;
Quoted 13 lines

drivers/usb/host/xhci.c int xhci_run(struct usb_hcd *hcd) · line 586

Quoted 13 lines
drivers/usb/host/xhci-plat.c +4 -0
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+	device_property_read_u32(sysdev, "imod-interval", &xhci->imod_interval);
+
  	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
  	if (IS_ERR(hcd->usb_phy)) {
  		ret = PTR_ERR(hcd->usb_phy);
drivers/usb/host/xhci.c +0 -0
index 2424d30..0b7755b 100644
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
  			"// Set the interrupt modulation register");

Just noticed the driver has all the time incorrectly used the word "modulation" instead of "moderation".

If you do the bindings change that Chunfeng pointed out above could you also change the "modulation" to "moderation" in this patch.

Don't worry about changing the old ones. There's a cleanup patch on its way that will remove most of them anyway.

Otherwise the xhci parts look good to me.

-Mathias

author
#3 lore raw

PATCH v3 1/1 usb: xhci: allow imod-interval to be configurable

Quoted 136 lines · Mathias Nyman
Quoted 122 lines · Chunfeng Yun
Quoted 47 lines · Adam Wallis

The xHCI driver currently has the IMOD set to 160, which translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller") introduced a QUIRK for the MTK platform to adjust this interval to 20, which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is due to the fact that the MTK controller IMOD interval is 8 times as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch introduces the ability for vendors to set the IMOD_INTERVAL as is optimal for their platform. By using device_property_read_u32() on "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If no interval is specified, the default of 40,000ns (IMOD=160) will be used.

No bounds checking has been implemented due to the fact that a vendor may have violated the spec and would need to specify a value outside of the max 8,000 IRQs/second limit specified in the xHCI spec.

Signed-off-by Adam Wallis <[email protected]>

---
changes from v2:
?? * Added PCI default value [Mathias]
?? * Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
?? * Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng] ?? * Updated bindings Documentation to use proper units [Rob Herring] ?? * Added imod-interval description and example to MTK binding documentation changes from v1:
?? * Removed device_property_read_u32() per suggestion from greg k-h ?? * Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

? Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
? Documentation/devicetree/bindings/usb/usb-xhci.txt????????? | 1 +
? drivers/usb/host/xhci-mtk.c???????????????????????????????? | 9 +++++++++
? drivers/usb/host/xhci-pci.c???????????????????????????????? | 3 +++
? drivers/usb/host/xhci-plat.c??????????????????????????????? | 4 ++++
? drivers/usb/host/xhci.c???????????????????????????????????? | 7 ++-----
? drivers/usb/host/xhci.h???????????????????????????????????? | 2 ++

? 7 files changed, 23 insertions(+), 5 deletions(-)

Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +0 -0
index 3059596..45bbf18 100644
@@ -46,6 +46,7 @@ Optional properties:
Quoted 4 lines

?? - pinctrl-names : a pinctrl state named "default" must be defined ?? - pinctrl-0 : pin control group
????? See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns

I think, as Rob suggested before, recommend to have a unit suffix appended to the property name. s/imod-interval/imod-interval-ns
?

Quoted 66 lines

? ? Example:
? usb30: usb at 11270000 {

Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
@@ -66,6 +67,7 @@ usb30: usb at 11270000 {
 ????? usb3-lpm-capable;
 ????? mediatek,syscon-wakeup = <&pericfg>;
 ????? mediatek,wakeup-src = <1>;
+??? imod-interval = <10000>;
 ? };

? ? 2nd: dual-role mode with xHCI driver

Documentation/devicetree/bindings/usb/usb-xhci.txt +0 -0
index ae6e484..89b68f1 100644
@@ -29,6 +29,7 @@ Optional properties:

??? - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM ??? - usb3-lpm-capable: determines if platform is USB3 LPM capable ??? - quirk-broken-port-ped: set if the controller has broken port disable mechanism
+? - imod-interval: Default interval is 40000ns
? ? Example:
????? usb at f0931000 {

drivers/usb/host/xhci-mtk.c +9 -0
index b62a1d2..278ea3b 100644
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 ? ????? xhci = hcd_to_xhci(hcd);
 ????? xhci->main_hcd = hcd;
+
+??? /*
+???? * imod_interval is the interrupt modulation value in nanoseconds.
+???? * The increment interval is 8 times as much as that defined in
+???? * the xHCI spec on MTK's controller.
+???? */
+??? xhci->imod_interval = 5000;
+??? device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
 ????? xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
 ????????????? dev_name(dev), hcd);
 ????? if (!xhci->shared_hcd) {
drivers/usb/host/xhci-pci.c +0 -0
index 7ef1274..efbe57b 100644
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
 ????? if (!xhci->sbrn)
 ????????? pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);

? +??? /* imod_interval is the interrupt modulation value in nanoseconds. */ +??? xhci->imod_interval = 40000;
+
????? retval = xhci_gen_setup(hcd, xhci_pci_quirks);
????? if (retval)
????????? return retval;

drivers/usb/host/xhci-plat.c +0 -0
index 09f164f..b78be87 100644
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 ????? if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 ????????? xhci->quirks |= XHCI_BROKEN_PORT_PED;

? +??? /* imod_interval is the interrupt modulation value in nanoseconds. */ +??? xhci->imod_interval = 40000;
+??? device_property_read_u32(sysdev, "imod-interval", &xhci->imod_interval); +
????? hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);

Quoted 8 lines

drivers/usb/host/xhci.c int xhci_run(struct usb_hcd *hcd) · line 586

Quoted 8 lines

????? if (IS_ERR(hcd->usb_phy)) {
????????? ret = PTR_ERR(hcd->usb_phy);

drivers/usb/host/xhci.c +0 -0
index 2424d30..0b7755b 100644
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
 ????????????? "// Set the interrupt modulation register");

Just noticed the driver has all the time incorrectly used the word "modulation" instead
of "moderation".

Quoted 2 lines

If you do the bindings change that Chunfeng pointed out above could you also change the "modulation" to "moderation" in this patch.

Sure thing, I will make the change for this patch.

Quoted 12 lines

Don't worry about changing the old ones. There's a cleanup patch on its way that will remove most of them anyway.

Otherwise the xhci parts look good to me.

-Mathias

Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at? http://vger.kernel.org/majordomo-info.html
Signature · 4 lines
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
author
#4 lore raw

PATCH v3 1/1 usb: xhci: allow imod-interval to be configurable

In reply to Chunfeng Yun
Quoted 42 lines · Chunfeng Yun
Quoted 41 lines · Adam Wallis

The xHCI driver currently has the IMOD set to 160, which translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller") introduced a QUIRK for the MTK platform to adjust this interval to 20, which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is due to the fact that the MTK controller IMOD interval is 8 times as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch introduces the ability for vendors to set the IMOD_INTERVAL as is optimal for their platform. By using device_property_read_u32() on "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If no interval is specified, the default of 40,000ns (IMOD=160) will be used.

No bounds checking has been implemented due to the fact that a vendor may have violated the spec and would need to specify a value outside of the max 8,000 IRQs/second limit specified in the xHCI spec.

Signed-off-by Adam Wallis <[email protected]>

---
changes from v2:
* Added PCI default value [Mathias]
* Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
* Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
* Updated bindings Documentation to use proper units [Rob Herring]
* Added imod-interval description and example to MTK binding documentation
changes from v1:
* Removed device_property_read_u32() per suggestion from greg k-h
* Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
 Documentation/devicetree/bindings/usb/usb-xhci.txt          | 1 +
 drivers/usb/host/xhci-mtk.c                                 | 9 +++++++++
 drivers/usb/host/xhci-pci.c                                 | 3 +++
 drivers/usb/host/xhci-plat.c                                | 4 ++++
 drivers/usb/host/xhci.c                                     | 7 ++-----
 drivers/usb/host/xhci.h                                     | 2 ++
 7 files changed, 23 insertions(+), 5 deletions(-)
Quoted 12 lines

Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt Optional properties: · line 49

Quoted 9 lines
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
index 3059596..45bbf18 100644
@@ -46,6 +46,7 @@ Optional properties:
  - pinctrl-names : a pinctrl state named "default" must be defined
  - pinctrl-0 : pin control group
 	See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns

I think, as Rob suggested before, recommend to have a unit suffix appended to the property name. s/imod-interval/imod-interval-ns

Thanks, I definitely misunderstood his comments the first go around. I will make the change.

Quoted 111 lines
Quoted 95 lines
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +1 -0
 Example:
 usb30: usb at 11270000 {

@@ -66,6 +67,7 @@ usb30: usb at 11270000 {
 	usb3-lpm-capable;
 	mediatek,syscon-wakeup = <&pericfg>;
 	mediatek,wakeup-src = <1>;
+	imod-interval = <10000>;
 };
 
 2nd: dual-role mode with xHCI driver
Documentation/devicetree/bindings/usb/usb-xhci.txt +1 -0
index ae6e484..89b68f1 100644
@@ -29,6 +29,7 @@ Optional properties:
   - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable mechanism
+  - imod-interval: Default interval is 40000ns
 
 Example:
 	usb at f0931000 {
drivers/usb/host/xhci-mtk.c +9 -0
index b62a1d2..278ea3b 100644
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
 	xhci = hcd_to_xhci(hcd);
 	xhci->main_hcd = hcd;
+
+	/*
+	 * imod_interval is the interrupt modulation value in nanoseconds.
+	 * The increment interval is 8 times as much as that defined in
+	 * the xHCI spec on MTK's controller.
+	 */
+	xhci->imod_interval = 5000;
+	device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
 	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
 			dev_name(dev), hcd);
 	if (!xhci->shared_hcd) {
drivers/usb/host/xhci-pci.c +3 -0
index 7ef1274..efbe57b 100644
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
 	if (!xhci->sbrn)
 		pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
 	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
 	if (retval)
 		return retval;
drivers/usb/host/xhci-plat.c +4 -0
index 09f164f..b78be87 100644
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+	device_property_read_u32(sysdev, "imod-interval", &xhci->imod_interval);
+
 	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
 	if (IS_ERR(hcd->usb_phy)) {
 		ret = PTR_ERR(hcd->usb_phy);
drivers/usb/host/xhci.c +2 -5
index 2424d30..0b7755b 100644
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
 			"// Set the interrupt modulation register");
 	temp = readl(&xhci->ir_set->irq_control);
 	temp &= ~ER_IRQ_INTERVAL_MASK;
-	/*
-	 * the increment interval is 8 times as much as that defined
-	 * in xHCI spec on MTK's controller
-	 */
-	temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
+	temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
+
 	writel(temp, &xhci->ir_set->irq_control);
 
 	/* Set the HCD state before we enable the irqs */
drivers/usb/host/xhci.h +0 -0
index 99a014a..2a4177b 100644

drivers/usb/host/xhci.h struct xhci_hcd { · line 1724

Quoted 8 lines
drivers/usb/host/xhci.h +2 -0
@@ -1717,6 +1717,8 @@ struct xhci_hcd {
 	u8		max_interrupters;
 	u8		max_ports;
 	u8		isoc_threshold;
+	/* imod_interval in ns (I * 250ns) */
+	u32		imod_interval;
 	int		event_ring_max;
 	/* 4KB min, 128MB max */
 	int		page_size;

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Signature · 4 lines
Adam Wallis
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
maintainer
#5 lore raw

PATCH v3 1/1 usb: xhci: allow imod-interval to be configurable

In reply to Adam Wallis
Quoted 145 lines · Adam Wallis

The xHCI driver currently has the IMOD set to 160, which translates to an IMOD interval of 40,000ns (160 * 250)ns

Commit 0cbd4b34cda9 ("xhci: mediatek: support MTK xHCI host controller") introduced a QUIRK for the MTK platform to adjust this interval to 20, which translates to an IMOD interval of 5,000ns (20 * 250)ns. This is due to the fact that the MTK controller IMOD interval is 8 times as much as defined in xHCI spec.

Instead of adding more quirk bits for additional platforms, this patch introduces the ability for vendors to set the IMOD_INTERVAL as is optimal for their platform. By using device_property_read_u32() on "imod-interval", the IMOD INTERVAL can be specified in nano seconds. If no interval is specified, the default of 40,000ns (IMOD=160) will be used.

No bounds checking has been implemented due to the fact that a vendor may have violated the spec and would need to specify a value outside of the max 8,000 IRQs/second limit specified in the xHCI spec.

Signed-off-by Adam Wallis <[email protected]>

---
changes from v2:
* Added PCI default value [Mathias]
* Removed xhci-mtk.h from xhci-plat.c [Chunfeng Yun]
* Removed MTK quirk from xhci-plat and moved logic to xhci-mtk [Chunfeng]
* Updated bindings Documentation to use proper units [Rob Herring]
* Added imod-interval description and example to MTK binding documentation
changes from v1:
* Removed device_property_read_u32() per suggestion from greg k-h
* Used ER_IRQ_INTERVAL_MASK in place of (u16) cast

 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt | 2 ++
 Documentation/devicetree/bindings/usb/usb-xhci.txt          | 1 +
 drivers/usb/host/xhci-mtk.c                                 | 9 +++++++++
 drivers/usb/host/xhci-pci.c                                 | 3 +++
 drivers/usb/host/xhci-plat.c                                | 4 ++++
 drivers/usb/host/xhci.c                                     | 7 ++-----
 drivers/usb/host/xhci.h                                     | 2 ++
 7 files changed, 23 insertions(+), 5 deletions(-)
Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt +2 -0
index 3059596..45bbf18 100644
@@ -46,6 +46,7 @@ Optional properties:
  - pinctrl-names : a pinctrl state named "default" must be defined
  - pinctrl-0 : pin control group
 	See: Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+ - imod-interval: Default interval is 5000ns
 
 Example:
 usb30: usb at 11270000 {

@@ -66,6 +67,7 @@ usb30: usb at 11270000 {
 	usb3-lpm-capable;
 	mediatek,syscon-wakeup = <&pericfg>;
 	mediatek,wakeup-src = <1>;
+	imod-interval = <10000>;
 };
 
 2nd: dual-role mode with xHCI driver
Documentation/devicetree/bindings/usb/usb-xhci.txt +1 -0
index ae6e484..89b68f1 100644
@@ -29,6 +29,7 @@ Optional properties:
   - usb2-lpm-disable: indicate if we don't want to enable USB2 HW LPM
   - usb3-lpm-capable: determines if platform is USB3 LPM capable
   - quirk-broken-port-ped: set if the controller has broken port disable mechanism
+  - imod-interval: Default interval is 40000ns
 
 Example:
 	usb at f0931000 {
drivers/usb/host/xhci-mtk.c +9 -0
index b62a1d2..278ea3b 100644
@@ -674,6 +674,15 @@ static int xhci_mtk_probe(struct platform_device *pdev)
 
 	xhci = hcd_to_xhci(hcd);
 	xhci->main_hcd = hcd;
+
+	/*
+	 * imod_interval is the interrupt modulation value in nanoseconds.
+	 * The increment interval is 8 times as much as that defined in
+	 * the xHCI spec on MTK's controller.
+	 */
+	xhci->imod_interval = 5000;
+	device_property_read_u32(dev, "imod-interval", &xhci->imod_interval);
+
 	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
 			dev_name(dev), hcd);
 	if (!xhci->shared_hcd) {
drivers/usb/host/xhci-pci.c +3 -0
index 7ef1274..efbe57b 100644
@@ -234,6 +234,9 @@ static int xhci_pci_setup(struct usb_hcd *hcd)
 	if (!xhci->sbrn)
 		pci_read_config_byte(pdev, XHCI_SBRN_OFFSET, &xhci->sbrn);
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
 	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
 	if (retval)
 		return retval;
drivers/usb/host/xhci-plat.c +4 -0
index 09f164f..b78be87 100644
@@ -269,6 +269,10 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (device_property_read_bool(&pdev->dev, "quirk-broken-port-ped"))
 		xhci->quirks |= XHCI_BROKEN_PORT_PED;
 
+	/* imod_interval is the interrupt modulation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+	device_property_read_u32(sysdev, "imod-interval", &xhci->imod_interval);
+
 	hcd->usb_phy = devm_usb_get_phy_by_phandle(sysdev, "usb-phy", 0);
 	if (IS_ERR(hcd->usb_phy)) {
 		ret = PTR_ERR(hcd->usb_phy);
drivers/usb/host/xhci.c +2 -5
index 2424d30..0b7755b 100644
@@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
 			"// Set the interrupt modulation register");
 	temp = readl(&xhci->ir_set->irq_control);
 	temp &= ~ER_IRQ_INTERVAL_MASK;
-	/*
-	 * the increment interval is 8 times as much as that defined
-	 * in xHCI spec on MTK's controller
-	 */
-	temp |= (u32) ((xhci->quirks & XHCI_MTK_HOST) ? 20 : 160);
+	temp |= (xhci->imod_interval / 250) & ER_IRQ_INTERVAL_MASK;
+
 	writel(temp, &xhci->ir_set->irq_control);
 
 	/* Set the HCD state before we enable the irqs */
drivers/usb/host/xhci.h +0 -0
index 99a014a..2a4177b 100644

drivers/usb/host/xhci.h struct xhci_hcd { · line 1724

Quoted 8 lines
drivers/usb/host/xhci.h +2 -0
@@ -1717,6 +1717,8 @@ struct xhci_hcd {
 	u8		max_interrupters;
 	u8		max_ports;
 	u8		isoc_threshold;
+	/* imod_interval in ns (I * 250ns) */
+	u32		imod_interval;
 	int		event_ring_max;
 	/* 4KB min, 128MB max */
 	int		page_size;
Tested-by Chunfeng Yun <[email protected]>

Thanks