From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathias Nyman Subject: Re: [PATCH v4] usb: xhci: allow imod-interval to be configurable Date: Tue, 5 Dec 2017 10:48:49 +0200 Message-ID: References: <1512397671-28733-1-git-send-email-awallis@codeaurora.org> <1512440100.17567.172.camel@mhfsdcap03> <99ea0ee4-1fff-73df-d9b5-0f756dfc5635@codeaurora.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <99ea0ee4-1fff-73df-d9b5-0f756dfc5635-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> Content-Language: en-US Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Adam Wallis , Chunfeng Yun Cc: Greg Kroah-Hartman , Rob Herring , Mark Rutland , Matthias Brugger , Mathias Nyman , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, timur-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org List-Id: devicetree@vger.kernel.org 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 >>> Signed-off-by: Adam Wallis >>> --- >>> 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