diff mbox series

[03/11] dt-bindings: usb: mtk-xhci: add compatible for mt8195

Message ID 1627635002-24521-3-git-send-email-chunfeng.yun@mediatek.com
State Accepted
Commit e2cd76907fcc8c7438a50b4c44a8073f2228208c
Headers show
Series [01/11] dt-bindings: usb: mtk-xhci: add support property 'tpl-support' | expand

Commit Message

Chunfeng Yun (云春峰) July 30, 2021, 8:49 a.m. UTC
There are 4 USB controllers on MT8195, the controllers (IP1~IP3,
exclude IP0) have a wrong default SOF/ITP interval which is
calculated from the frame counter clock 24Mhz by default, but
in fact, the frame counter clock is 48Mhz, so we should set
the accurate interval according to 48Mhz. Here add a new compatible
for MT8195, it's also supported in driver. But the first controller
(IP0) has no such issue, we prefer to use generic compatible,
e.g. mt8192's compatible.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 1 +
 1 file changed, 1 insertion(+)

Comments

Rob Herring (Arm) Aug. 6, 2021, 8:43 p.m. UTC | #1
On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:
> There are 4 USB controllers on MT8195, the controllers (IP1~IP3,

> exclude IP0) have a wrong default SOF/ITP interval which is

> calculated from the frame counter clock 24Mhz by default, but

> in fact, the frame counter clock is 48Mhz, so we should set

> the accurate interval according to 48Mhz. Here add a new compatible

> for MT8195, it's also supported in driver. But the first controller

> (IP0) has no such issue, we prefer to use generic compatible,

> e.g. mt8192's compatible.


That only works until you find some 8195 bug common to all instances. 
Can't you read the clock frequency?

> 

> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

> ---

>  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml

> index 61a0e550b5d6..753e043e5327 100644

> --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml

> +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml

> @@ -31,6 +31,7 @@ properties:

>            - mediatek,mt8173-xhci

>            - mediatek,mt8183-xhci

>            - mediatek,mt8192-xhci

> +          - mediatek,mt8195-xhci

>        - const: mediatek,mtk-xhci

>  

>    reg:

> -- 

> 2.18.0

> 

>
Chunfeng Yun (云春峰) Aug. 11, 2021, 8:02 a.m. UTC | #2
On Fri, 2021-08-06 at 14:43 -0600, Rob Herring wrote:
> On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:

> > There are 4 USB controllers on MT8195, the controllers (IP1~IP3,

> > exclude IP0) have a wrong default SOF/ITP interval which is

> > calculated from the frame counter clock 24Mhz by default, but

> > in fact, the frame counter clock is 48Mhz, so we should set

> > the accurate interval according to 48Mhz. Here add a new compatible

> > for MT8195, it's also supported in driver. But the first controller

> > (IP0) has no such issue, we prefer to use generic compatible,

> > e.g. mt8192's compatible.

> 

> That only works until you find some 8195 bug common to all

> instances. 

It's also OK for IP0 to use mt8195's compatible, these setting value is
the same as IP0's default value, use mt8192's may avoid these dummy
setting.

> Can't you read the clock frequency?

No, it may be divided internally, not the same as the clock source.

> 

> > 

> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>

> > ---

> >  Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml | 1 +

> >  1 file changed, 1 insertion(+)

> > 

> > diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-

> > xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-

> > xhci.yaml

> > index 61a0e550b5d6..753e043e5327 100644

> > --- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml

> > +++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml

> > @@ -31,6 +31,7 @@ properties:

> >            - mediatek,mt8173-xhci

> >            - mediatek,mt8183-xhci

> >            - mediatek,mt8192-xhci

> > +          - mediatek,mt8195-xhci

> >        - const: mediatek,mtk-xhci

> >  

> >    reg:

> > -- 

> > 2.18.0

> > 

