On 2/1/2023 4:50 PM, Heikki Krogerus wrote:
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 case i can think is connect a mobile device to PC through USB A-C cable,
and run data role switch command on mobile device, it definitely will
fail, right ?
the problem is if ppm can't response reset timely, the data role switch
will exit after 10 seconds,
it is very long time.
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.
do you mean other command can run before role switch command operation ?
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
