Re: [PATCH 2/4] usb: xhci-mtk: modify the SOF/ITP interval for mt8195

2023-02-13 Thread 云春峰
On Mon, 2023-02-13 at 22:00 +0100, Marek Vasut wrote:
> On 2/13/23 02:46, Chunfeng Yun (云春峰) wrote:
> > On Fri, 2023-02-10 at 11:32 +0100, Marek Vasut wrote:
> > > On 2/10/23 09:33, Chunfeng Yun wrote:
> > > [...]
> > > > @@ -50,6 +50,27 @@
> > > >#define IPPC_U3_CTRL(p)  (IPPC_U3_CTRL_0P + ((p) *
> > > > 0x08))
> > > >#define IPPC_U2_CTRL(p)  (IPPC_U2_CTRL_0P + ((p) *
> > > > 0x08))
> > > >
> > > > +/* xHCI CSR */
> > > > +#define LS_EOF_CFG 0x930
> > > > +#define LSEOF_OFFSET   0x89
> > > > +
> > > > +#define FS_EOF_CFG 0x934
> > > > +#define FSEOF_OFFSET   0x2e
> > > > +
> > > > +#define SS_GEN1_EOF_CFG0x93c
> > > > +#define SSG1EOF_OFFSET 0x78
> > > > +
> > > > +#define HFCNTR_CFG 0x944
> > > > +#define ITP_DELTA_CLK  (0xa << 1)
> > > > +#define ITP_DELTA_CLK_MASK GENMASK(5, 1)
> > > > +#define FRMCNT_LEV1_RANG   (0x12b << 8)
> > > 
> > > Look at FIELD_PREP() macro, that should let you avoid the (0x12b
> > > <<
> > > 8) .
Sorry, misunderstood you

> > 
> > Seems not use FIELD_PREP() macro here.
> > It's not a mask, it's the value set in below mask
> > FRMCNT_LEV1_RANG_MASK.
> 
> So that would be
> 
> FIELD_PREP(FRMCNT_LEV1_RANG_MASK, 0x12b)
> 
> I think ?
Sure, I'll use it instead

Thanks a lot

