Re: UCSI:CCG: AMD Platform

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

On Mon, Feb 10, 2020 at 03:39:38PM +0530, Shah, Nehal-bakulchandra wrote:
> Hi
> 
> Sorry for the delayed response. I was on vacation.
> On 2/3/2020 7:02 PM, Heikki Krogerus wrote:
> > On Mon, Feb 03, 2020 at 03:28:11PM +0200, Heikki Krogerus wrote:
> >> Hi,
> >>
> >> On Mon, Feb 03, 2020 at 10:52:52AM +0530, Shah, Nehal-bakulchandra wrote:
> >>> Currently i am working on enabling UCSI support
> >>> for CCGx based controller on AMD GPU Cards.
> >>>
> >>> Now i am observing the issue reported here when
> >>> i unplug the cable.
> >>>
> >>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.redhat.com%2Fshow_bug.cgi%3Fid%3D1762031&data=02%7C01%7CNehal-bakulchandra.Shah%40amd.com%7Ceb1ac5e877db4fa9d75f08d7a8ad87a3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637163335569266081&sdata=KemJKkVhpqDo%2FSbHhVaMz7jrcploEALJYg%2BRWvhJ7bM%3D&reserved=0
> >>>
> >>> Also would like to know is there any way we can
> >>> get user level notifications for UCSI?
> >>
> >> If you want to see the actual UCSI notification in user space, then
> >> that is not possible, but the driver does produce trace output, and I
> >> would actually like to see what we got there. You need debugfs to be
> >> mounted. Then try the following:
> >>
> >>         # Unload all UCSI modules
> >>         modprobe -r ucsi_acpi
> >>
> >>         # At this point you should plug-in the problematic device
> >>
> >>         # Reload the UCSI core module
> >>         modprobe typec_ucsi
> >>
> >>         # Enable UCSI tracing
> >>         echo 1 > /sys/kernel/debug/tracing/events/ucsi/enable
> >>
> >>         # Now reload the ACPI glue driver
> >>         modprobe ucsi_acpi
> >>
> >>         # Unplug the problematic device so that you see the error
> >>
> >>         # Finally dump the trace output
> >>         cat /sys/kernel/debug/tracing/trace
> >>
> >> So if that works, please send the trace output to me.
> > 
> > Actually, first things first. Please share your dmesg output. Are you
> > using ucsi_acpi or ucsi_ccg glue driver?
> > 
> > thanks,
> > 
> 
> I am using CCG based UCSI driver without any
> modification.For I2C part i have written custom
> driver.
> 
> I have attached the trace out and dmesg crash log.
> 
> Please have a look

Thanks for the logs. Can you test the attached diff?

thanks,

-- 
heikki

[-- Attachment #2: port_lock.diff --]
[-- Type: text/plain, Size: 1485 bytes --]

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ddf2ad3752de..37837bf5385b 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -870,12 +870,16 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	con->num = index + 1;
 	con->ucsi = ucsi;
 
+	mutex_lock(&con->lock);
+
 	/* Get connector capability */
 	command = UCSI_GET_CONNECTOR_CAPABILITY;
 	command |= UCSI_CONNECTOR_NUMBER(con->num);
 	ret = ucsi_run_command(ucsi, command, &con->cap, sizeof(con->cap));
-	if (ret < 0)
+	if (ret < 0) {
+		mutex_unlock(&con->lock);
 		return ret;
+	}
 
 	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP)
 		cap->data = TYPEC_PORT_DRD;
@@ -907,8 +911,10 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 
 	/* Register the connector */
 	con->port = typec_register_port(ucsi->dev, cap);
-	if (IS_ERR(con->port))
+	if (IS_ERR(con->port)) {
+		mutex_unlock(&con->lock);
 		return PTR_ERR(con->port);
+	}
 
 	/* Alternate modes */
 	ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_CON);
@@ -922,6 +928,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 			       sizeof(con->status));
 	if (ret < 0) {
 		dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
+		mutex_unlock(&con->lock);
 		return 0;
 	}
 
@@ -956,6 +963,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 
 	trace_ucsi_register_port(con->num, &con->status);
 
+	mutex_unlock(&con->lock);
 	return 0;
 }