[RFC,1/7] usb: typec: Generalize mux mode names

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

On Tue, May 15, 2018 at 11:24:07PM +0200, Mats Karrman wrote:
> Hi Heikki,
> 
> On 05/15/2018 09:30 AM, Heikki Krogerus wrote:
> 
> > Hi Mats,
> >
> > On Fri, May 11, 2018 at 09:28:04PM +0200, Mats Karrman wrote:
> >> On 2018-05-11 13:14, Heikki Krogerus wrote:
> >>
> >>> On Fri, May 11, 2018 at 11:05:55AM +0200, Mats Karrman wrote:
> >>>> On 2018-05-10 19:49, Heikki Krogerus wrote:
> >>>>
> >>>>> On Thu, May 10, 2018 at 10:04:21AM +0200, Mats Karrman wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> On 05/09/2018 02:49 PM, Heikki Krogerus wrote:
> >>>>>>
> >>>>>>> On Tue, May 08, 2018 at 09:10:13PM +0200, Mats Karrman wrote:
> >>>>>>>> Hi,
> >>>>>>>>
> >>>>>>>> On 05/08/2018 04:25 PM, Heikki Krogerus wrote:
> >>>>>>>>
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> On Mon, May 07, 2018 at 11:19:40PM +0200, Mats Karrman wrote:
> >>>>>>>>>>>> Even so, when the mux driver "set" function is called, it will just get the
> >>>>>>>>>>>> mode argument but since the mode (TYPEC_STATE_...) is overlapping for different
> >>>>>>>>>>>> AMs if I understand your proposal correctly, the mux also needs to know what AM
> >>>>>>>>>>>> is active.
> >>>>>>>>>>>> Does this imply that the mux set function signature need to change?
> >>>>>>>>>>> My plan was actually to propose we get rid of the current mux handling
> >>>>>>>>>>> (just leave the orientation switch) in favour of the notifications I'm
> >>>>>>>>>>> introducing with the type-c bus for the alternate modes. The current
> >>>>>>>>>>> mux handling is definitely not enough, and does not work in every
> >>>>>>>>>>> scenario, like also you pointed out.
> >>>>>>>>>> So, the mux need to subscribe to each svid:mode pair it is interested in using
> >>>>>>>>>> typec_altmode_register_notifier() and then use those callbacks to switch the correct
> >>>>>>>>>> signals to the connector. And a driver for an off-the-shelf mux device could have
> >>>>>>>>>> the translation between svid:mode pairs and mux device specific control specified by
> >>>>>>>>>> of/acpi properties. Right?
> >>>>>>>>> Yes. That is the plan. Would it work for you?
> >>>>>>>> I think so. I'll give it a go. When about do you think you'll post the next version
> >>>>>>>> of your RFC? Or do you have an updated series available somewhere public?
> >>>>>>> I'll try to put together and post the next version tomorrow.
> >>>>>>>
> >>>>>>> My original plan was actually to use just the notifications with the
> >>>>>>> muxes, but one thing to consider with the notifications is that in
> >>>>>>> practice we have to increment the ref count for the alt mode devices
> >>>>>>> when ever something registers a notifier.
> >>>>>>>
> >>>>>>> To me that does not feel ideal. The dependency should go the other way
> >>>>>>> around in case of the muxes. That is why I liked the separate API and
> >>>>>>> handling for the muxes felt better, as it will do just that. The mux
> >>>>>>> is then a "service" that the port driver can as for, and if it gets a
> >>>>>>> handle to a mux, the mux will have its ref count incremented.
> >>>>>>>
> >>>>>>> So I think fixing the mux API would perhaps be better after all.
> >>>>>>> Thoughts?
> >>>>>> So, we're back to a mux API similar to the current one, where:
> >>>>>> - the port driver and the mux driver are connected through "graph"
> >>>>>> - alt mode driver finds its port mux using the typec class mux api
> >>>>>> - the mux mode setting function arguments now include both svid and mode
> >>>>>>
> >>>>>> I like that.
> >>>>>>
> >>>>>> One thought that popped up again is if we, somewhere down the line,
> >>>>>> will see some super device that support many different alternate modes
> >>>>>> on the same port and therefore will need to have multiple mux devices?
> >>>>>> However I think the mux api could be extended (later on) to support some
> >>>>>> aggregate mux device that manages multiple physical devices.
> >>>>> If we simply had always a mux for every alternate mode, that would not
> >>>>> be a problem. So the port would have its own mux, and every supported
> >>>>> alternate mode also had its own. I think that removes the need to deal
> >>>>> with the svid:mode when using the muxes, as they are already tied to a
> >>>>> specific alternate modes, right? With a single mux device, for example
> >>>>> pi3usb30532, the driver just needs to register a mux for the port and
> >>>>> separate mux for DP, but I don't think that's a huge problem.
> >>>> Hmm... As an hypothetical example I have written a driver for another mux
> >>>> from TI and according to its data sheet:
> >>>>
> >>>> """
> >>>> The HD3SS460 is a generic analog differential
> >>>> passive switch that can work for any high speed
> >>>> interface applications as long as it is biased at a
> >>>> common mode voltage range of 0-2V and has
> >>>> differential signaling with differential amplitude up to
> >>>> 1800mVpp....
> >>>> """
> >>>>
> >>>> What I am thinking is that it e.g. would be possible to use this/a mux with USBSS +
> >>>> 2ch DP + 2ch something else (HDMI?, ThunderBolt?, ???). The problem here is
> >>>> that it is a general mux device so the driver writer does not know what types of
> >>>> muxes to register. I guess it could also be configured using properties but that
> >>>> would be very complicated.
> >>> Why? All the mux driver needs to get from device properties is the
> >>> SVID and the mode.
> >> Sigh... Again, if the same mux handles signals for more than one alternate mode
> >> the driver won't know what alternate mode is intended if it only receives the
> >> connector state which use overlapping numbers for different alternate modes.
> > You are missing the point. We are now registering separate struct
> > typec_mux for every alt mode. The ->set callback will need to be
> > implemented separately for every alt mode.
> >
> > So in case of TI HD3SS460, we need to initially register a struct
> > typec_mux for DP and implement a function for the ->set callback for
> > DP only. If we later need to support another alt mode with that mux
> > (HDMI perhaps), we need to register second struct typec_mux and
> > implement separate function for that alt mode alone and point the
> > ->set callback of the second struct typec_mux to that.
> 
> No, I'm not missing the point... At least not that one :)
> But I think you are missing my point that a driver for a general
> purpose mux device will end up having to register a struct typec_mux
> and implement a ->set function for every possible alternate mode
> that eventually will exist (and can be used with that mux).

OK, we are on the same page. So back to my question. I'll rephrase:
How would separate ->set functions differ from delivering the
SVID:mode on top of the SVID specific connector state to the mux? You
still need to handle every alternate mode separately in the mux
driver.

I doubt that HD3SS460 will (or even can) be used with anything else
except DP. Maybe with HDMI. It definitely will not be usable with all
alternate modes. Realistically, I think we are talking about two, max
three alternate modes any mux driver will ever need to handle.


Thanks,