[PATCH v4] usb: xhci: allow imod-interval to be configurable

[PATCH v4] usb: xhci: allow imod-interval to be configurable

[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-ns", 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.

Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
changes from v3:
  * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
  * Changed "modulation" to "moderation" throughout patch [Mathias]
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                                | 5 +++++
 drivers/usb/host/xhci.c                                     | 7 ++-----
 drivers/usb/host/xhci.h                                     | 2 ++
 7 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
index 3059596..9ff5602 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
@@ -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-ns: default interrupt moderation interval is 5000ns
 
 Example:
 usb30: usb@11270000 {
@@ -66,6 +67,7 @@ usb30: usb@11270000 {
 	usb3-lpm-capable;
 	mediatek,syscon-wakeup = <&pericfg>;
 	mediatek,wakeup-src = <1>;
+	imod-interval-ns = <10000>;
 };
 
 2nd: dual-role mode with xHCI driver
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
index ae6e484..969908d 100644
--- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -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-ns: default interrupt moderation interval is 5000ns
 
 Example:
 	usb@f0931000 {
diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index b62a1d2..1cb2a8b 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -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 moderation 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-ns", &xhci->imod_interval);
+
 	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
 			dev_name(dev), hcd);
 	if (!xhci->shared_hcd) {
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 7ef1274..4bcddd4 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -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 moderation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+
 	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
 	if (retval)
 		return retval;
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 09f164f..6f03830 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -269,6 +269,11 @@ 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 moderation value in nanoseconds. */
+	xhci->imod_interval = 40000;
+	device_property_read_u32(sysdev, "imod-interval-ns",
+				 &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);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2424d30..0b7755b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -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 */
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 99a014a..2a4177b 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -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;
-- 
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.

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Timur Tabi]

On 12/4/17 8:27 AM, Adam Wallis wrote:
> If no interval is specified, the default of 40,000ns (IMOD=160) will be
> used.

...

> + - imod-interval-ns: default interrupt moderation interval is 5000ns

...

> +  - imod-interval-ns: default interrupt moderation interval is 5000ns

...

> +	xhci->imod_interval = 5000;

...

> +	xhci->imod_interval = 40000;

...

> +	xhci->imod_interval = 40000;

Is the default 5,000 or 40,000?  I can't tell.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Adam Wallis]

Timur

On 12/4/2017 9:58 AM, Timur Tabi wrote:
> On 12/4/17 8:27 AM, Adam Wallis wrote:
>> If no interval is specified, the default of 40,000ns (IMOD=160) will be
>> used.
> 
> ...
> 
>> + - imod-interval-ns: default interrupt moderation interval is 5000ns
> 
> ...
> 
>> +  - imod-interval-ns: default interrupt moderation interval is 5000ns
> 
> ...
> 
>> +    xhci->imod_interval = 5000;
> 
> ...
> 
>> +    xhci->imod_interval = 40000;
> 
> ...
> 
>> +    xhci->imod_interval = 40000;
> 
> Is the default 5,000 or 40,000?  I can't tell.
> 
As noted in the patch description, xhci-plat devices will use a default of
40,000, while MTK devices will use a default of 5,000. This is also documented
in the probe for each relevant driver. This is further confirmed in the
description for each driver's binding TXT file.

Adam

-- 
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.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Timur Tabi]

On 12/04/2017 09:48 AM, Adam Wallis wrote:
> As noted in the patch description, xhci-plat devices will use a default of
> 40,000, while MTK devices will use a default of 5,000. This is also documented
> in the probe for each relevant driver. This is further confirmed in the
> description for each driver's binding TXT file.

Serves me right for reviewing patches before I've had my morning coffee.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Chunfeng Yun]

On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
> 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-ns", 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.
> 
> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> changes from v3:
>   * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
>   * Changed "modulation" to "moderation" throughout patch [Mathias]
> 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                                | 5 +++++
>  drivers/usb/host/xhci.c                                     | 7 ++-----
>  drivers/usb/host/xhci.h                                     | 2 ++
>  7 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> index 3059596..9ff5602 100644
> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
> @@ -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-ns: default interrupt moderation interval is 5000ns
>  
>  Example:
>  usb30: usb@11270000 {
> @@ -66,6 +67,7 @@ usb30: usb@11270000 {
>  	usb3-lpm-capable;
>  	mediatek,syscon-wakeup = <&pericfg>;
>  	mediatek,wakeup-src = <1>;
> +	imod-interval-ns = <10000>;
>  };
>  
>  2nd: dual-role mode with xHCI driver
> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> index ae6e484..969908d 100644
> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
> @@ -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-ns: default interrupt moderation interval is 5000ns
>  
>  Example:
>  	usb@f0931000 {
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index b62a1d2..1cb2a8b 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -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 moderation 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-ns", &xhci->imod_interval);
> +
>  	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
>  			dev_name(dev), hcd);
>  	if (!xhci->shared_hcd) {
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 7ef1274..4bcddd4 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -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 moderation value in nanoseconds. */
> +	xhci->imod_interval = 40000;
> +
>  	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
>  	if (retval)
>  		return retval;
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 09f164f..6f03830 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -269,6 +269,11 @@ 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 moderation value in nanoseconds. */
> +	xhci->imod_interval = 40000;
> +	device_property_read_u32(sysdev, "imod-interval-ns",
> +				 &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);
> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
> index 2424d30..0b7755b 100644
> --- a/drivers/usb/host/xhci.c
> +++ b/drivers/usb/host/xhci.c
> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
>  			"// Set the interrupt modulation register");
s/modulation/moderation

>  	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;
> +
No need a blank line

>  	writel(temp, &xhci->ir_set->irq_control);
>  
>  	/* Set the HCD state before we enable the irqs */
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index 99a014a..2a4177b 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -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;

Thanks


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Adam Wallis]

On 12/4/2017 9:15 PM, Chunfeng Yun wrote:
> On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
>> 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-ns", 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.
>>
>> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>> changes from v3:
>>   * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
>>   * Changed "modulation" to "moderation" throughout patch [Mathias]
>> 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                                | 5 +++++
>>  drivers/usb/host/xhci.c                                     | 7 ++-----
>>  drivers/usb/host/xhci.h                                     | 2 ++
>>  7 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> index 3059596..9ff5602 100644
>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>> @@ -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-ns: default interrupt moderation interval is 5000ns
>>  
>>  Example:
>>  usb30: usb@11270000 {
>> @@ -66,6 +67,7 @@ usb30: usb@11270000 {
>>  	usb3-lpm-capable;
>>  	mediatek,syscon-wakeup = <&pericfg>;
>>  	mediatek,wakeup-src = <1>;
>> +	imod-interval-ns = <10000>;
>>  };
>>  
>>  2nd: dual-role mode with xHCI driver
>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> index ae6e484..969908d 100644
>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>> @@ -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-ns: default interrupt moderation interval is 5000ns
>>  
>>  Example:
>>  	usb@f0931000 {
>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
>> index b62a1d2..1cb2a8b 100644
>> --- a/drivers/usb/host/xhci-mtk.c
>> +++ b/drivers/usb/host/xhci-mtk.c
>> @@ -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 moderation 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-ns", &xhci->imod_interval);
>> +
>>  	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
>>  			dev_name(dev), hcd);
>>  	if (!xhci->shared_hcd) {
>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>> index 7ef1274..4bcddd4 100644
>> --- a/drivers/usb/host/xhci-pci.c
>> +++ b/drivers/usb/host/xhci-pci.c
>> @@ -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 moderation value in nanoseconds. */
>> +	xhci->imod_interval = 40000;
>> +
>>  	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
>>  	if (retval)
>>  		return retval;
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 09f164f..6f03830 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -269,6 +269,11 @@ 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 moderation value in nanoseconds. */
>> +	xhci->imod_interval = 40000;
>> +	device_property_read_u32(sysdev, "imod-interval-ns",
>> +				 &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);
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index 2424d30..0b7755b 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
>>  			"// Set the interrupt modulation register");
> s/modulation/moderation

Mathias said there was no need to change the existing modulation strings - only
the ones that I had added.

> 
>>  	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;
>> +
> No need a blank line

If this patch goes through another version, I will remove this line

> 
>>  	writel(temp, &xhci->ir_set->irq_control);
>>  
>>  	/* Set the HCD state before we enable the irqs */
>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> index 99a014a..2a4177b 100644
>> --- a/drivers/usb/host/xhci.h
>> +++ b/drivers/usb/host/xhci.h
>> @@ -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;
> 
> Thanks
> 
> 


-- 
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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Mathias Nyman]

On 05.12.2017 04:54, Adam Wallis wrote:
> On 12/4/2017 9:15 PM, Chunfeng Yun wrote:
>> On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
>>> 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-ns", 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.
>>>
>>> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
>>> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>> ---
>>> changes from v3:
>>>    * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
>>>    * Changed "modulation" to "moderation" throughout patch [Mathias]
>>> 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                                | 5 +++++
>>>   drivers/usb/host/xhci.c                                     | 7 ++-----
>>>   drivers/usb/host/xhci.h                                     | 2 ++
>>>   7 files changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>>> index 3059596..9ff5602 100644
>>> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>>> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.txt
>>> @@ -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-ns: default interrupt moderation interval is 5000ns
>>>   
>>>   Example:
>>>   usb30: usb@11270000 {
>>> @@ -66,6 +67,7 @@ usb30: usb@11270000 {
>>>   	usb3-lpm-capable;
>>>   	mediatek,syscon-wakeup = <&pericfg>;
>>>   	mediatek,wakeup-src = <1>;
>>> +	imod-interval-ns = <10000>;
>>>   };
>>>   
>>>   2nd: dual-role mode with xHCI driver
>>> diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> index ae6e484..969908d 100644
>>> --- a/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> +++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
>>> @@ -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-ns: default interrupt moderation interval is 5000ns
>>>   
>>>   Example:
>>>   	usb@f0931000 {
>>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
>>> index b62a1d2..1cb2a8b 100644
>>> --- a/drivers/usb/host/xhci-mtk.c
>>> +++ b/drivers/usb/host/xhci-mtk.c
>>> @@ -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 moderation 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-ns", &xhci->imod_interval);
>>> +
>>>   	xhci->shared_hcd = usb_create_shared_hcd(driver, dev,
>>>   			dev_name(dev), hcd);
>>>   	if (!xhci->shared_hcd) {
>>> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
>>> index 7ef1274..4bcddd4 100644
>>> --- a/drivers/usb/host/xhci-pci.c
>>> +++ b/drivers/usb/host/xhci-pci.c
>>> @@ -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 moderation value in nanoseconds. */
>>> +	xhci->imod_interval = 40000;
>>> +
>>>   	retval = xhci_gen_setup(hcd, xhci_pci_quirks);
>>>   	if (retval)
>>>   		return retval;
>>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>>> index 09f164f..6f03830 100644
>>> --- a/drivers/usb/host/xhci-plat.c
>>> +++ b/drivers/usb/host/xhci-plat.c
>>> @@ -269,6 +269,11 @@ 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 moderation value in nanoseconds. */
>>> +	xhci->imod_interval = 40000;
>>> +	device_property_read_u32(sysdev, "imod-interval-ns",
>>> +				 &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);
>>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>>> index 2424d30..0b7755b 100644
>>> --- a/drivers/usb/host/xhci.c
>>> +++ b/drivers/usb/host/xhci.c
>>> @@ -586,11 +586,8 @@ int xhci_run(struct usb_hcd *hcd)
>>>   			"// Set the interrupt modulation register");
>> s/modulation/moderation
> 
> Mathias said there was no need to change the existing modulation strings - only
> the ones that I had added.

Baolu is working on a cleanup patch that removes debug messages that became
useless with debugfs and proper tracing support, including this line.

Less conflicts to resolve for me if we don't change this.

> 
>>
>>>   	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;
>>> +
>> No need a blank line
> 
> If this patch goes through another version, I will remove this line

If Rob Acks this version I'll apply it and remove that blank line.

-Mathias
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Adam Wallis]

Rob,

On 12/5/2017 3:48 AM, Mathias Nyman wrote:
> On 05.12.2017 04:54, Adam Wallis wrote:
>> On 12/4/2017 9:15 PM, Chunfeng Yun wrote:
>>> On Mon, 2017-12-04 at 09:27 -0500, Adam Wallis wrote:
>>>> The xHCI driver currently has the IMOD set to 160, which
[..]
> 
> If Rob Acks this version I'll apply it and remove that blank line.
> 
> -Mathias
> 

Let me know if you have any other issues with this patch, otherwise with your
ACK, we are done.

Thanks

Adam

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel


-- 
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.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable

[Rob Herring]

On Mon, Dec 04, 2017 at 09:27:51AM -0500, Adam Wallis wrote:
> 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-ns", 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.
> 
> Tested-by: Chunfeng Yun <chunfeng.yun-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Adam Wallis <awallis-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---
> changes from v3:
>   * Changed imod-interval to imod-interval-ns [Rob Herring/Chunfeng]
>   * Changed "modulation" to "moderation" throughout patch [Mathias]
> 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                                | 5 +++++
>  drivers/usb/host/xhci.c                                     | 7 ++-----
>  drivers/usb/host/xhci.h                                     | 2 ++
>  7 files changed, 24 insertions(+), 5 deletions(-)

Reviewed-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html