Message ID | 1512397671-28733-1-git-send-email-awallis@codeaurora.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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.
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
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.
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@mediatek.com> > Signed-off-by: Adam Wallis <awallis@codeaurora.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
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@mediatek.com> >> Signed-off-by: Adam Wallis <awallis@codeaurora.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 > >
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@mediatek.com> >>> Signed-off-by: Adam Wallis <awallis@codeaurora.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
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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
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@mediatek.com> > Signed-off-by: Adam Wallis <awallis@codeaurora.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@kernel.org>
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;