diff mbox series

usb: mtk-xhci: Set the XHCI_NO_64BIT_SUPPORT quirk

Message ID 20191010075004.192818-1-tfiga@chromium.org (mailing list archive)
State New, archived
Headers show
Series usb: mtk-xhci: Set the XHCI_NO_64BIT_SUPPORT quirk | expand

Commit Message

Tomasz Figa Oct. 10, 2019, 7:50 a.m. UTC
MediaTek XHCI host controller does not support 64-bit addressing despite
the AC64 bit of HCCPARAMS1 register being set. The platform-specific
glue sets the DMA mask to 32 bits on its own, but it has no effect,
because xhci_gen_setup() overrides it according to hardware
capabilities.

Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
32-bit DMA mask instead.

Signed-off-by: Tomasz Figa <tfiga@chromium.org>
---
 drivers/usb/host/xhci-mtk.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Chunfeng Yun Oct. 10, 2019, 8:45 a.m. UTC | #1
Hi, Tomasz,

On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> MediaTek XHCI host controller does not support 64-bit addressing despite
> the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> glue sets the DMA mask to 32 bits on its own, but it has no effect,
> because xhci_gen_setup() overrides it according to hardware
> capabilities.
> 
> Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> 32-bit DMA mask instead.
> 
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> ---
>  drivers/usb/host/xhci-mtk.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index b18a6baef204a..4d101d52cc11b 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
>  	xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
>  	if (mtk->lpm_support)
>  		xhci->quirks |= XHCI_LPM_SUPPORT;
> +	/*
> +	 * MTK host controller does not support 64-bit addressing, despite
> +	 * having the AC64 bit of the HCCPARAMS1 register set.
> +	 */
> +	xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
Somes SoCs support 64bits in fact, so can't support this quirk, do you
encounter any issues without this quirk?

>  }

>  
>  /* called during probe() after chip reset completes */
> @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>  		goto disable_clk;
>  	}
>  
> -	/* Initialize dma_mask and coherent_dma_mask to 32-bits */
> -	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> -	if (ret)
> -		goto disable_clk;
> -
>  	hcd = usb_create_hcd(driver, dev, dev_name(dev));
>  	if (!hcd) {
>  		ret = -ENOMEM;
Tomasz Figa Oct. 10, 2019, 9 a.m. UTC | #2
Hi Chunfeng,

On Thu, Oct 10, 2019 at 5:45 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Hi, Tomasz,
>
> On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> > MediaTek XHCI host controller does not support 64-bit addressing despite
> > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > because xhci_gen_setup() overrides it according to hardware
> > capabilities.
> >
> > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > 32-bit DMA mask instead.
> >
> > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > ---
> >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index b18a6baef204a..4d101d52cc11b 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> >       xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >       if (mtk->lpm_support)
> >               xhci->quirks |= XHCI_LPM_SUPPORT;
> > +     /*
> > +      * MTK host controller does not support 64-bit addressing, despite
> > +      * having the AC64 bit of the HCCPARAMS1 register set.
> > +      */
> > +     xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> Somes SoCs support 64bits in fact, so can't support this quirk, do you
> encounter any issues without this quirk?
>

Thanks for taking a look at this patch.

Yes, on MT8183 the DMA mask ended up being set to 64 bits, but
according to the information I received from MediaTek, the controller
on that SoC only supports 32 bits.

If some SoCs support only 32 bits and some support 64 bits, we may
either need to use different DT compatible string for them or add a DT
property and set the quirk based on that. Right now in upstream we
have:

1) "mediatek,mt8173-xhci", used by:
MT8173

