> From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> Sent: Monday, April 25, 2022 4:10 PM
> To: Linyu Yuan (QUIC) <quic_linyyuan@xxxxxxxxxxx>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>; linux-
> usb@xxxxxxxxxxxxxxx; Jack Pham (QUIC) <quic_jackp@xxxxxxxxxxx>
> 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@xxxxxxxxxxx>
> > ---
> > 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
