diff mbox series

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

Message ID 20230210083303.7690-2-chunfeng.yun@mediatek.com
State Changes Requested
Delegated to: Marek Vasut
Headers show
Series [1/4] phy: phy-mtk-tphy: add support mt8195 | expand

Commit Message

Chunfeng Yun (云春峰) Feb. 10, 2023, 8:33 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 shall set the
accurate interval according to 48Mhz for those controllers.

Note:
The first controller no need set it, but if set it, shall change
tphy's pll at the same time.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/host/xhci-mtk.c | 47 +++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

Comments

Marek Vasut Feb. 10, 2023, 10:32 a.m. UTC | #1
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_CFG		0x93c
> +#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_CFG		0x990
> +#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 ?
Chunfeng Yun (云春峰) Feb. 13, 2023, 1:46 a.m. UTC | #2
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_CFG		0x93c
> > +#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_CFG		0x990
> > +#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
Marek Vasut Feb. 13, 2023, 9 p.m. UTC | #3
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_CFG		0x93c
>>> +#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_CFG		0x990
>>> +#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
Chunfeng Yun (云春峰) Feb. 14, 2023, 5:42 a.m. UTC | #4
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_CFG		0x93c
> > > > +#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_CFG		0x990
> > > > +#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
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 3838a990ec..46f8039960 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -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_CFG		0x93c
+#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)
+#define FRMCNT_LEV1_RANG_MASK	GENMASK(19, 8)
+
+#define SS_GEN2_EOF_CFG		0x990
+#define SSG2EOF_OFFSET		0x3c
+
+#define XSEOF_OFFSET_MASK	GENMASK(11, 0)
+
 struct mtk_xhci {
 	struct xhci_ctrl ctrl;	/* Needs to come first in this struct! */
 	struct xhci_hccr *hcd;
@@ -65,6 +86,29 @@  struct mtk_xhci {
 	u32 u2p_dis_msk;
 };
 
+/*
+ * workaround for mt8195:
+ * MT8195 has 4 controllers, the controller1~3's default SOF/ITP interval
+ * is calculated from the frame counter clock 24M, but in fact, the clock
+ * is 48M.
+ */
+static void xhci_mtk_set_frame_interval(struct mtk_xhci *mtk)
+{
+	void __iomem *mac = (void __iomem *)mtk->hcd;
+
+	if (!ofnode_device_is_compatible(dev_ofnode(mtk->dev), "mediatek,mt8195-xhci"))
+		return;
+
+	clrsetbits_le32(mac + HFCNTR_CFG,
+			ITP_DELTA_CLK_MASK | FRMCNT_LEV1_RANG_MASK,
+			ITP_DELTA_CLK | FRMCNT_LEV1_RANG);
+
+	clrsetbits_le32(mac + LS_EOF_CFG, XSEOF_OFFSET_MASK, LSEOF_OFFSET);
+	clrsetbits_le32(mac + FS_EOF_CFG, XSEOF_OFFSET_MASK, FSEOF_OFFSET);
+	clrsetbits_le32(mac + SS_GEN1_EOF_CFG, XSEOF_OFFSET_MASK, SSG1EOF_OFFSET);
+	clrsetbits_le32(mac + SS_GEN2_EOF_CFG, XSEOF_OFFSET_MASK, SSG2EOF_OFFSET);
+}
+
 static int xhci_mtk_host_enable(struct mtk_xhci *mtk)
 {
 	int u3_ports_disabed = 0;
@@ -278,6 +322,8 @@  static int xhci_mtk_probe(struct udevice *dev)
 	if (ret)
 		goto ssusb_init_err;
 
+	xhci_mtk_set_frame_interval(mtk);
+
 	mtk->ctrl.quirks = XHCI_MTK_HOST;
 	hcor = (struct xhci_hcor *)((uintptr_t)mtk->hcd +
 			HC_LENGTH(xhci_readl(&mtk->hcd->cr_capbase)));
@@ -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" },
 	{ }
 };