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). BR // Mats >>> 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, > > -- 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