Hi Heikki,
> -----Original Message-----
> From: linux-usb-owner@xxxxxxxxxxxxxxx <linux-usb-owner@xxxxxxxxxxxxxxx>
> On Behalf Of Heikki Krogerus
> Sent: Friday, December 13, 2019 4:38 AM
> To: Ajay Gupta <ajayg@xxxxxxxxxx>
> Cc: Ajay Gupta <ajaykuee@xxxxxxxxx>; linux-usb@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v6] usb: typec: ucsi: add support for separate DP altmode
> devices
>
> External email: Use caution opening links or attachments
>
>
> On Thu, Dec 12, 2019 at 05:42:28PM +0000, Ajay Gupta wrote:
> > Hi Heikki,
> >
> > > -----Original Message-----
> > > From: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > > Sent: Thursday, December 12, 2019 5:44 AM
> > > To: Ajay Gupta <ajaykuee@xxxxxxxxx>
> > > Cc: linux-usb@xxxxxxxxxxxxxxx; Ajay Gupta <ajayg@xxxxxxxxxx>
> > > Subject: Re: [PATCH v6] usb: typec: ucsi: add support for separate
> > > DP altmode devices
> > >
> > >
> > > Hi Ajay,
> > >
> > > On Fri, Nov 22, 2019 at 04:43:47PM -0800, Ajay Gupta wrote:
> > > > From: Ajay Gupta <ajayg@xxxxxxxxxx>
> > > >
> > > > CCGx controller used on NVIDIA GPU card has two separate display
> > > > altmode for two DP pin assignments. UCSI specification doesn't
> > > > prohibits using separate display altmode.
> > > >
> > > > Current UCSI Type-C framework expects only one display altmode for
> > > > all DP pin assignment. This patch squashes two separate display
> > > > altmode into single altmode to support controllers with separate
> > > > display altmode. We first read all the alternate modes of
> > > > connector and then run through it to know if there are separate
> > > > display altmodes. If so, it prepares a new port altmode set after
> > > > squashing two or more separate altmodes into one.
> > >
> > > I didn't see any major issues with this. There were still few extra
> > > spaces etc., but I can clean those. Maybe it would have been good to
> > > mention in the commit message that the reason why we need those two
> > > separate alt modes, for what is in reality two separate pin
> > > configurations, is limitations in UCSI specification, but never mind.
> > >
> > > I still don't like the approach, but since I'm unable to explain my
> > > idea, or have time to write something for this myself, I don't want
> > > block this any longer. It does not add that much code, so once I
> > > have time, I can always try to improve it myself, right?
> > >
> > > Otherwise it's OK by me. I'll queue it up.
> > Thanks for reviewing. Please feel free to improve the patch or commit
> message.
>
> One thing that I do want to do is I want to isolate the changes done to ucsi.c.
> Can you test the attached diff? It applies on top of this patch.
Tested the diff and it looks good. Please add additional change at [A] on top of
your diff..
Thanks
Ajay
> nvpublic
----------------------- [A] ---------------------------
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -368,11 +368,8 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
if (!len)
break;
- if (!alt.svid) {
- /* break out of outer loop and register */
- i = max_altmodes;
+ if (!alt.svid)
break;
- }
orig[k].mid = alt.mid;
orig[k].svid = alt.svid;
@@ -382,7 +379,7 @@ ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
* Update the original altmode table as some ppms may report
* multiple DP altmodes.
*/
- if (recipient == UCSI_RECIPIENT_CON && ucsi->ops->update_altmodes)
+ if (recipient == UCSI_RECIPIENT_CON)
multi_dp = ucsi->ops->update_altmodes(ucsi, orig, updated);
/* now register altmodes */
----------------------------------------------------------
>
> thanks,
>
> --
> heikki