2)"mediatek,mtk-xhci", used by:
MT2712
MT7622
MT8183 (not yet upstream, but I suppose it's on the mailing lists)

Would you be able to check which of the SoCs above report 64 bits but
support only 32? (and so would need this quirk)

Best regards,
Tomasz

> >  }
>
> >
> >  /* called during probe() after chip reset completes */
> > @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >               goto disable_clk;
> >       }
> >
> > -     /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> > -     ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > -     if (ret)
> > -             goto disable_clk;
> > -
> >       hcd = usb_create_hcd(driver, dev, dev_name(dev));
> >       if (!hcd) {
> >               ret = -ENOMEM;
>
>
Nicolas Boichat Oct. 10, 2019, 9:07 a.m. UTC | #3
On Thu, Oct 10, 2019 at 3:50 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> MediaTek XHCI host controller does not support 64-bit addressing despite
> the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> glue sets the DMA mask to 32 bits on its own, but it has no effect,
> because xhci_gen_setup() overrides it according to hardware
> capabilities.
>
> Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> 32-bit DMA mask instead.
>
> Signed-off-by: Tomasz Figa <tfiga@chromium.org>

Can we add a Fixes: tag for stable backports?
(after addressing the other comments of course)


> ---
>  drivers/usb/host/xhci-mtk.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> index b18a6baef204a..4d101d52cc11b 100644
> --- a/drivers/usb/host/xhci-mtk.c
> +++ b/drivers/usb/host/xhci-mtk.c
> @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
>         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
>         if (mtk->lpm_support)
>                 xhci->quirks |= XHCI_LPM_SUPPORT;
> +       /*
> +        * MTK host controller does not support 64-bit addressing, despite
> +        * having the AC64 bit of the HCCPARAMS1 register set.
> +        */
> +       xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
>  }
>
>  /* called during probe() after chip reset completes */
> @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
>                 goto disable_clk;
>         }
>
> -       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> -       ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> -       if (ret)
> -               goto disable_clk;
> -
>         hcd = usb_create_hcd(driver, dev, dev_name(dev));
>         if (!hcd) {
>                 ret = -ENOMEM;
> --
> 2.23.0.581.g78d2f28ef7-goog
>
Tomasz Figa Oct. 10, 2019, 9:11 a.m. UTC | #4
On Thu, Oct 10, 2019 at 6:08 PM Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> On Thu, Oct 10, 2019 at 3:50 PM Tomasz Figa <tfiga@chromium.org> wrote:
> >
> > MediaTek XHCI host controller does not support 64-bit addressing despite
> > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > because xhci_gen_setup() overrides it according to hardware
> > capabilities.
> >
> > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > 32-bit DMA mask instead.
> >
> > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
>
> Can we add a Fixes: tag for stable backports?
> (after addressing the other comments of course)
>

The problem with Fixes: is that this patch depends on the quirk being
there, but the offending code was merged earlier. Do you know how to
handle such cases?

>
> > ---
> >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > index b18a6baef204a..4d101d52cc11b 100644
> > --- a/drivers/usb/host/xhci-mtk.c
> > +++ b/drivers/usb/host/xhci-mtk.c
> > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> >         if (mtk->lpm_support)
> >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > +       /*
> > +        * MTK host controller does not support 64-bit addressing, despite
> > +        * having the AC64 bit of the HCCPARAMS1 register set.
> > +        */
> > +       xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> >  }
> >
> >  /* called during probe() after chip reset completes */
> > @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> >                 goto disable_clk;
> >         }
> >
> > -       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> > -       ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > -       if (ret)
> > -               goto disable_clk;
> > -
> >         hcd = usb_create_hcd(driver, dev, dev_name(dev));
> >         if (!hcd) {
> >                 ret = -ENOMEM;
> > --
> > 2.23.0.581.g78d2f28ef7-goog
> >
Nicolas Boichat Oct. 10, 2019, 9:37 a.m. UTC | #5
On Thu, Oct 10, 2019 at 5:11 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> On Thu, Oct 10, 2019 at 6:08 PM Nicolas Boichat <drinkcat@chromium.org> wrote:
> >
> > On Thu, Oct 10, 2019 at 3:50 PM Tomasz Figa <tfiga@chromium.org> wrote:
> > >
> > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > because xhci_gen_setup() overrides it according to hardware
> > > capabilities.
> > >
> > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > 32-bit DMA mask instead.
> > >
> > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> >
> > Can we add a Fixes: tag for stable backports?
> > (after addressing the other comments of course)
> >
>
> The problem with Fixes: is that this patch depends on the quirk being
> there, but the offending code was merged earlier. Do you know how to
> handle such cases?

Oh, interesting. I think this is documented here:
https://github.com/torvalds/linux/blob/master/Documentation/process/stable-kernel-rules.rst

Something like this:
Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle
Cc: <stable@vger.kernel.org> # 3.3.x

(Where 3.3.x is the first release that contains the commit indicated
in the Fixes tag)

Try that, worst case you'll get automated emails from stable
maintainers asking you how to fix the issue.

>
> >
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index b18a6baef204a..4d101d52cc11b 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >         if (mtk->lpm_support)
> > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +       /*
> > > +        * MTK host controller does not support 64-bit addressing, despite
> > > +        * having the AC64 bit of the HCCPARAMS1 register set.
> > > +        */
> > > +       xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > >  }
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >                 goto disable_clk;
> > >         }
> > >
> > > -       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> > > -       ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > > -       if (ret)
> > > -               goto disable_clk;
> > > -
> > >         hcd = usb_create_hcd(driver, dev, dev_name(dev));
> > >         if (!hcd) {
> > >                 ret = -ENOMEM;
> > > --
> > > 2.23.0.581.g78d2f28ef7-goog
> > >
Tomasz Figa Oct. 10, 2019, 10:09 a.m. UTC | #6
On Thu, Oct 10, 2019 at 6:37 PM Nicolas Boichat <drinkcat@chromium.org> wrote:
>
> On Thu, Oct 10, 2019 at 5:11 PM Tomasz Figa <tfiga@chromium.org> wrote:
> >
> > On Thu, Oct 10, 2019 at 6:08 PM Nicolas Boichat <drinkcat@chromium.org> wrote:
> > >
> > > On Thu, Oct 10, 2019 at 3:50 PM Tomasz Figa <tfiga@chromium.org> wrote:
> > > >
> > > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > > because xhci_gen_setup() overrides it according to hardware
> > > > capabilities.
> > > >
> > > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > > 32-bit DMA mask instead.
> > > >
> > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > >
> > > Can we add a Fixes: tag for stable backports?
> > > (after addressing the other comments of course)
> > >
> >
> > The problem with Fixes: is that this patch depends on the quirk being
> > there, but the offending code was merged earlier. Do you know how to
> > handle such cases?
>
> Oh, interesting. I think this is documented here:
> https://github.com/torvalds/linux/blob/master/Documentation/process/stable-kernel-rules.rst
>
> Something like this:
> Cc: <stable@vger.kernel.org> # 3.3.x: a1f84a3: sched: Check for idle
> Cc: <stable@vger.kernel.org> # 3.3.x
>
> (Where 3.3.x is the first release that contains the commit indicated
> in the Fixes tag)
>
> Try that, worst case you'll get automated emails from stable
> maintainers asking you how to fix the issue.
>

Oh, great, thanks a lot!

Best regards,
Tomasz

> >
> > >
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index b18a6baef204a..4d101d52cc11b 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >         xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >         if (mtk->lpm_support)
> > > >                 xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +       /*
> > > > +        * MTK host controller does not support 64-bit addressing, despite
> > > > +        * having the AC64 bit of the HCCPARAMS1 register set.
> > > > +        */
> > > > +       xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > > >  }
> > > >
> > > >  /* called during probe() after chip reset completes */
> > > > @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > > >                 goto disable_clk;
> > > >         }
> > > >
> > > > -       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> > > > -       ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > > > -       if (ret)
> > > > -               goto disable_clk;
> > > > -
> > > >         hcd = usb_create_hcd(driver, dev, dev_name(dev));
> > > >         if (!hcd) {
> > > >                 ret = -ENOMEM;
> > > > --
> > > > 2.23.0.581.g78d2f28ef7-goog
> > > >
Chunfeng Yun Oct. 10, 2019, 10:59 a.m. UTC | #7
On Thu, 2019-10-10 at 18:00 +0900, Tomasz Figa wrote:
> Hi Chunfeng,
> 
> On Thu, Oct 10, 2019 at 5:45 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > Hi, Tomasz,
> >
> > On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > because xhci_gen_setup() overrides it according to hardware
> > > capabilities.
Yes, this is what I want to do, maybe need remove DMA mask setting in
platform-specific.

> > >
> > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > 32-bit DMA mask instead.
> > >
> > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > ---
> > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > index b18a6baef204a..4d101d52cc11b 100644
> > > --- a/drivers/usb/host/xhci-mtk.c
> > > +++ b/drivers/usb/host/xhci-mtk.c
> > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > >       xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > >       if (mtk->lpm_support)
> > >               xhci->quirks |= XHCI_LPM_SUPPORT;
> > > +     /*
> > > +      * MTK host controller does not support 64-bit addressing, despite
> > > +      * having the AC64 bit of the HCCPARAMS1 register set.
> > > +      */
> > > +     xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > Somes SoCs support 64bits in fact, so can't support this quirk, do you
> > encounter any issues without this quirk?
> >
> 
> Thanks for taking a look at this patch.
> 
> Yes, on MT8183 the DMA mask ended up being set to 64 bits, but
> according to the information I received from MediaTek, the controller
> on that SoC only supports 32 bits.
As I know, mt8183 doesn't support memory greater than 4G mode.

> 
> If some SoCs support only 32 bits and some support 64 bits, we may
> either need to use different DT compatible string for them or add a DT
> property and set the quirk based on that. Right now in upstream we
> have:
> 
> 1) "mediatek,mt8173-xhci", used by:
> MT8173
> 
> 2)"mediatek,mtk-xhci", used by:
> MT2712
> MT7622
> MT8183 (not yet upstream, but I suppose it's on the mailing lists)
> 
> Would you be able to check which of the SoCs above report 64 bits but
> support only 32? (and so would need this quirk)
I'm afraid I can't, almost all MTK SoCs supporting xHCI are using this
driver, AC64 should be set rightly according to addressing capability.

 
> 
> Best regards,
> Tomasz
> 
> > >  }
> >
> > >
> > >  /* called during probe() after chip reset completes */
> > > @@ -488,11 +493,6 @@ static int xhci_mtk_probe(struct platform_device *pdev)
> > >               goto disable_clk;
> > >       }
> > >
> > > -     /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> > > -     ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > > -     if (ret)
> > > -             goto disable_clk;
> > > -
> > >       hcd = usb_create_hcd(driver, dev, dev_name(dev));
> > >       if (!hcd) {
> > >               ret = -ENOMEM;
> >
> >
Tomasz Figa Oct. 11, 2019, 4:35 a.m. UTC | #8
On Thu, Oct 10, 2019 at 7:59 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> On Thu, 2019-10-10 at 18:00 +0900, Tomasz Figa wrote:
> > Hi Chunfeng,
> >
> > On Thu, Oct 10, 2019 at 5:45 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > Hi, Tomasz,
> > >
> > > On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> > > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > > because xhci_gen_setup() overrides it according to hardware
> > > > capabilities.
> Yes, this is what I want to do, maybe need remove DMA mask setting in
> platform-specific.
>
> > > >
> > > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > > 32-bit DMA mask instead.
> > > >
> > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > ---
> > > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > index b18a6baef204a..4d101d52cc11b 100644
> > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > >       xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > >       if (mtk->lpm_support)
> > > >               xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > +     /*
> > > > +      * MTK host controller does not support 64-bit addressing, despite
> > > > +      * having the AC64 bit of the HCCPARAMS1 register set.
> > > > +      */
> > > > +     xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > > Somes SoCs support 64bits in fact, so can't support this quirk, do you
> > > encounter any issues without this quirk?
> > >
> >
> > Thanks for taking a look at this patch.
> >
> > Yes, on MT8183 the DMA mask ended up being set to 64 bits, but
> > according to the information I received from MediaTek, the controller
> > on that SoC only supports 32 bits.
> As I know, mt8183 doesn't support memory greater than 4G mode.
>

We have 4GB of DRAM at 0x40000000-0x140000000 on our board with
MT8183. What happens if you attempt to use the memory from
0x100000000-0x140000000 with the XHCI controller on this SoC?

> >
> > If some SoCs support only 32 bits and some support 64 bits, we may
> > either need to use different DT compatible string for them or add a DT
> > property and set the quirk based on that. Right now in upstream we
> > have:
> >
> > 1) "mediatek,mt8173-xhci", used by:
> > MT8173
> >
> > 2)"mediatek,mtk-xhci", used by:
> > MT2712
> > MT7622
> > MT8183 (not yet upstream, but I suppose it's on the mailing lists)
> >
> > Would you be able to check which of the SoCs above report 64 bits but
> > support only 32? (and so would need this quirk)
> I'm afraid I can't, almost all MTK SoCs supporting xHCI are using this
> driver, AC64 should be set rightly according to addressing capability.
>

Does it mean that only MT8183 may be the only SoC with a problem with
this capability bit?

Matthias, do you have access to MT2712 and MT7622 devices? I have
MT8173 and MT8183, so I can check them, but would be good to check
this on the other ones too.

Best regards,
Tomasz
Chunfeng Yun Oct. 11, 2019, 6:53 a.m. UTC | #9
On Fri, 2019-10-11 at 13:35 +0900, Tomasz Figa wrote:
> On Thu, Oct 10, 2019 at 7:59 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Thu, 2019-10-10 at 18:00 +0900, Tomasz Figa wrote:
> > > Hi Chunfeng,
> > >
> > > On Thu, Oct 10, 2019 at 5:45 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > Hi, Tomasz,
> > > >
> > > > On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> > > > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > > > because xhci_gen_setup() overrides it according to hardware
> > > > > capabilities.
> > Yes, this is what I want to do, maybe need remove DMA mask setting in
> > platform-specific.
> >
> > > > >
> > > > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > > > 32-bit DMA mask instead.
> > > > >
> > > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > > ---
> > > > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > index b18a6baef204a..4d101d52cc11b 100644
> > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > >       xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > >       if (mtk->lpm_support)
> > > > >               xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > +     /*
> > > > > +      * MTK host controller does not support 64-bit addressing, despite
> > > > > +      * having the AC64 bit of the HCCPARAMS1 register set.
> > > > > +      */
> > > > > +     xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > > > Somes SoCs support 64bits in fact, so can't support this quirk, do you
> > > > encounter any issues without this quirk?
> > > >
> > >
> > > Thanks for taking a look at this patch.
> > >
> > > Yes, on MT8183 the DMA mask ended up being set to 64 bits, but
> > > according to the information I received from MediaTek, the controller
> > > on that SoC only supports 32 bits.
> > As I know, mt8183 doesn't support memory greater than 4G mode.
> >
> 
> We have 4GB of DRAM at 0x40000000-0x140000000 on our board with
> MT8183. What happens if you attempt to use the memory from
> 0x100000000-0x140000000 with the XHCI controller on this SoC?
Ok, I'll contact USB SA of MT8183, and discuss this problem.
I guess MT8183 don't plan to support 4G mode when kick off.

> 
> > >
> > > If some SoCs support only 32 bits and some support 64 bits, we may
> > > either need to use different DT compatible string for them or add a DT
> > > property and set the quirk based on that. Right now in upstream we
> > > have:
> > >
> > > 1) "mediatek,mt8173-xhci", used by:
> > > MT8173
> > >
> > > 2)"mediatek,mtk-xhci", used by:
> > > MT2712
> > > MT7622
> > > MT8183 (not yet upstream, but I suppose it's on the mailing lists)
> > >
> > > Would you be able to check which of the SoCs above report 64 bits but
> > > support only 32? (and so would need this quirk)
> > I'm afraid I can't, almost all MTK SoCs supporting xHCI are using this
> > driver, AC64 should be set rightly according to addressing capability.
> >
> 
> Does it mean that only MT8183 may be the only SoC with a problem with
> this capability bit?
Maybe, I'll check it with USB DE.

Thanks

> 
> Matthias, do you have access to MT2712 and MT7622 devices? I have
> MT8173 and MT8183, so I can check them, but would be good to check
> this on the other ones too.
> 
> Best regards,
> Tomasz
Chunfeng Yun Oct. 30, 2019, 8:14 a.m. UTC | #10
Hi Tomasz,

On Fri, 2019-10-11 at 13:35 +0900, Tomasz Figa wrote:
> On Thu, Oct 10, 2019 at 7:59 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > On Thu, 2019-10-10 at 18:00 +0900, Tomasz Figa wrote:
> > > Hi Chunfeng,
> > >
> > > On Thu, Oct 10, 2019 at 5:45 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > >
> > > > Hi, Tomasz,
> > > >
> > > > On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> > > > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > > > because xhci_gen_setup() overrides it according to hardware
> > > > > capabilities.
> > Yes, this is what I want to do, maybe need remove DMA mask setting in
> > platform-specific.
> >
> > > > >
> > > > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > > > 32-bit DMA mask instead.
> > > > >
> > > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > > ---
> > > > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > index b18a6baef204a..4d101d52cc11b 100644
> > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > >       xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > >       if (mtk->lpm_support)
> > > > >               xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > +     /*
> > > > > +      * MTK host controller does not support 64-bit addressing, despite
> > > > > +      * having the AC64 bit of the HCCPARAMS1 register set.
> > > > > +      */
> > > > > +     xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > > > Somes SoCs support 64bits in fact, so can't support this quirk, do you
> > > > encounter any issues without this quirk?
> > > >
> > >
> > > Thanks for taking a look at this patch.
> > >
> > > Yes, on MT8183 the DMA mask ended up being set to 64 bits, but
> > > according to the information I received from MediaTek, the controller
> > > on that SoC only supports 32 bits.
> > As I know, mt8183 doesn't support memory greater than 4G mode.
> >
> 
> We have 4GB of DRAM at 0x40000000-0x140000000 on our board with
> MT8183. What happens if you attempt to use the memory from
> 0x100000000-0x140000000 with the XHCI controller on this SoC?

Sorry for the late reply.

I've checked it with USB DE, USB IP supports 64bit on MT8183, so no need
set XHCI_NO_64BIT_SUPPORT. 
Would you please help to send a new patch to remove local dma mask
setting, no need set it.

Thanks a lot


> 
> > >
> > > If some SoCs support only 32 bits and some support 64 bits, we may
> > > either need to use different DT compatible string for them or add a DT
> > > property and set the quirk based on that. Right now in upstream we
> > > have:
> > >
> > > 1) "mediatek,mt8173-xhci", used by:
> > > MT8173
> > >
> > > 2)"mediatek,mtk-xhci", used by:
> > > MT2712
> > > MT7622
> > > MT8183 (not yet upstream, but I suppose it's on the mailing lists)
> > >
> > > Would you be able to check which of the SoCs above report 64 bits but
> > > support only 32? (and so would need this quirk)
> > I'm afraid I can't, almost all MTK SoCs supporting xHCI are using this
> > driver, AC64 should be set rightly according to addressing capability.
> >
> 
> Does it mean that only MT8183 may be the only SoC with a problem with
> this capability bit?
> 
> Matthias, do you have access to MT2712 and MT7622 devices? I have
> MT8173 and MT8183, so I can check them, but would be good to check
> this on the other ones too.


> 
> Best regards,
> Tomasz
Tomasz Figa Nov. 7, 2019, 4:26 a.m. UTC | #11
On Wed, Oct 30, 2019 at 5:15 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> Hi Tomasz,
>
> On Fri, 2019-10-11 at 13:35 +0900, Tomasz Figa wrote:
> > On Thu, Oct 10, 2019 at 7:59 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > >
> > > On Thu, 2019-10-10 at 18:00 +0900, Tomasz Figa wrote:
> > > > Hi Chunfeng,
> > > >
> > > > On Thu, Oct 10, 2019 at 5:45 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> > > > >
> > > > > Hi, Tomasz,
> > > > >
> > > > > On Thu, 2019-10-10 at 16:50 +0900, Tomasz Figa wrote:
> > > > > > MediaTek XHCI host controller does not support 64-bit addressing despite
> > > > > > the AC64 bit of HCCPARAMS1 register being set. The platform-specific
> > > > > > glue sets the DMA mask to 32 bits on its own, but it has no effect,
> > > > > > because xhci_gen_setup() overrides it according to hardware
> > > > > > capabilities.
> > > Yes, this is what I want to do, maybe need remove DMA mask setting in
> > > platform-specific.
> > >
> > > > > >
> > > > > > Use the XHCI_NO_64BIT_SUPPORT quirk to tell the XHCI core to force
> > > > > > 32-bit DMA mask instead.
> > > > > >
> > > > > > Signed-off-by: Tomasz Figa <tfiga@chromium.org>
> > > > > > ---
> > > > > >  drivers/usb/host/xhci-mtk.c | 10 +++++-----
> > > > > >  1 file changed, 5 insertions(+), 5 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
> > > > > > index b18a6baef204a..4d101d52cc11b 100644
> > > > > > --- a/drivers/usb/host/xhci-mtk.c
> > > > > > +++ b/drivers/usb/host/xhci-mtk.c
> > > > > > @@ -395,6 +395,11 @@ static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
> > > > > >       xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
> > > > > >       if (mtk->lpm_support)
> > > > > >               xhci->quirks |= XHCI_LPM_SUPPORT;
> > > > > > +     /*
> > > > > > +      * MTK host controller does not support 64-bit addressing, despite
> > > > > > +      * having the AC64 bit of the HCCPARAMS1 register set.
> > > > > > +      */
> > > > > > +     xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
> > > > > Somes SoCs support 64bits in fact, so can't support this quirk, do you
> > > > > encounter any issues without this quirk?
> > > > >
> > > >
> > > > Thanks for taking a look at this patch.
> > > >
> > > > Yes, on MT8183 the DMA mask ended up being set to 64 bits, but
> > > > according to the information I received from MediaTek, the controller
> > > > on that SoC only supports 32 bits.
> > > As I know, mt8183 doesn't support memory greater than 4G mode.
> > >
> >
> > We have 4GB of DRAM at 0x40000000-0x140000000 on our board with
> > MT8183. What happens if you attempt to use the memory from
> > 0x100000000-0x140000000 with the XHCI controller on this SoC?
>
> Sorry for the late reply.
>
> I've checked it with USB DE, USB IP supports 64bit on MT8183, so no need
> set XHCI_NO_64BIT_SUPPORT.
> Would you please help to send a new patch to remove local dma mask
> setting, no need set it.
>
> Thanks a lot
>

Thanks Chunfeng. Sounds good then. I'll send a follow-up clean-up
patch once I catch up after a conference and vacation. :)

