| Message ID | 20250121145008.22936-4-chunfeng.yun@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/4] dt-bindings: usb: mtk-xhci: add support mt8196 | expand |
On Wed, 2025-01-22 at 10:30 +0100, AngeloGioacchino Del Regno wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > Il 21/01/25 15:50, Chunfeng Yun ha scritto: > > There are three USB controllers on mt8196, each controller's wakeup > > control is different, add some specific versions for them. > > Here add only for dual-role controllers. > > > > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> > > From the datasheets, I can read the following: > > IP0: host -> 0x1670_0000 device(mtu3) -> 0x1670_1000 > IP1: host -> 0x1671_0000 device(mtu3) -> 0x1671_1000 > IP2: host -> 0x1672_0000 device(mtu3) -> 0x1672_1000 > I'll check it. > ...this means that you're missing the IP2 here, which you did not > miss in the > commit adding the wakeup control in mtk-xhci instead. > > So, since I see that all of the USB IPs are behind MTU3, and that > there is no > USB IP that does *not* support gadget mode (so, there's no USB IP > that does NOT > support MTU3), you shall add all three here, and you shall drop the > commit that > adds the wakeup control in mtk-xhci entirely. No, I'll still add them in mtk-xhci driver as before for the cases that only use xhci only, no need use mtu3 driver. As I said before, event the controller supports dual-role mode, which don't mean that it can use this upstream mtu3 driver, some SoC have limitation and can't support the dual-role mode switch used in upstream driver. but all SoC can use upstream xhci-mtk driver. that why I add some SoC's wakeup control in xhci-mtk, but not in mtu3 driver. > > This is because there will be no DT node declaring only XHCI. > > Since after the proposed change all controllers will be MTU3 -> XHCI, > there's > no need to add the same in the mtk-xhci driver. I think it's better to leave the selection to the customer, for example, on chromebook, we only use xhci driver and do not enable mtu3. Thanks > > Cheers, > Angelo > > > --- > > v2: add wakeup for dual-role controllers > > --- > > drivers/usb/mtu3/mtu3_host.c | 17 +++++++++++++++++ > > 1 file changed, 17 insertions(+) > > > > diff --git a/drivers/usb/mtu3/mtu3_host.c > > b/drivers/usb/mtu3/mtu3_host.c > > index 7c657ea2dabd..d65b0f318436 100644 > > --- a/drivers/usb/mtu3/mtu3_host.c > > +++ b/drivers/usb/mtu3/mtu3_host.c > > @@ -46,6 +46,11 @@ > > #define WC1_IS_P_95 BIT(12) > > #define WC1_IS_EN_P0_95 BIT(6) > > > > +/* mt8196 */ > > +#define PERI_WK_CTRL0_8196 0x08 > > +#define WC0_IS_EN_P0_96 BIT(0) > > +#define WC0_IS_EN_P1_96 BIT(7) > > + > > /* mt2712 etc */ > > #define PERI_SSUSB_SPM_CTRL 0x0 > > #define SSC_IP_SLEEP_EN BIT(4) > > @@ -59,6 +64,8 @@ enum ssusb_uwk_vers { > > SSUSB_UWK_V1_3, /* mt8195 IP0 */ > > SSUSB_UWK_V1_5 = 105, /* mt8195 IP2 */ > > SSUSB_UWK_V1_6, /* mt8195 IP3 */ > > + SSUSB_UWK_V1_7, /* mt8196 IP0 */ > > + SSUSB_UWK_V1_8, /* mt8196 IP1 */ > > }; > > > > /* > > @@ -100,6 +107,16 @@ static void ssusb_wakeup_ip_sleep_set(struct > > ssusb_mtk *ssusb, bool enable) > > msk = WC0_IS_EN_P3_95 | WC0_IS_C_95(0x7) | > > WC0_IS_P_95; > > val = enable ? (WC0_IS_EN_P3_95 | WC0_IS_C_95(0x1)) : > > 0; > > break; > > + case SSUSB_UWK_V1_7: > > + reg = ssusb->uwk_reg_base + PERI_WK_CTRL0_8196; > > + msk = WC0_IS_EN_P0_96; > > + val = enable ? msk : 0; > > + break; > > + case SSUSB_UWK_V1_8: > > + reg = ssusb->uwk_reg_base + PERI_WK_CTRL0_8196; > > + msk = WC0_IS_EN_P1_96; > > + val = enable ? msk : 0; > > + break; > > case SSUSB_UWK_V2: > > reg = ssusb->uwk_reg_base + PERI_SSUSB_SPM_CTRL; > > msk = SSC_IP_SLEEP_EN | SSC_SPM_INT_EN; > > >
Il 09/02/25 04:31, Chunfeng Yun (云春峰) ha scritto: > On Wed, 2025-01-22 at 10:30 +0100, AngeloGioacchino Del Regno wrote: >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> >> >> Il 21/01/25 15:50, Chunfeng Yun ha scritto: >>> There are three USB controllers on mt8196, each controller's wakeup >>> control is different, add some specific versions for them. >>> Here add only for dual-role controllers. >>> >>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> >> >> From the datasheets, I can read the following: >> >> IP0: host -> 0x1670_0000 device(mtu3) -> 0x1670_1000 >> IP1: host -> 0x1671_0000 device(mtu3) -> 0x1671_1000 >> IP2: host -> 0x1672_0000 device(mtu3) -> 0x1672_1000 >> > I'll check it. > >> ...this means that you're missing the IP2 here, which you did not >> miss in the >> commit adding the wakeup control in mtk-xhci instead. >> >> So, since I see that all of the USB IPs are behind MTU3, and that >> there is no >> USB IP that does *not* support gadget mode (so, there's no USB IP >> that does NOT >> support MTU3), you shall add all three here, and you shall drop the >> commit that >> adds the wakeup control in mtk-xhci entirely. > No, I'll still add them in mtk-xhci driver as before for the cases that > only use xhci only, no need use mtu3 driver. > > As I said before, event the controller supports dual-role mode, which > don't mean that it can use this upstream mtu3 driver, some SoC have > limitation and can't support the dual-role mode switch used in upstream > driver. but all SoC can use upstream xhci-mtk driver. that why I add > some SoC's wakeup control in xhci-mtk, but not in mtu3 driver. > >> >> This is because there will be no DT node declaring only XHCI. >> >> Since after the proposed change all controllers will be MTU3 -> XHCI, >> there's >> no need to add the same in the mtk-xhci driver. > I think it's better to leave the selection to the customer, for > example, on chromebook, we only use xhci driver and do not enable mtu3. > Chromebooks can use MTU3 and lock it in HOST mode only. The MTU3 hardware is there - don't hide it. Regards, Angelo > Thanks > >> >> Cheers, >> Angelo >> >>> --- >>> v2: add wakeup for dual-role controllers >>> --- >>> drivers/usb/mtu3/mtu3_host.c | 17 +++++++++++++++++ >>> 1 file changed, 17 insertions(+) >>> >>> diff --git a/drivers/usb/mtu3/mtu3_host.c >>> b/drivers/usb/mtu3/mtu3_host.c >>> index 7c657ea2dabd..d65b0f318436 100644 >>> --- a/drivers/usb/mtu3/mtu3_host.c >>> +++ b/drivers/usb/mtu3/mtu3_host.c >>> @@ -46,6 +46,11 @@ >>> #define WC1_IS_P_95 BIT(12) >>> #define WC1_IS_EN_P0_95 BIT(6) >>> >>> +/* mt8196 */ >>> +#define PERI_WK_CTRL0_8196 0x08 >>> +#define WC0_IS_EN_P0_96 BIT(0) >>> +#define WC0_IS_EN_P1_96 BIT(7) >>> + >>> /* mt2712 etc */ >>> #define PERI_SSUSB_SPM_CTRL 0x0 >>> #define SSC_IP_SLEEP_EN BIT(4) >>> @@ -59,6 +64,8 @@ enum ssusb_uwk_vers { >>> SSUSB_UWK_V1_3, /* mt8195 IP0 */ >>> SSUSB_UWK_V1_5 = 105, /* mt8195 IP2 */ >>> SSUSB_UWK_V1_6, /* mt8195 IP3 */ >>> + SSUSB_UWK_V1_7, /* mt8196 IP0 */ >>> + SSUSB_UWK_V1_8, /* mt8196 IP1 */ >>> }; >>> >>> /* >>> @@ -100,6 +107,16 @@ static void ssusb_wakeup_ip_sleep_set(struct >>> ssusb_mtk *ssusb, bool enable) >>> msk = WC0_IS_EN_P3_95 | WC0_IS_C_95(0x7) | >>> WC0_IS_P_95; >>> val = enable ? (WC0_IS_EN_P3_95 | WC0_IS_C_95(0x1)) : >>> 0; >>> break; >>> + case SSUSB_UWK_V1_7: >>> + reg = ssusb->uwk_reg_base + PERI_WK_CTRL0_8196; >>> + msk = WC0_IS_EN_P0_96; >>> + val = enable ? msk : 0; >>> + break; >>> + case SSUSB_UWK_V1_8: >>> + reg = ssusb->uwk_reg_base + PERI_WK_CTRL0_8196; >>> + msk = WC0_IS_EN_P1_96; >>> + val = enable ? msk : 0; >>> + break; >>> case SSUSB_UWK_V2: >>> reg = ssusb->uwk_reg_base + PERI_SSUSB_SPM_CTRL; >>> msk = SSC_IP_SLEEP_EN | SSC_SPM_INT_EN; >> >> >>
diff --git a/drivers/usb/mtu3/mtu3_host.c b/drivers/usb/mtu3/mtu3_host.c index 7c657ea2dabd..d65b0f318436 100644 --- a/drivers/usb/mtu3/mtu3_host.c +++ b/drivers/usb/mtu3/mtu3_host.c @@ -46,6 +46,11 @@ #define WC1_IS_P_95 BIT(12) #define WC1_IS_EN_P0_95 BIT(6) +/* mt8196 */ +#define PERI_WK_CTRL0_8196 0x08 +#define WC0_IS_EN_P0_96 BIT(0) +#define WC0_IS_EN_P1_96 BIT(7) + /* mt2712 etc */ #define PERI_SSUSB_SPM_CTRL 0x0 #define SSC_IP_SLEEP_EN BIT(4) @@ -59,6 +64,8 @@ enum ssusb_uwk_vers { SSUSB_UWK_V1_3, /* mt8195 IP0 */ SSUSB_UWK_V1_5 = 105, /* mt8195 IP2 */ SSUSB_UWK_V1_6, /* mt8195 IP3 */ + SSUSB_UWK_V1_7, /* mt8196 IP0 */ + SSUSB_UWK_V1_8, /* mt8196 IP1 */ }; /* @@ -100,6 +107,16 @@ static void ssusb_wakeup_ip_sleep_set(struct ssusb_mtk *ssusb, bool enable) msk = WC0_IS_EN_P3_95 | WC0_IS_C_95(0x7) | WC0_IS_P_95; val = enable ? (WC0_IS_EN_P3_95 | WC0_IS_C_95(0x1)) : 0; break; + case SSUSB_UWK_V1_7: + reg = ssusb->uwk_reg_base + PERI_WK_CTRL0_8196; + msk = WC0_IS_EN_P0_96; + val = enable ? msk : 0; + break; + case SSUSB_UWK_V1_8: + reg = ssusb->uwk_reg_base + PERI_WK_CTRL0_8196; + msk = WC0_IS_EN_P1_96; + val = enable ? msk : 0; + break; case SSUSB_UWK_V2: reg = ssusb->uwk_reg_base + PERI_SSUSB_SPM_CTRL; msk = SSC_IP_SLEEP_EN | SSC_SPM_INT_EN;
There are three USB controllers on mt8196, each controller's wakeup control is different, add some specific versions for them. Here add only for dual-role controllers. Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com> --- v2: add wakeup for dual-role controllers --- drivers/usb/mtu3/mtu3_host.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)