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@xxxxxxxxxxxx> > > 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; > > >