Re: UCSI:CCG: AMD Platform

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

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

Hi,

On Sat, Feb 29, 2020 at 08:55:50AM +0530, Shah, Nehal-bakulchandra wrote:
> Hi
> On 2/27/2020 10:29 PM, Shah, Nehal-bakulchandra wrote:
> > Hi
> > On 2/27/2020 5:53 PM, Heikki Krogerus wrote:
> >> On Mon, Feb 24, 2020 at 02:38:12PM +0530, Shah, Nehal-bakulchandra wrote:
> >>> Hi
> >>>
> >>> On 2/14/2020 7:58 PM, Shah, Nehal-bakulchandra wrote:
> >>>> Hi
> >>>>
> >>>> On 2/13/2020 5:35 PM, Heikki Krogerus wrote:
> >>>>> On Thu, Feb 13, 2020 at 02:00:14PM +0200, Heikki Krogerus wrote:
> >>>>>>> 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?
> >>>>> Actually, don't try that one. Try this one instead.
> >>>> Sure i will update on this on Monday.
> >>>>
> >>>>
> >>>> thanks
> >>>>
> >>>> Nehal
> >>> Patch is not solving the issue. I have attached both trace and dmesg output.
> >> How about if you try this (the attached patch) together with that
> >> previous diff?
> >>
> >> thanks,
> > Sure, infact i suspected that in first place and tried same logic but it was failed but now i will check with both patch combine and shall update.
> >
> > Thanks
> >
> > Nehal Shah
> 
> This is still crashing .

Sorry about the slow process with this, and the late reply.

Right now I'm out of ideas. I'll need to get my hands on the products
that allow me to reproduce the issue. Staring at the code does not
help anymore.

I'm going to cleanup the code a little bit in any case. I'm attaching
a diff with my changes. I don't think it will fix this issue, but I
would appreciate if you tested it in any case, just to be sure.

thanks,

-- 
heikki

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

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index 048381c058a5..1a5c76907827 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -263,10 +263,12 @@ static void ucsi_displayport_work(struct work_struct *work)
 	mutex_unlock(&dp->con->lock);
 }
 
-void ucsi_displayport_remove_partner(struct typec_altmode *alt)
+void ucsi_displayport_remove_partner(struct typec_altmode *pdev)
 {
+	const struct typec_altmode *alt;
 	struct ucsi_dp *dp;
 
+	alt = typec_altmode_get_partner(pdev);
 	if (!alt)
 		return;
 
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ddf2ad3752de..921499407403 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -432,8 +432,12 @@ static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
 		command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
 		command |= UCSI_GET_ALTMODE_OFFSET(i);
 		len = ucsi_run_command(con->ucsi, command, alt, sizeof(alt));
-		if (len <= 0)
+		if (len <= 0) {
+			/* Assuming that all alt modes are now registered */
+			if (recipient == UCSI_RECIPIENT_SOP && i)
+				break;
 			return len;
+		}
 
 		/*
 		 * This code is requesting one alt mode at a time, but some PPMs
@@ -464,9 +468,8 @@ static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
 
 static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
 {
-	const struct typec_altmode *pdev;
 	struct typec_altmode **adev;
-	int i = 0;
+	int i;
 
 	switch (recipient) {
 	case UCSI_RECIPIENT_CON:
@@ -479,16 +482,15 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
 		return;
 	}
 
-	while (adev[i]) {
+	for (i = 0; adev[i]; i++) {
 		if (recipient == UCSI_RECIPIENT_SOP &&
 		    (adev[i]->svid == USB_TYPEC_DP_SID ||
-			(adev[i]->svid == USB_TYPEC_NVIDIA_VLINK_SID &&
-			adev[i]->vdo != USB_TYPEC_NVIDIA_VLINK_DBG_VDO))) {
-			pdev = typec_altmode_get_partner(adev[i]);
-			ucsi_displayport_remove_partner((void *)pdev);
-		}
+		     (adev[i]->svid == USB_TYPEC_NVIDIA_VLINK_SID &&
+		      adev[i]->vdo != USB_TYPEC_NVIDIA_VLINK_DBG_VDO)))
+			ucsi_displayport_remove_partner(adev[i]);
+
 		typec_unregister_altmode(adev[i]);
-		adev[i++] = NULL;
+		adev[i] = NULL;
 	}
 }
 
@@ -583,8 +585,8 @@ static void ucsi_partner_change(struct ucsi_connector *con)
 	ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_SOP);
 	if (ret)
 		dev_err(con->ucsi->dev,
-			"con%d: failed to register partner alternate modes\n",
-			con->num);
+			"con%d: failed to register partner alternate modes (%d)\n",
+			con->num, ret);
 	else
 		ucsi_altmode_update_active(con);
 }
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index d834e236c6df..85267df13bf1 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -40,7 +40,8 @@ static inline void typec_altmode_set_drvdata(struct typec_altmode *altmode,
 	dev_set_drvdata(&altmode->dev, data);
 }
 
-static inline void *typec_altmode_get_drvdata(struct typec_altmode *altmode)
+static inline void *
+typec_altmode_get_drvdata(const struct typec_altmode *altmode)
 {
 	return dev_get_drvdata(&altmode->dev);
 }