> >
Rob Herring (Arm) Aug. 18, 2021, 2:20 p.m. UTC | #3
On Wed, Aug 11, 2021 at 3:02 AM Chunfeng Yun (云春峰)
<Chunfeng.Yun@mediatek.com> wrote:
>

> On Fri, 2021-08-06 at 14:43 -0600, Rob Herring wrote:

> > On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:

> > > There are 4 USB controllers on MT8195, the controllers (IP1~IP3,

> > > exclude IP0) have a wrong default SOF/ITP interval which is

> > > calculated from the frame counter clock 24Mhz by default, but

> > > in fact, the frame counter clock is 48Mhz, so we should set

> > > the accurate interval according to 48Mhz. Here add a new compatible

> > > for MT8195, it's also supported in driver. But the first controller

> > > (IP0) has no such issue, we prefer to use generic compatible,

> > > e.g. mt8192's compatible.

> >

> > That only works until you find some 8195 bug common to all

> > instances.

> It's also OK for IP0 to use mt8195's compatible, these setting value is

> the same as IP0's default value, use mt8192's may avoid these dummy

> setting.


I still don't understand. By use mt8192's compatible, that means you
have for IP0:

compatible = "mediatek,mt8192-xhci", "mediatek,mtk-xhci";

And for the rest:
compatible = "mediatek,mt8195-xhci", "mediatek,mtk-xhci";

If there's a 8195 quirk you need to work around, then you can't on
IP0. You need to be able to address quirks in the future without
changing the DTB. That is why we require SoC specific compatibles even
when IP blocks are 'the same'.

Rob
Chunfeng Yun (云春峰) Aug. 19, 2021, 11:36 a.m. UTC | #4
On Wed, 2021-08-18 at 09:20 -0500, Rob Herring wrote:
> On Wed, Aug 11, 2021 at 3:02 AM Chunfeng Yun (云春峰)

> <Chunfeng.Yun@mediatek.com> wrote:

> > 

> > On Fri, 2021-08-06 at 14:43 -0600, Rob Herring wrote:

> > > On Fri, Jul 30, 2021 at 04:49:54PM +0800, Chunfeng Yun wrote:

> > > > There are 4 USB controllers on MT8195, the controllers

> > > > (IP1~IP3,

> > > > exclude IP0) have a wrong default SOF/ITP interval which is

> > > > calculated from the frame counter clock 24Mhz by default, but

> > > > in fact, the frame counter clock is 48Mhz, so we should set

> > > > the accurate interval according to 48Mhz. Here add a new

> > > > compatible

> > > > for MT8195, it's also supported in driver. But the first

> > > > controller

> > > > (IP0) has no such issue, we prefer to use generic compatible,

> > > > e.g. mt8192's compatible.

> > > 

> > > That only works until you find some 8195 bug common to all

> > > instances.

> > 

> > It's also OK for IP0 to use mt8195's compatible, these setting

> > value is

> > the same as IP0's default value, use mt8192's may avoid these dummy

> > setting.

> 

> I still don't understand. By use mt8192's compatible, that means you

> have for IP0:

> 

> compatible = "mediatek,mt8192-xhci", "mediatek,mtk-xhci";

> 

> And for the rest:

> compatible = "mediatek,mt8195-xhci", "mediatek,mtk-xhci";

No, use "mediatek,mt8195-xhci" is also ok for IP0.

Seems need modify commit log and remove last sentence to avoid
misunderstanding.

Thanks

> 

> If there's a 8195 quirk you need to work around, then you can't on

> IP0. You need to be able to address quirks in the future without

> changing the DTB. That is why we require SoC specific compatibles

> even

> when IP blocks are 'the same'.

> 

> Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
index 61a0e550b5d6..753e043e5327 100644
--- a/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/mediatek,mtk-xhci.yaml
@@ -31,6 +31,7 @@  properties:
           - mediatek,mt8173-xhci
           - mediatek,mt8183-xhci
           - mediatek,mt8192-xhci
+          - mediatek,mt8195-xhci
       - const: mediatek,mtk-xhci
 
   reg: