| Message ID | 20240119123537.3614838-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | cc509b6a47e7c8998d9e41c273191299d5d9d631 |
| Headers | show |
| Series | usb: chipidea: core: handle power lost in workqueue | expand |
On 24-01-19 20:35:37, Xu Yang wrote: > When power is recycled in usb controller during system power management, > the controller will recognize it and switch role if role has been changed > during power lost. In current design, it will be completed in resume() > function. However, this may bring issues since usb class devices have > their pm operations too and these device's resume() functions are still > not being called at this point. When usb controller recognized host role > should be stopped, these usb class devices will be removed at this point. > But these usb class devices can't be removed in some cases, such as scsi > devices. Since scsi driver may sync data to U-disk, however it will block > there because scsi drvier can only handle pm request when is in suspended > state. Therefore, there may exist a dependency between ci_resume() and usb > class device's resume(). To break this potential dependency, we need to > handle power lost work in a workqueue. > > Fixes: 74494b33211d ("usb: chipidea: core: add controller resume support when controller is powered off") > cc: <stable@vger.kernel.org> > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> Acked-by: Peter Chen <peter.chen@kernel.org> > --- > drivers/usb/chipidea/ci.h | 2 ++ > drivers/usb/chipidea/core.c | 44 ++++++++++++++++++++----------------- > 2 files changed, 26 insertions(+), 20 deletions(-) > > diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h > index d9bb3d3f026e..2a38e1eb6546 100644 > --- a/drivers/usb/chipidea/ci.h > +++ b/drivers/usb/chipidea/ci.h > @@ -176,6 +176,7 @@ struct hw_bank { > * @enabled_otg_timer_bits: bits of enabled otg timers > * @next_otg_timer: next nearest enabled timer to be expired > * @work: work for role changing > + * @power_lost_work: work for power lost handling > * @wq: workqueue thread > * @qh_pool: allocation pool for queue heads > * @td_pool: allocation pool for transfer descriptors > @@ -226,6 +227,7 @@ struct ci_hdrc { > enum otg_fsm_timer next_otg_timer; > struct usb_role_switch *role_switch; > struct work_struct work; > + struct work_struct power_lost_work; > struct workqueue_struct *wq; > > struct dma_pool *qh_pool; > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c > index 41014f93cfdf..835bf2428dc6 100644 > --- a/drivers/usb/chipidea/core.c > +++ b/drivers/usb/chipidea/core.c > @@ -856,6 +856,27 @@ static int ci_extcon_register(struct ci_hdrc *ci) > return 0; > } > > +static void ci_power_lost_work(struct work_struct *work) > +{ > + struct ci_hdrc *ci = container_of(work, struct ci_hdrc, power_lost_work); > + enum ci_role role; > + > + disable_irq_nosync(ci->irq); > + pm_runtime_get_sync(ci->dev); > + if (!ci_otg_is_fsm_mode(ci)) { > + role = ci_get_role(ci); > + > + if (ci->role != role) { > + ci_handle_id_switch(ci); > + } else if (role == CI_ROLE_GADGET) { > + if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV)) > + usb_gadget_vbus_connect(&ci->gadget); > + } > + } > + pm_runtime_put_sync(ci->dev); > + enable_irq(ci->irq); > +} > + > static DEFINE_IDA(ci_ida); > > struct platform_device *ci_hdrc_add_device(struct device *dev, > @@ -1045,6 +1066,8 @@ static int ci_hdrc_probe(struct platform_device *pdev) > > spin_lock_init(&ci->lock); > mutex_init(&ci->mutex); > + INIT_WORK(&ci->power_lost_work, ci_power_lost_work); > + > ci->dev = dev; > ci->platdata = dev_get_platdata(dev); > ci->imx28_write_fix = !!(ci->platdata->flags & > @@ -1396,25 +1419,6 @@ static int ci_suspend(struct device *dev) > return 0; > } > > -static void ci_handle_power_lost(struct ci_hdrc *ci) > -{ > - enum ci_role role; > - > - disable_irq_nosync(ci->irq); > - if (!ci_otg_is_fsm_mode(ci)) { > - role = ci_get_role(ci); > - > - if (ci->role != role) { > - ci_handle_id_switch(ci); > - } else if (role == CI_ROLE_GADGET) { > - if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV)) > - usb_gadget_vbus_connect(&ci->gadget); > - } > - } > - > - enable_irq(ci->irq); > -} > - > static int ci_resume(struct device *dev) > { > struct ci_hdrc *ci = dev_get_drvdata(dev); > @@ -1446,7 +1450,7 @@ static int ci_resume(struct device *dev) > ci_role(ci)->resume(ci, power_lost); > > if (power_lost) > - ci_handle_power_lost(ci); > + queue_work(system_freezable_wq, &ci->power_lost_work); > > if (ci->supports_runtime_pm) { > pm_runtime_disable(dev); > -- > 2.34.1 > >
diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h index d9bb3d3f026e..2a38e1eb6546 100644 --- a/drivers/usb/chipidea/ci.h +++ b/drivers/usb/chipidea/ci.h @@ -176,6 +176,7 @@ struct hw_bank { * @enabled_otg_timer_bits: bits of enabled otg timers * @next_otg_timer: next nearest enabled timer to be expired * @work: work for role changing + * @power_lost_work: work for power lost handling * @wq: workqueue thread * @qh_pool: allocation pool for queue heads * @td_pool: allocation pool for transfer descriptors @@ -226,6 +227,7 @@ struct ci_hdrc { enum otg_fsm_timer next_otg_timer; struct usb_role_switch *role_switch; struct work_struct work; + struct work_struct power_lost_work; struct workqueue_struct *wq; struct dma_pool *qh_pool; diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c index 41014f93cfdf..835bf2428dc6 100644 --- a/drivers/usb/chipidea/core.c +++ b/drivers/usb/chipidea/core.c @@ -856,6 +856,27 @@ static int ci_extcon_register(struct ci_hdrc *ci) return 0; } +static void ci_power_lost_work(struct work_struct *work) +{ + struct ci_hdrc *ci = container_of(work, struct ci_hdrc, power_lost_work); + enum ci_role role; + + disable_irq_nosync(ci->irq); + pm_runtime_get_sync(ci->dev); + if (!ci_otg_is_fsm_mode(ci)) { + role = ci_get_role(ci); + + if (ci->role != role) { + ci_handle_id_switch(ci); + } else if (role == CI_ROLE_GADGET) { + if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV)) + usb_gadget_vbus_connect(&ci->gadget); + } + } + pm_runtime_put_sync(ci->dev); + enable_irq(ci->irq); +} + static DEFINE_IDA(ci_ida); struct platform_device *ci_hdrc_add_device(struct device *dev, @@ -1045,6 +1066,8 @@ static int ci_hdrc_probe(struct platform_device *pdev) spin_lock_init(&ci->lock); mutex_init(&ci->mutex); + INIT_WORK(&ci->power_lost_work, ci_power_lost_work); + ci->dev = dev; ci->platdata = dev_get_platdata(dev); ci->imx28_write_fix = !!(ci->platdata->flags & @@ -1396,25 +1419,6 @@ static int ci_suspend(struct device *dev) return 0; } -static void ci_handle_power_lost(struct ci_hdrc *ci) -{ - enum ci_role role; - - disable_irq_nosync(ci->irq); - if (!ci_otg_is_fsm_mode(ci)) { - role = ci_get_role(ci); - - if (ci->role != role) { - ci_handle_id_switch(ci); - } else if (role == CI_ROLE_GADGET) { - if (ci->is_otg && hw_read_otgsc(ci, OTGSC_BSV)) - usb_gadget_vbus_connect(&ci->gadget); - } - } - - enable_irq(ci->irq); -} - static int ci_resume(struct device *dev) { struct ci_hdrc *ci = dev_get_drvdata(dev); @@ -1446,7 +1450,7 @@ static int ci_resume(struct device *dev) ci_role(ci)->resume(ci, power_lost); if (power_lost) - ci_handle_power_lost(ci); + queue_work(system_freezable_wq, &ci->power_lost_work); if (ci->supports_runtime_pm) { pm_runtime_disable(dev);
When power is recycled in usb controller during system power management, the controller will recognize it and switch role if role has been changed during power lost. In current design, it will be completed in resume() function. However, this may bring issues since usb class devices have their pm operations too and these device's resume() functions are still not being called at this point. When usb controller recognized host role should be stopped, these usb class devices will be removed at this point. But these usb class devices can't be removed in some cases, such as scsi devices. Since scsi driver may sync data to U-disk, however it will block there because scsi drvier can only handle pm request when is in suspended state. Therefore, there may exist a dependency between ci_resume() and usb class device's resume(). To break this potential dependency, we need to handle power lost work in a workqueue. Fixes: 74494b33211d ("usb: chipidea: core: add controller resume support when controller is powered off") cc: <stable@vger.kernel.org> Signed-off-by: Xu Yang <xu.yang_2@nxp.com> --- drivers/usb/chipidea/ci.h | 2 ++ drivers/usb/chipidea/core.c | 44 ++++++++++++++++++++----------------- 2 files changed, 26 insertions(+), 20 deletions(-)