On Fri, Apr 22, 2022 at 11:10:22AM +0800, Linyu Yuan wrote: > When role switch module probe late than ucsi module, > fwnode_usb_role_switch_get() will return -EPROBE_DEFER, > it is better to restart ucsi init work to find > it again every 100ms, total wait time is 10 second. > > It also means change ucsi init work to delayed_work. > > Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx> Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx> > --- > v2: keep original con->num in debug log > v3: change return value from -EAGAIN to PTR_ERR() > v4: change subject line, > add counter for retry limit, > correct commit descripton to match change in V3 > v5: small update of commit description > > drivers/usb/typec/ucsi/ucsi.c | 32 ++++++++++++++++++++------------ > drivers/usb/typec/ucsi/ucsi.h | 6 +++++- > 2 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index ce9192e..11f8808 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1053,6 +1053,14 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > con->num = index + 1; > con->ucsi = ucsi; > > + cap->fwnode = ucsi_find_fwnode(con); > + con->usb_role_sw = fwnode_usb_role_switch_get(cap->fwnode); > + if (IS_ERR(con->usb_role_sw)) { > + dev_err(ucsi->dev, "con%d: failed to get usb role switch\n", > + con->num); > + return PTR_ERR(con->usb_role_sw); > + } > + > /* Delay other interactions with the con until registration is complete */ > mutex_lock(&con->lock); > > @@ -1088,7 +1096,6 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DEBUG_ACCESSORY) > *accessory = TYPEC_ACCESSORY_DEBUG; > > - cap->fwnode = ucsi_find_fwnode(con); > cap->driver_data = con; > cap->ops = &ucsi_ops; > > @@ -1147,13 +1154,6 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > ucsi_port_psy_changed(con); > } > > - con->usb_role_sw = fwnode_usb_role_switch_get(cap->fwnode); > - if (IS_ERR(con->usb_role_sw)) { > - dev_err(ucsi->dev, "con%d: failed to get usb role switch\n", > - con->num); > - con->usb_role_sw = NULL; > - } > - > /* Only notify USB controller if partner supports USB data */ > if (!(UCSI_CONSTAT_PARTNER_FLAGS(con->status.flags) & UCSI_CONSTAT_PARTNER_FLAG_USB)) > u_role = USB_ROLE_NONE; > @@ -1286,12 +1286,20 @@ static int ucsi_init(struct ucsi *ucsi) > > static void ucsi_init_work(struct work_struct *work) > { > - struct ucsi *ucsi = container_of(work, struct ucsi, work); > + struct ucsi *ucsi = container_of(work, struct ucsi, work.work); > int ret; > > ret = ucsi_init(ucsi); > if (ret) > dev_err(ucsi->dev, "PPM init failed (%d)\n", ret); > + > + if (ret == -EPROBE_DEFER) { > + if (ucsi->work_count++ > UCSI_ROLE_SWITCH_WAIT_COUNT) > + return; > + > + queue_delayed_work(system_long_wq, &ucsi->work, > + UCSI_ROLE_SWITCH_INTERVAL); > + } > } > > /** > @@ -1331,7 +1339,7 @@ struct ucsi *ucsi_create(struct device *dev, const struct ucsi_operations *ops) > if (!ucsi) > return ERR_PTR(-ENOMEM); > > - INIT_WORK(&ucsi->work, ucsi_init_work); > + INIT_DELAYED_WORK(&ucsi->work, ucsi_init_work); > mutex_init(&ucsi->ppm_lock); > ucsi->dev = dev; > ucsi->ops = ops; > @@ -1366,7 +1374,7 @@ int ucsi_register(struct ucsi *ucsi) > if (!ucsi->version) > return -ENODEV; > > - queue_work(system_long_wq, &ucsi->work); > + queue_delayed_work(system_long_wq, &ucsi->work, 0); > > return 0; > } > @@ -1383,7 +1391,7 @@ void ucsi_unregister(struct ucsi *ucsi) > u64 cmd = UCSI_SET_NOTIFICATION_ENABLE; > > /* Make sure that we are not in the middle of driver initialization */ > - cancel_work_sync(&ucsi->work); > + cancel_delayed_work_sync(&ucsi->work); > > /* Disable notifications */ > ucsi->ops->async_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd)); > diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h > index 280f1e1..8eb391e 100644 > --- a/drivers/usb/typec/ucsi/ucsi.h > +++ b/drivers/usb/typec/ucsi/ucsi.h > @@ -287,7 +287,11 @@ struct ucsi { > struct ucsi_capability cap; > struct ucsi_connector *connector; > > - struct work_struct work; > + struct delayed_work work; > + int work_count; > +#define UCSI_ROLE_SWITCH_RETRY_PER_HZ 10 > +#define UCSI_ROLE_SWITCH_INTERVAL (HZ / UCSI_ROLE_SWITCH_RETRY_PER_HZ) > +#define UCSI_ROLE_SWITCH_WAIT_COUNT (10 * UCSI_ROLE_SWITCH_RETRY_PER_HZ) > > /* PPM Communication lock */ > struct mutex ppm_lock; > -- > 2.7.4 -- heikki