On Mon, Apr 25, 2022 at 06:18:05PM +0800, Linyu Yuan wrote:
> 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 ucsi_unregister_connectors() for two places,
> inside this function, if con->wq is NULL, it will break the loop,
> if other kind of error happen after con->wq allocated,
> ucsi/typec related API is safe to unregister.
>
> Also 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,
> remove the extra one connector to save memory.
>
> Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> 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
> v6: merge patch#2 in v5 to this one,
> remove con->port = NULL; and change break condition
> in ucsi_unregister_connectors().
> v7: break for loop when !con->wq
>
> drivers/usb/typec/ucsi/ucsi.c | 53 +++++++++++++++++++++++--------------------
> 1 file changed, 29 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index f0c2fa1..a168510 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1186,6 +1186,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->wq)
> + break;
> +
> + cancel_work_sync(&con->work);
> + ucsi_unregister_partner(con);
> + ucsi_unregister_altmodes(con, UCSI_RECIPIENT_CON);
> + ucsi_unregister_port_psy(con);
> + 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 +1220,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;
> @@ -1225,7 +1250,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;
> @@ -1249,15 +1274,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 +1380,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 +1387,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
--
heikki
