RE: [PATCH v5] usb: typec: ucsi: add support for separate DP altmode devices

[Ajay Gupta <ajayg@nvidia.com>]

Hi Heikki,

> -----Original Message-----
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Sent: Friday, September 13, 2019 8:20 AM
> To: Ajay Gupta <ajaykuee@gmail.com>
> Cc: linux-usb@vger.kernel.org; Ajay Gupta <ajayg@nvidia.com>
> Subject: Re: [PATCH v5] usb: typec: ucsi: add support for separate DP altmode
> devices
> 
> Hi Ajay,
> 
> On Fri, Sep 06, 2019 at 09:55:16AM -0700, Ajay Gupta wrote:
> > From: Ajay Gupta <ajayg@nvidia.com>
> >
> > 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'm going to go over this one more time during the merge window. I have an
> idea how to improve this. Please bear with me. I have to make sure all options
> were considered.
> 
> I'm really sorry to block this for so long, but I'm just really really afraid of
> mixing special case solutions like this into otherwise generic code.

I can update the patch to make sure generic code flow remains as-is and special
case is executed only for UCSI controllers which needs squashing of duplicate
DP altmodes.
We can use the new function pointer update_altmode() added in "struct ucsi_ppm" in v5
of my patch for this purpose.

if (recipient == UCSI_RECIPIENT_CON && ucsi->ppm && ucsi->ppm->update_altmodes)
	Get all the altmodes first, pass it to update_altmodes() for squashing if needed,
	and then register each altmodes.
else
	Use the current method where we get and register one altmode at a time.

Please let me know if this looks fine to you.

Thanks
> nvpublic
> They always
> make it difficult to modify the code later, and unfortunately we have them all
> over the kernel. Look at the serial driver 8250_core/port/*.cas an example.
> Ideally we should be able to "modularize" the whole alternate mode handling
> in the ucsi driver since there are now two different ways the PPMs can handle
> them instead of simply mixing the support for the new method straight into
> the existing structures.
> 
> thanks,
> 
> --
> heikki