On Tue, Jan 31, 2023 at 05:52:15PM +0800, Linyu Yuan wrote:
> In ucsi_dr_swap() and ucsi_pr_swap(), it will wait complete.
> it is better change role switch command to async mode which will avoid
> reset ppm in condition that data/power switch can't operate.
I think I need a little bit more information. Reseting the whole PPM
is a heavy operation, I do admit that, but you are not really
explaining what happens in your case when the driver does it - why is
it a problem?
The proposal of using async_write here is in any case not acceptable.
You would now end up generationg a spurious command completion event
that can in worst case will screws up some other command.
If the PPM reset is the problem here, then perhaps the proper thing to
do would be to remove that instead?
thanks,
> Signed-off-by: Linyu Yuan <quic_linyyuan@xxxxxxxxxxx>
> ---
> drivers/usb/typec/ucsi/ucsi.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 00fc867..466ae93 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -997,17 +997,21 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
>
> static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
> {
> + struct ucsi *ucsi = con->ucsi;
> int ret;
>
> - ret = ucsi_send_command(con->ucsi, command, NULL, 0);
> + mutex_lock(&ucsi->ppm_lock);
> + ret = ucsi->ops->async_write(ucsi, UCSI_CONTROL, &command, sizeof(command));
> + mutex_unlock(&ucsi->ppm_lock);
> +
> if (ret == -ETIMEDOUT) {
> u64 c;
>
> /* PPM most likely stopped responding. Resetting everything. */
> - ucsi_reset_ppm(con->ucsi);
> + ucsi_reset_ppm(ucsi);
>
> - c = UCSI_SET_NOTIFICATION_ENABLE | con->ucsi->ntfy;
> - ucsi_send_command(con->ucsi, c, NULL, 0);
> + c = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
> + ucsi_send_command(ucsi, c, NULL, 0);
>
> ucsi_reset_connector(con, true);
> }
> --
> 2.7.4
--
heikki
