* [PATCH v5 0/3] usb: typec: ucsi: allow retry to find role switch @ 2022-04-22 3:10 Linyu Yuan 2022-04-22 3:10 ` [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() Linyu Yuan ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Linyu Yuan @ 2022-04-22 3:10 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, Jack Pham, Linyu Yuan when one role switch module built as module, it may load late than ucsi, this change series allow retry from ucsi. v2: improve ucsi_connector_clean() to cover all condition, and add one new change to avoid allocate one unused connector. v3: fix comment from ucsi maintainer. v4: fix review comment from Heikki Krogerus, merge patch#1 and patch#2 in V3 into one patch, add counter for retry limit, and some other minor fix. v5: only update commit description of two patches Linyu Yuan (3): usb: typec: ucsi: add a common function ucsi_unregister_connectors() usb: typec: ucsi: do not allocate one extra unused connector usb: typec: ucsi: Wait for the USB role switches drivers/usb/typec/ucsi/ucsi.c | 86 +++++++++++++++++++++++++------------------ drivers/usb/typec/ucsi/ucsi.h | 6 ++- 2 files changed, 55 insertions(+), 37 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() 2022-04-22 3:10 [PATCH v5 0/3] usb: typec: ucsi: allow retry to find role switch Linyu Yuan @ 2022-04-22 3:10 ` Linyu Yuan 2022-04-25 8:09 ` Heikki Krogerus 2022-04-22 3:10 ` [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector Linyu Yuan 2022-04-22 3:10 ` [PATCH v5 3/3] usb: typec: ucsi: Wait for the USB role switches Linyu Yuan 2 siblings, 1 reply; 9+ messages in thread From: Linyu Yuan @ 2022-04-22 3:10 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, Jack Pham, Linyu Yuan As con->port will be used in error path of ucsi_init(), it should be NULL or valid. In error path of ucsi_init(), it will unregister all valid ucsi connectors, and similar operation also happen in ucsi_unregister(), add a common function for two places. Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> --- v2: improve ucsi_connector_clean(), check total number of connector. v3: rename to ucsi_unregister_connectors(), suggest by maintainer v4: merge patch#1 in V3, fix a typo samiliar -> similar in commit description v5: no change drivers/usb/typec/ucsi/ucsi.c | 52 ++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index f0c2fa1..af9a2a1 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -1100,6 +1100,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) con->port = typec_register_port(ucsi->dev, cap); if (IS_ERR(con->port)) { ret = PTR_ERR(con->port); + con->port = NULL; goto out; } @@ -1186,6 +1187,32 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) return ret; } +static void ucsi_unregister_connectors(struct ucsi *ucsi) +{ + struct ucsi_connector *con; + int i; + + if (!ucsi->connector) + return; + + for (i = 0; i < ucsi->cap.num_connectors; i++) { + con = &ucsi->connector[i]; + if (!con->port) + break; + + cancel_work_sync(&con->work); + ucsi_unregister_partner(con); + ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); + ucsi_unregister_port_psy(con); + if (con->wq) + destroy_workqueue(con->wq); + typec_unregister_port(con->port); + } + + kfree(ucsi->connector); + ucsi->connector = NULL; +} + /** * ucsi_init - Initialize UCSI interface * @ucsi: UCSI to be initialized @@ -1194,7 +1221,6 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) */ static int ucsi_init(struct ucsi *ucsi) { - struct ucsi_connector *con; u64 command; int ret; int i; @@ -1249,15 +1275,7 @@ static int ucsi_init(struct ucsi *ucsi) return 0; err_unregister: - for (con = ucsi->connector; con->port; con++) { - ucsi_unregister_partner(con); - ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); - ucsi_unregister_port_psy(con); - if (con->wq) - destroy_workqueue(con->wq); - typec_unregister_port(con->port); - con->port = NULL; - } + ucsi_unregister_connectors(ucsi); err_reset: memset(&ucsi->cap, 0, sizeof(ucsi->cap)); @@ -1363,7 +1381,6 @@ EXPORT_SYMBOL_GPL(ucsi_register); void ucsi_unregister(struct ucsi *ucsi) { u64 cmd = UCSI_SET_NOTIFICATION_ENABLE; - int i; /* Make sure that we are not in the middle of driver initialization */ cancel_work_sync(&ucsi->work); @@ -1371,18 +1388,7 @@ void ucsi_unregister(struct ucsi *ucsi) /* Disable notifications */ ucsi->ops->async_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd)); - for (i = 0; i < ucsi->cap.num_connectors; i++) { - cancel_work_sync(&ucsi->connector[i].work); - ucsi_unregister_partner(&ucsi->connector[i]); - ucsi_unregister_altmodes(&ucsi->connector[i], - UCSI_RECIPIENT_CON); - ucsi_unregister_port_psy(&ucsi->connector[i]); - if (ucsi->connector[i].wq) - destroy_workqueue(ucsi->connector[i].wq); - typec_unregister_port(ucsi->connector[i].port); - } - - kfree(ucsi->connector); + ucsi_unregister_connectors(ucsi); } EXPORT_SYMBOL_GPL(ucsi_unregister); -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() 2022-04-22 3:10 ` [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() Linyu Yuan @ 2022-04-25 8:09 ` Heikki Krogerus 2022-04-25 8:23 ` Linyu Yuan (QUIC) 0 siblings, 1 reply; 9+ messages in thread From: Heikki Krogerus @ 2022-04-25 8:09 UTC (permalink / raw) To: Linyu Yuan; +Cc: Greg Kroah-Hartman, linux-usb, Jack Pham On Fri, Apr 22, 2022 at 11:10:20AM +0800, Linyu Yuan wrote: > As con->port will be used in error path of ucsi_init(), > it should be NULL or valid. > > In error path of ucsi_init(), it will unregister all valid ucsi connectors, > and similar operation also happen in ucsi_unregister(), > add a common function for two places. > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > --- > v2: improve ucsi_connector_clean(), check total number of connector. > v3: rename to ucsi_unregister_connectors(), suggest by maintainer > v4: merge patch#1 in V3, fix a typo samiliar -> similar in commit description > v5: no change > > drivers/usb/typec/ucsi/ucsi.c | 52 ++++++++++++++++++++++++------------------- > 1 file changed, 29 insertions(+), 23 deletions(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index f0c2fa1..af9a2a1 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1100,6 +1100,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > con->port = typec_register_port(ucsi->dev, cap); > if (IS_ERR(con->port)) { > ret = PTR_ERR(con->port); > + con->port = NULL; I'm not sure you need to add that line. See below. > goto out; > } > > @@ -1186,6 +1187,32 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > return ret; > } > > +static void ucsi_unregister_connectors(struct ucsi *ucsi) > +{ > + struct ucsi_connector *con; > + int i; > + > + if (!ucsi->connector) > + return; > + > + for (i = 0; i < ucsi->cap.num_connectors; i++) { > + con = &ucsi->connector[i]; > + if (!con->port) > + break; if (IS_ERR_OR_NULL(con->port)) break; > + cancel_work_sync(&con->work); > + ucsi_unregister_partner(con); > + ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); > + ucsi_unregister_port_psy(con); > + if (con->wq) > + destroy_workqueue(con->wq); > + typec_unregister_port(con->port); > + } > + > + kfree(ucsi->connector); > + ucsi->connector = NULL; > +} > + > /** > * ucsi_init - Initialize UCSI interface > * @ucsi: UCSI to be initialized > @@ -1194,7 +1221,6 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) > */ > static int ucsi_init(struct ucsi *ucsi) > { > - struct ucsi_connector *con; > u64 command; > int ret; > int i; > @@ -1249,15 +1275,7 @@ static int ucsi_init(struct ucsi *ucsi) > return 0; > > err_unregister: > - for (con = ucsi->connector; con->port; con++) { > - ucsi_unregister_partner(con); > - ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON); > - ucsi_unregister_port_psy(con); > - if (con->wq) > - destroy_workqueue(con->wq); > - typec_unregister_port(con->port); > - con->port = NULL; > - } > + ucsi_unregister_connectors(ucsi); > > err_reset: > memset(&ucsi->cap, 0, sizeof(ucsi->cap)); > @@ -1363,7 +1381,6 @@ EXPORT_SYMBOL_GPL(ucsi_register); > void ucsi_unregister(struct ucsi *ucsi) > { > u64 cmd = UCSI_SET_NOTIFICATION_ENABLE; > - int i; > > /* Make sure that we are not in the middle of driver initialization */ > cancel_work_sync(&ucsi->work); > @@ -1371,18 +1388,7 @@ void ucsi_unregister(struct ucsi *ucsi) > /* Disable notifications */ > ucsi->ops->async_write(ucsi, UCSI_CONTROL, &cmd, sizeof(cmd)); > > - for (i = 0; i < ucsi->cap.num_connectors; i++) { > - cancel_work_sync(&ucsi->connector[i].work); > - ucsi_unregister_partner(&ucsi->connector[i]); > - ucsi_unregister_altmodes(&ucsi->connector[i], > - UCSI_RECIPIENT_CON); > - ucsi_unregister_port_psy(&ucsi->connector[i]); > - if (ucsi->connector[i].wq) > - destroy_workqueue(ucsi->connector[i].wq); > - typec_unregister_port(ucsi->connector[i].port); > - } > - > - kfree(ucsi->connector); > + ucsi_unregister_connectors(ucsi); > } > EXPORT_SYMBOL_GPL(ucsi_unregister); thanks, -- heikki ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() 2022-04-25 8:09 ` Heikki Krogerus @ 2022-04-25 8:23 ` Linyu Yuan (QUIC) 0 siblings, 0 replies; 9+ messages in thread From: Linyu Yuan (QUIC) @ 2022-04-25 8:23 UTC (permalink / raw) To: Heikki Krogerus, Linyu Yuan (QUIC) Cc: Greg Kroah-Hartman, linux-usb, Jack Pham (QUIC) > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Monday, April 25, 2022 4:10 PM > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux- > usb@vger.kernel.org; Jack Pham (QUIC) <quic_jackp@quicinc.com> > Subject: Re: [PATCH v5 1/3] usb: typec: ucsi: add a common function > ucsi_unregister_connectors() > > On Fri, Apr 22, 2022 at 11:10:20AM +0800, Linyu Yuan wrote: > > As con->port will be used in error path of ucsi_init(), > > it should be NULL or valid. > > > > In error path of ucsi_init(), it will unregister all valid ucsi connectors, > > and similar operation also happen in ucsi_unregister(), > > add a common function for two places. > > > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > > --- > > v2: improve ucsi_connector_clean(), check total number of connector. > > v3: rename to ucsi_unregister_connectors(), suggest by maintainer > > v4: merge patch#1 in V3, fix a typo samiliar -> similar in commit description > > v5: no change > > > > drivers/usb/typec/ucsi/ucsi.c | 52 ++++++++++++++++++++++++----------- > -------- > > 1 file changed, 29 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > > index f0c2fa1..af9a2a1 100644 > > --- a/drivers/usb/typec/ucsi/ucsi.c > > +++ b/drivers/usb/typec/ucsi/ucsi.c > > @@ -1100,6 +1100,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int > index) > > con->port = typec_register_port(ucsi->dev, cap); > > if (IS_ERR(con->port)) { > > ret = PTR_ERR(con->port); > > + con->port = NULL; > > I'm not sure you need to add that line. See below. > > > goto out; > > } > > > > @@ -1186,6 +1187,32 @@ static int ucsi_register_port(struct ucsi *ucsi, int > index) > > return ret; > > } > > > > +static void ucsi_unregister_connectors(struct ucsi *ucsi) > > +{ > > + struct ucsi_connector *con; > > + int i; > > + > > + if (!ucsi->connector) > > + return; > > + > > + for (i = 0; i < ucsi->cap.num_connectors; i++) { > > + con = &ucsi->connector[i]; > > + if (!con->port) > > + break; > > if (IS_ERR_OR_NULL(con->port)) > break; Good suggestion, will change it next version. > > thanks, > > -- > heikki ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector 2022-04-22 3:10 [PATCH v5 0/3] usb: typec: ucsi: allow retry to find role switch Linyu Yuan 2022-04-22 3:10 ` [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() Linyu Yuan @ 2022-04-22 3:10 ` Linyu Yuan 2022-04-25 8:13 ` Heikki Krogerus 2022-04-22 3:10 ` [PATCH v5 3/3] usb: typec: ucsi: Wait for the USB role switches Linyu Yuan 2 siblings, 1 reply; 9+ messages in thread From: Linyu Yuan @ 2022-04-22 3:10 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, Jack Pham, Linyu Yuan In ucsi_init(), it allocate number of (ucsi->cap.num_connectors + 1) connectors, there is one extra as the ending. ucsi_unregister_connectors() is safe to unregister all ucsi connectors according ucsi->cap.num_connectors. Let's remove the extra one connector to save memory. Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> --- v2: new change v3: no change v4: fix a typo extral -> extra in commit description v5: update commit description drivers/usb/typec/ucsi/ucsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index af9a2a1..ce9192e 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -1251,7 +1251,7 @@ static int ucsi_init(struct ucsi *ucsi) } /* Allocate the connectors. Released in ucsi_unregister() */ - ucsi->connector = kcalloc(ucsi->cap.num_connectors + 1, + ucsi->connector = kcalloc(ucsi->cap.num_connectors, sizeof(*ucsi->connector), GFP_KERNEL); if (!ucsi->connector) { ret = -ENOMEM; -- 2.7.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector 2022-04-22 3:10 ` [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector Linyu Yuan @ 2022-04-25 8:13 ` Heikki Krogerus 2022-04-25 8:23 ` Linyu Yuan (QUIC) 0 siblings, 1 reply; 9+ messages in thread From: Heikki Krogerus @ 2022-04-25 8:13 UTC (permalink / raw) To: Linyu Yuan; +Cc: Greg Kroah-Hartman, linux-usb, Jack Pham On Fri, Apr 22, 2022 at 11:10:21AM +0800, Linyu Yuan wrote: > In ucsi_init(), it allocate number of (ucsi->cap.num_connectors + 1) > connectors, there is one extra as the ending. > ucsi_unregister_connectors() is safe to unregister all ucsi connectors > according ucsi->cap.num_connectors. > > Let's remove the extra one connector to save memory. Maybe you could just merge this one into the first patch. > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > --- > v2: new change > v3: no change > v4: fix a typo extral -> extra in commit description > v5: update commit description > > drivers/usb/typec/ucsi/ucsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > index af9a2a1..ce9192e 100644 > --- a/drivers/usb/typec/ucsi/ucsi.c > +++ b/drivers/usb/typec/ucsi/ucsi.c > @@ -1251,7 +1251,7 @@ static int ucsi_init(struct ucsi *ucsi) > } > > /* Allocate the connectors. Released in ucsi_unregister() */ > - ucsi->connector = kcalloc(ucsi->cap.num_connectors + 1, > + ucsi->connector = kcalloc(ucsi->cap.num_connectors, > sizeof(*ucsi->connector), GFP_KERNEL); > if (!ucsi->connector) { > ret = -ENOMEM; > -- > 2.7.4 thanks, -- heikki ^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector 2022-04-25 8:13 ` Heikki Krogerus @ 2022-04-25 8:23 ` Linyu Yuan (QUIC) 0 siblings, 0 replies; 9+ messages in thread From: Linyu Yuan (QUIC) @ 2022-04-25 8:23 UTC (permalink / raw) To: Heikki Krogerus, Linyu Yuan (QUIC) Cc: Greg Kroah-Hartman, linux-usb, Jack Pham (QUIC) > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Monday, April 25, 2022 4:14 PM > To: Linyu Yuan (QUIC) <quic_linyyuan@quicinc.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; linux- > usb@vger.kernel.org; Jack Pham (QUIC) <quic_jackp@quicinc.com> > Subject: Re: [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra > unused connector > > On Fri, Apr 22, 2022 at 11:10:21AM +0800, Linyu Yuan wrote: > > In ucsi_init(), it allocate number of (ucsi->cap.num_connectors + 1) > > connectors, there is one extra as the ending. > > ucsi_unregister_connectors() is safe to unregister all ucsi connectors > > according ucsi->cap.num_connectors. > > > > Let's remove the extra one connector to save memory. > > Maybe you could just merge this one into the first patch. Sure, thanks, will merge next version. > > > Signed-off-by: Linyu Yuan <quic_linyyuan@quicinc.com> > > --- > > v2: new change > > v3: no change > > v4: fix a typo extral -> extra in commit description > > v5: update commit description > > > > drivers/usb/typec/ucsi/ucsi.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c > > index af9a2a1..ce9192e 100644 > > --- a/drivers/usb/typec/ucsi/ucsi.c > > +++ b/drivers/usb/typec/ucsi/ucsi.c > > @@ -1251,7 +1251,7 @@ static int ucsi_init(struct ucsi *ucsi) > > } > > > > /* Allocate the connectors. Released in ucsi_unregister() */ > > - ucsi->connector = kcalloc(ucsi->cap.num_connectors + 1, > > + ucsi->connector = kcalloc(ucsi->cap.num_connectors, > > sizeof(*ucsi->connector), GFP_KERNEL); > > if (!ucsi->connector) { > > ret = -ENOMEM; > > -- > > 2.7.4 > > thanks, > > -- > heikki ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v5 3/3] usb: typec: ucsi: Wait for the USB role switches 2022-04-22 3:10 [PATCH v5 0/3] usb: typec: ucsi: allow retry to find role switch Linyu Yuan 2022-04-22 3:10 ` [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() Linyu Yuan 2022-04-22 3:10 ` [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector Linyu Yuan @ 2022-04-22 3:10 ` Linyu Yuan 2022-04-25 8:14 ` Heikki Krogerus 2 siblings, 1 reply; 9+ messages in thread From: Linyu Yuan @ 2022-04-22 3:10 UTC (permalink / raw) To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, Jack Pham, Linyu Yuan 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@quicinc.com> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v5 3/3] usb: typec: ucsi: Wait for the USB role switches 2022-04-22 3:10 ` [PATCH v5 3/3] usb: typec: ucsi: Wait for the USB role switches Linyu Yuan @ 2022-04-25 8:14 ` Heikki Krogerus 0 siblings, 0 replies; 9+ messages in thread From: Heikki Krogerus @ 2022-04-25 8:14 UTC (permalink / raw) To: Linyu Yuan; +Cc: Greg Kroah-Hartman, linux-usb, Jack Pham 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@quicinc.com> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-04-25 8:24 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-04-22 3:10 [PATCH v5 0/3] usb: typec: ucsi: allow retry to find role switch Linyu Yuan 2022-04-22 3:10 ` [PATCH v5 1/3] usb: typec: ucsi: add a common function ucsi_unregister_connectors() Linyu Yuan 2022-04-25 8:09 ` Heikki Krogerus 2022-04-25 8:23 ` Linyu Yuan (QUIC) 2022-04-22 3:10 ` [PATCH v5 2/3] usb: typec: ucsi: do not allocate one extra unused connector Linyu Yuan 2022-04-25 8:13 ` Heikki Krogerus 2022-04-25 8:23 ` Linyu Yuan (QUIC) 2022-04-22 3:10 ` [PATCH v5 3/3] usb: typec: ucsi: Wait for the USB role switches Linyu Yuan 2022-04-25 8:14 ` Heikki Krogerus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.