Re: [PATCH 2/2] usb: mtk-xhci: add support remote wakeup of mt8196

[AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>]

Il 07/01/25 06:07, Chen-Yu Tsai ha scritto:
> On Mon, Jan 6, 2025 at 10:58 PM Chunfeng Yun (云春峰)
> <Chunfeng.Yun@mediatek.com> wrote:
>>
>> On Tue, 2024-11-26 at 09:50 +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 26/11/24 03:55, Chunfeng Yun ha scritto:
>>>> There are 2 USB controllers on mt8196, each controller's wakeup
>>>> control is
>>>> different, add some specific versions for them.
>>>>
>>>
>>> Is there any MTU3 controller in MT8196, like all other MediaTek SoCs?
>> Yes.
>>>
>>> If so, then please just add the wakeup control to the MTU3 driver,
>>> otherwise
>>> we are going to duplicate this for yet another SoC, like we've done
>>> for MT8192,
>>> MT8195, MT8188 and MT8186 already...
>> Even I add it in MTU3 driver, I still need add it in xhci-mtk driver,
>> some projects only use host mode;
>>
>> I can also add it in MTU3 driver and send out new patches.
>>
>> But it's not a good idea to duplicate it into MTU3 driver directly for
>> some SoC which has limitation on dual-role switch when using upstream
>> driver.
> 
> I think the idea behind Angelo's point is that MTU3 should be used for
> all projects, regardless whether they are host mode only or not.
> The hardware is there. Don't hide it.
> 

Yes, I confirm that's exactly my point.

Thanks for clarifying, Chen-Yu.

Cheers!

> ChenYu
> 
>> Thanks
>>
>>>
>>> Cheers,
>>> Angelo
>>>
>>>> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
>>>> ---
>>>>    drivers/usb/host/xhci-mtk.c | 18 ++++++++++++++++++
>>>>    1 file changed, 18 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-
>>>> mtk.c
>>>> index 3252e3d2d79c..31223912b0b4 100644
>>>> --- a/drivers/usb/host/xhci-mtk.c
>>>> +++ b/drivers/usb/host/xhci-mtk.c
>>>> @@ -113,6 +113,12 @@
>>>>    #define WC1_IS_P_95         BIT(12)
>>>>    #define WC1_IS_EN_P0_95             BIT(6)
>>>>
>>>> +/* mt8196 */
>>>> +#define PERI_WK_CTRL0_8196   0x08
>>>> +#define UWK_V1_7_CTRL2_MASK  0x5
>>>> +
>>>> +#define WCP1_IS_EN           BIT(7) /* port1 en bit */
>>>> +
>>>>    /* mt2712 etc */
>>>>    #define PERI_SSUSB_SPM_CTRL 0x0
>>>>    #define SSC_IP_SLEEP_EN     BIT(4)
>>>> @@ -129,6 +135,8 @@ enum ssusb_uwk_vers {
>>>>        SSUSB_UWK_V1_4,         /* mt8195 IP1 */
>>>>        SSUSB_UWK_V1_5,         /* mt8195 IP2 */
>>>>        SSUSB_UWK_V1_6,         /* mt8195 IP3 */
>>>> +     SSUSB_UWK_V1_7,         /* mt8196 IP0 */
>>>> +     SSUSB_UWK_V1_8,         /* mt8196 IP1 */
>>>>    };
>>>>
>>>>    /*
>>>> @@ -381,6 +389,16 @@ static void usb_wakeup_ip_sleep_set(struct
>>>> xhci_hcd_mtk *mtk, 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 = mtk->uwk_reg_base + PERI_WK_CTRL0_8196;
>>>> +             msk = UWK_V1_7_CTRL2_MASK;
>>>> +             val = enable ? msk : 0;
>>>> +             break;
>>>> +     case SSUSB_UWK_V1_8:
>>>> +             reg = mtk->uwk_reg_base + PERI_WK_CTRL0_8196;
>>>> +             msk = WCP1_IS_EN;
>>>> +             val = enable ? msk : 0;
>>>> +             break;
>>>>        case SSUSB_UWK_V2:
>>>>                reg = mtk->uwk_reg_base + PERI_SSUSB_SPM_CTRL;
>>>>                msk = SSC_IP_SLEEP_EN | SSC_SPM_INT_EN;
>>>
>>>
>>>