> 
> > > > +#define FRMCNT_LEV1_RANG_MASK  GENMASK(19, 8)
> > > > +
> > > > +#define SS_GEN2_EOF_CFG0x990
> > > > +#define SSG2EOF_OFFSET 0x3c
> > > > +
> > > > +#define XSEOF_OFFSET_MASK  GENMASK(11, 0)
> > > 
> > > [...]
> > > 
> > > > @@ -308,6 +354,7 @@ static int xhci_mtk_remove(struct udevice
> > > > *dev)
> > > >
> > > >static const struct udevice_id xhci_mtk_ids[] = {
> > > > { .compatible = "mediatek,mtk-xhci" },
> > > > +   { .compatible = "mediatek,mt8195-xhci" },
> > > 
> > > Is the extra compatible string really needed, can't the driver
> > > match
> > > on
> > > the generic one ?
> > 
> > These settings are a workaround only for mt8195 to fix HW issue,
> > can't
> > use generic compatible.
> 
> Ah, I see, OK


Re: [PATCH 2/4] usb: xhci-mtk: modify the SOF/ITP interval for mt8195

2023-02-13 Thread Marek Vasut

On 2/13/23 02:46, Chunfeng Yun (云春峰) wrote:

On Fri, 2023-02-10 at 11:32 +0100, Marek Vasut wrote:

On 2/10/23 09:33, Chunfeng Yun wrote:
[...]

@@ -50,6 +50,27 @@
   #define IPPC_U3_CTRL(p)  (IPPC_U3_CTRL_0P + ((p) * 0x08))
   #define IPPC_U2_CTRL(p)  (IPPC_U2_CTRL_0P + ((p) * 0x08))
   
+/* xHCI CSR */

+#define LS_EOF_CFG 0x930
+#define LSEOF_OFFSET   0x89
+
+#define FS_EOF_CFG 0x934
+#define FSEOF_OFFSET   0x2e
+
+#define SS_GEN1_EOF_CFG0x93c
+#define SSG1EOF_OFFSET 0x78
+
+#define HFCNTR_CFG 0x944
+#define ITP_DELTA_CLK  (0xa << 1)
+#define ITP_DELTA_CLK_MASK GENMASK(5, 1)
+#define FRMCNT_LEV1_RANG   (0x12b << 8)


Look at FIELD_PREP() macro, that should let you avoid the (0x12b <<
8) .

Seems not use FIELD_PREP() macro here.
It's not a mask, it's the value set in below mask
FRMCNT_LEV1_RANG_MASK.


So that would be

FIELD_PREP(FRMCNT_LEV1_RANG_MASK, 0x12b)

I think ?


+#define FRMCNT_LEV1_RANG_MASK  GENMASK(19, 8)
+
+#define SS_GEN2_EOF_CFG0x990
+#define SSG2EOF_OFFSET 0x3c
+
+#define XSEOF_OFFSET_MASK  GENMASK(11, 0)


[...]


@@ -308,6 +354,7 @@ static int xhci_mtk_remove(struct udevice *dev)
   
   static const struct udevice_id xhci_mtk_ids[] = {

{ .compatible = "mediatek,mtk-xhci" },
+   { .compatible = "mediatek,mt8195-xhci" },


Is the extra compatible string really needed, can't the driver match
on
the generic one ?

These settings are a workaround only for mt8195 to fix HW issue, can't
use generic compatible.


Ah, I see, OK


Re: [PATCH 2/4] usb: xhci-mtk: modify the SOF/ITP interval for mt8195

2023-02-12 Thread 云春峰
On Fri, 2023-02-10 at 11:32 +0100, Marek Vasut wrote:
> On 2/10/23 09:33, Chunfeng Yun wrote:
> [...]
> > @@ -50,6 +50,27 @@
> >   #define IPPC_U3_CTRL(p)   (IPPC_U3_CTRL_0P + ((p) * 0x08))
> >   #define IPPC_U2_CTRL(p)   (IPPC_U2_CTRL_0P + ((p) * 0x08))
> >   
> > +/* xHCI CSR */
> > +#define LS_EOF_CFG 0x930
> > +#define LSEOF_OFFSET   0x89
> > +
> > +#define FS_EOF_CFG 0x934
> > +#define FSEOF_OFFSET   0x2e
> > +
> > +#define SS_GEN1_EOF_CFG0x93c
> > +#define SSG1EOF_OFFSET 0x78
> > +
> > +#define HFCNTR_CFG 0x944
> > +#define ITP_DELTA_CLK  (0xa << 1)
> > +#define ITP_DELTA_CLK_MASK GENMASK(5, 1)
> > +#define FRMCNT_LEV1_RANG   (0x12b << 8)
> 
> Look at FIELD_PREP() macro, that should let you avoid the (0x12b <<
> 8) .
Seems not use FIELD_PREP() macro here.
It's not a mask, it's the value set in below mask
FRMCNT_LEV1_RANG_MASK.

> 
> > +#define FRMCNT_LEV1_RANG_MASK  GENMASK(19, 8)
> > +
> > +#define SS_GEN2_EOF_CFG0x990
> > +#define SSG2EOF_OFFSET 0x3c
> > +
> > +#define XSEOF_OFFSET_MASK  GENMASK(11, 0)
> 
> [...]
> 
> > @@ -308,6 +354,7 @@ static int xhci_mtk_remove(struct udevice *dev)
> >   
> >   static const struct udevice_id xhci_mtk_ids[] = {
> > { .compatible = "mediatek,mtk-xhci" },
> > +   { .compatible = "mediatek,mt8195-xhci" },
> 
> Is the extra compatible string really needed, can't the driver match
> on 
> the generic one ?
These settings are a workaround only for mt8195 to fix HW issue, can't
use generic compatible.

Thanks a lot




Re: [PATCH 2/4] usb: xhci-mtk: modify the SOF/ITP interval for mt8195

2023-02-10 Thread Marek Vasut

On 2/10/23 09:33, Chunfeng Yun wrote:
[...]

@@ -50,6 +50,27 @@
  #define IPPC_U3_CTRL(p)   (IPPC_U3_CTRL_0P + ((p) * 0x08))
  #define IPPC_U2_CTRL(p)   (IPPC_U2_CTRL_0P + ((p) * 0x08))
  
+/* xHCI CSR */

+#define LS_EOF_CFG 0x930
+#define LSEOF_OFFSET   0x89
+
+#define FS_EOF_CFG 0x934
+#define FSEOF_OFFSET   0x2e
+
+#define SS_GEN1_EOF_CFG0x93c
+#define SSG1EOF_OFFSET 0x78
+
+#define HFCNTR_CFG 0x944
+#define ITP_DELTA_CLK  (0xa << 1)
+#define ITP_DELTA_CLK_MASK GENMASK(5, 1)
+#define FRMCNT_LEV1_RANG   (0x12b << 8)


Look at FIELD_PREP() macro, that should let you avoid the (0x12b << 8) .


+#define FRMCNT_LEV1_RANG_MASK  GENMASK(19, 8)
+
+#define SS_GEN2_EOF_CFG0x990
+#define SSG2EOF_OFFSET 0x3c
+
+#define XSEOF_OFFSET_MASK  GENMASK(11, 0)


[...]


@@ -308,6 +354,7 @@ static int xhci_mtk_remove(struct udevice *dev)
  
  static const struct udevice_id xhci_mtk_ids[] = {

{ .compatible = "mediatek,mtk-xhci" },
+   { .compatible = "mediatek,mt8195-xhci" },


Is the extra compatible string really needed, can't the driver match on 
the generic one ?