On 22-08-12 18:57:19, Xu Yang wrote:
> In current design, ci_usb_role_switch_set() will call pm_runtime_get_sync()
> firstly, then handle role switch events. But pm_runtime_get_sync() may fail
> to resume controller sometimes. This may happen when doing system suspend
> and enabled typec wakeup source as below:
> 1. starting system suspend, controller is suspended by runtime pm before.
> 2. somehow controller get resumed by runtime pm.
> 3. ci_suspend() is called. runtime_status = RPM_ACTIVE now.
> 4. ci_usb_role_switch_set() is called due to role switch events.
> 5. pm_runtime_get_sync() return 1.
>
> This is because pm.runtime_status is still RPM_ACTIVE after ci_suspend().
> Then the driver execute wakeup operations in ci_irq(). So there is a need
> to call ci_irq() again like extcon do.
You mean role switch happens after ci_lpm is 1, you need to execute
ci_irq twice? I have not run chipidea platform these years, could @Jun
Li help review this patch first?
>
> Fixes: 876d4e1e8298 ("usb: chipidea: core: add wakeup support for extcon")
> Cc: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> ---
> drivers/usb/chipidea/core.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> index 6330fa911792..886b68e45826 100644
> --- a/drivers/usb/chipidea/core.c
> +++ b/drivers/usb/chipidea/core.c
> @@ -1305,12 +1305,14 @@ static void ci_extcon_wakeup_int(struct ci_hdrc *ci)
> cable_id = &ci->platdata->id_extcon;
> cable_vbus = &ci->platdata->vbus_extcon;
>
> - if (!IS_ERR(cable_id->edev) && ci->is_otg &&
> - (otgsc & OTGSC_IDIE) && (otgsc & OTGSC_IDIS))
> + if ((!IS_ERR(cable_id->edev) || !IS_ERR(ci->role_switch))
> + && ci->is_otg && (otgsc & OTGSC_IDIE)
> + && (otgsc & OTGSC_IDIS))
> ci_irq(ci);
>
> - if (!IS_ERR(cable_vbus->edev) && ci->is_otg &&
> - (otgsc & OTGSC_BSVIE) && (otgsc & OTGSC_BSVIS))
> + if ((!IS_ERR(cable_vbus->edev) || !IS_ERR(ci->role_switch))
> + && ci->is_otg && (otgsc & OTGSC_BSVIE)
> + && (otgsc & OTGSC_BSVIS))
> ci_irq(ci);
> }
With your change, ci_irq needs to execute every time at role-switch case
every time, please confirm it is expected
--
Thanks,
Peter Chen