Best regards,
Tomasz

>
> >
> > > >
> > > > If some SoCs support only 32 bits and some support 64 bits, we may
> > > > either need to use different DT compatible string for them or add a DT
> > > > property and set the quirk based on that. Right now in upstream we
> > > > have:
> > > >
> > > > 1) "mediatek,mt8173-xhci", used by:
> > > > MT8173
> > > >
> > > > 2)"mediatek,mtk-xhci", used by:
> > > > MT2712
> > > > MT7622
> > > > MT8183 (not yet upstream, but I suppose it's on the mailing lists)
> > > >
> > > > Would you be able to check which of the SoCs above report 64 bits but
> > > > support only 32? (and so would need this quirk)
> > > I'm afraid I can't, almost all MTK SoCs supporting xHCI are using this
> > > driver, AC64 should be set rightly according to addressing capability.
> > >
> >
> > Does it mean that only MT8183 may be the only SoC with a problem with
> > this capability bit?
> >
> > Matthias, do you have access to MT2712 and MT7622 devices? I have
> > MT8173 and MT8183, so I can check them, but would be good to check
> > this on the other ones too.
>
>
> >
> > Best regards,
> > Tomasz
>
>
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index b18a6baef204a..4d101d52cc11b 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -395,6 +395,11 @@  static void xhci_mtk_quirks(struct device *dev, struct xhci_hcd *xhci)
 	xhci->quirks |= XHCI_SPURIOUS_SUCCESS;
 	if (mtk->lpm_support)
 		xhci->quirks |= XHCI_LPM_SUPPORT;
+	/*
+	 * MTK host controller does not support 64-bit addressing, despite
+	 * having the AC64 bit of the HCCPARAMS1 register set.
+	 */
+	xhci->quirks |= XHCI_NO_64BIT_SUPPORT;
 }
 
 /* called during probe() after chip reset completes */
@@ -488,11 +493,6 @@  static int xhci_mtk_probe(struct platform_device *pdev)
 		goto disable_clk;
 	}
 
-	/* Initialize dma_mask and coherent_dma_mask to 32-bits */
-	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
-	if (ret)
-		goto disable_clk;
-
 	hcd = usb_create_hcd(driver, dev, dev_name(dev));
 	if (!hcd) {
 		ret = -ENOMEM;