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. > > Is there a problem providing both svid and sub-mode in the mux set call? The > > > partner drivers should all know what svid they implement. > > By sub-mode, what do you mean? The SVID specific connector state value > > you already get with the mux ->set callback. The mode index number is > > not very useful (with DP for example it will always be 1). > > > > In any case, the mux driver will still need to interpret the SVID > > specific connector states, so what would it change if we supplied also > > the SVID and mode on top of that with the ->set callback? > > Ditto 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
