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, -- heikki -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html