RE: [PATCH 00/14] usb: typec: UCSI driver overhaul

[Ajay Gupta <ajayg@nvidia.com>]

Hi Heikki,

> > > > Hi Ajay,
> > > >
> > > > Here's the pretty much complete rewrite of the I/O handling that I
> > > > was talking about. The first seven patches are not actually
> > > > related to this stuff, but I'm including them here because the
> > > > rest of the series is made on top of them. I'm including also that
> > > > fix patch I send you earlier.
> > > >
> > > > After this it should be easier to handle quirks. My idea how to
> > > > handle the multi-instance connector alt modes is that we "emulate"
> > > > the PPM in ucsi_ccg.c in order to handle them, so ucsi.c is not
> > > > touched at
> > all.
> > > >
> > > > We can now get the connector alternate modes that the actual
> > > > controller supplies during probe - before registering the ucsi
> > > > interface
> > > How can ucsi_ccg.c get the connector alternate modes before
> > > registering ucsi interface? PPM reset, notification enable, etc.
> > > is done during ucsi registration. UCSI spec says:
> > > " The only commands the PPM is required to process in the "PPM Idle
> > > (Notifications Disabled)" state are SET_NOTIFICATION_ENABLE and
> > > PPM_RESE"
> > >
> > > Also, it doesn't look correct if ucsi_ccg.c has to replicate most of
> > > the stuff done in ucsi_init() of ucsi.c.
> >
> > How about if we split ucsi_init() into a function that first simply
> > constructs the struct ucsi and struct ucsi_connector instances without
> > registering anything, and into separate functions that then register
> > the ports, altmodes and what have you. I don't think that should be a
> > huge problem. It will make ucsi.c even more like a library, which is probable
> a good thing.
> Do you mean the solution to follow steps (a->b->c->d1) or (a->b->c->d2) ?
> a) ucsi_ccg.c calls first part of split ucsi_init()
> b) ucsi_ccg.c uses ucsi_send_command() to collect actual alternate modes.
> c) ucsi_ccg.c looks into actual alternate modes and squashes if duplicate
> altmode found.
> d1) ucsi_ccg.c calls new method to register connector alternate modes.
> This method issues GET_ALTERNATE_MODES command again and ucsi_ccg.c is
> expected to send squashed alternate mode.  This will require changes in
> .read(), .sync_write() and
> .async_write() to make it appear as if the squashed data coming from the ppm.
> OR
> d2) ucsi_ccg.c calls new method to register squashed connector alternate
> modes.
> This method doesn't issue GET_ALTERNATE_MODES commands to PPM but
> simply registers the alternate mode values passed to this function.
> 
> If you mean the (a->b->c->d2) solution then it looks fine to me and would wait
> for patches from you. This solution would mean that GET_ALTERNATE_MODES
> for connector is done only by each ucsi_xxx.c and not by ucsi.c

I am waiting for your comments on this.

Thanks
> nvpublic
> > I can prepare patches for that too if you like?
> > After that you should be able to get the struct ucsi instance that
> > represents the "real" PPM without registering anything by calling a
> > single function, most likely ucsi_init(). And after getting that you
> > can construct the connector alternate modes that we actually register.
> > Finally you register the final interface which does not use
> > ucsi_ccg_ops, but instead something like ucsi_nvidia_ops.
> I didn't understand this part. ucsi_ccg_ops has .read(), .sync_write() and
>  .async_write() interface and they remain same for all ucsi_ccg controllers.
> 
> Thanks
> > How would this sound to you?
> >
> > Br,
> >
> > --
> > heikki