All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-08 14:25 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-08 14:25 UTC (permalink / raw)
  To: Mats Karrman
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

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?


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-21 13:04 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-21 13:04 UTC (permalink / raw)
  To: Mats Karrman; +Cc: linux-usb

On Fri, May 18, 2018 at 07:26:40AM +0200, Mats Karrman wrote:
> On 2018-05-17 13:50, Heikki Krogerus wrote:
> 
> > On Wed, May 16, 2018 at 11:11:02PM +0200, Mats Karrman wrote:
> > > On 05/16/2018 01:43 PM, Heikki Krogerus wrote:
> > > 
> > > > 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.
> > > My idea, as I tried to explain before, is to use properties for mapping
> > > mux device specific states to svid:mode:state. The mux driver would not
> > > need to know anything about alternate modes, the responsibility would be
> > > with the system architect to get the mapping right in firmware.
> > OK, I understand. I'm a little bit scared about that kind of mappings.
> > At least the connector state value we are dealing with here is Linux
> > kernel specific index number, so I don't know if it's OK to get that
> > from a device property. I wonder if string identifiers would be more
> > acceptable for hardware descriptions?
> 
> Yes, something in the line of:
> 
> ?????? mappings {
> ?????? ?????? map {
> ?????? ?????? ?????? svid = "displayport";
> ?????? ?????? ?????? state = "pin-assy-c";
> ?????? ?????? ?????? setting = "somethingsomething";?????? // device driver specific
> ?????? ?????? };
> ?????? ?????? map {
> ?????? ?????? ?????? ...etc...
> ?????? };
> 
> Then the svid and state strings could be mapped to numbers by typec/class API
> and setting strings by the mux driver internally and the mux driver will act on
> svid:state info from the partner driver(s) tied to the mux.
> 
> > Strings or numbers, I guess we would need to document somewhere to
> > which alternate mode connector state a number/string is mapped to. I
> > don't know if OF bindings is enough?
> 
> That's what I had in mind.
> 
> > Btw. I'm not convinced we would ever get this information from ACPI
> > tables on devices targeted for windows, but let's not worry about that
> > now.
> 
> Ah, I admit I did not consider this case. Being used to embeddeed/devicetree systems
> I just imagined the hardware description as something you are always in control of.
> I'm afraid I have no idea about this.

It should not prevent us from using the solution. We just can't assume
that the HW description can always supply this information, and
implement the solution with that in mind as usual.


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-18  5:26 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-18  5:26 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: linux-usb

On 2018-05-17 13:50, Heikki Krogerus wrote:

> On Wed, May 16, 2018 at 11:11:02PM +0200, Mats Karrman wrote:
>> On 05/16/2018 01:43 PM, Heikki Krogerus wrote:
>>
>>> 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.
>> My idea, as I tried to explain before, is to use properties for mapping
>> mux device specific states to svid:mode:state. The mux driver would not
>> need to know anything about alternate modes, the responsibility would be
>> with the system architect to get the mapping right in firmware.
> OK, I understand. I'm a little bit scared about that kind of mappings.
> At least the connector state value we are dealing with here is Linux
> kernel specific index number, so I don't know if it's OK to get that
> from a device property. I wonder if string identifiers would be more
> acceptable for hardware descriptions?

Yes, something in the line of:

     mappings {
         map {
             svid = "displayport";
             state = "pin-assy-c";
             setting = "somethingsomething";    // device driver specific
         };
         map {
             ...etc...
     };

Then the svid and state strings could be mapped to numbers by typec/class API
and setting strings by the mux driver internally and the mux driver will act on
svid:state info from the partner driver(s) tied to the mux.

> Strings or numbers, I guess we would need to document somewhere to
> which alternate mode connector state a number/string is mapped to. I
> don't know if OF bindings is enough?

That's what I had in mind.

> Btw. I'm not convinced we would ever get this information from ACPI
> tables on devices targeted for windows, but let's not worry about that
> now.

Ah, I admit I did not consider this case. Being used to embeddeed/devicetree systems
I just imagined the hardware description as something you are always in control of.
I'm afraid I have no idea about this.

BR // Mats

> In any case, I'm not against your idea.
>
>>> 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.
>> I'm not so sure. Time will tell.
>> And using HD3SS460 as an example, there are multiple ways to connect the
>> DP signals to the MUX that the hw designer may want to take advantage of
>> to get the best possible lay-out
> OK.
>
> Thanks,
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-17 11:50 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-17 11:50 UTC (permalink / raw)
  To: Mats Karrman; +Cc: linux-usb

On Wed, May 16, 2018 at 11:11:02PM +0200, Mats Karrman wrote:
> On 05/16/2018 01:43 PM, Heikki Krogerus wrote:
> 
> > 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.
> 
> My idea, as I tried to explain before, is to use properties for mapping
> mux device specific states to svid:mode:state. The mux driver would not
> need to know anything about alternate modes, the responsibility would be
> with the system architect to get the mapping right in firmware.

OK, I understand. I'm a little bit scared about that kind of mappings.
At least the connector state value we are dealing with here is Linux
kernel specific index number, so I don't know if it's OK to get that
from a device property. I wonder if string identifiers would be more
acceptable for hardware descriptions?

Strings or numbers, I guess we would need to document somewhere to
which alternate mode connector state a number/string is mapped to. I
don't know if OF bindings is enough?

Btw. I'm not convinced we would ever get this information from ACPI
tables on devices targeted for windows, but let's not worry about that
now.

In any case, I'm not against your idea.

> > 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.
> 
> I'm not so sure. Time will tell.
> And using HD3SS460 as an example, there are multiple ways to connect the
> DP signals to the MUX that the hw designer may want to take advantage of
> to get the best possible lay-out

OK.


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-16 21:11 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-16 21:11 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: linux-usb

On 05/16/2018 01:43 PM, Heikki Krogerus wrote:

> 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.

My idea, as I tried to explain before, is to use properties for mapping
mux device specific states to svid:mode:state. The mux driver would not
need to know anything about alternate modes, the responsibility would be
with the system architect to get the mapping right in firmware.

> 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.

I'm not so sure. Time will tell.
And using HD3SS460 as an example, there are multiple ways to connect the
DP signals to the MUX that the hw designer may want to take advantage of
to get the best possible lay-out

BR // Mats
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-16 11:43 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-16 11:43 UTC (permalink / raw)
  To: Mats Karrman; +Cc: linux-usb

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,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-15 21:24 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-15 21:24 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: linux-usb

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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-15  7:30 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-15  7:30 UTC (permalink / raw)
  To: Mats Karrman; +Cc: linux-usb

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,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-11 19:28 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-11 19:28 UTC (permalink / raw)
  To: Heikki Krogerus; +Cc: linux-usb

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.

> 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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-09 12:49 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-09 12:49 UTC (permalink / raw)
  To: Mats Karrman
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

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?


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-08 19:10 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-08 19:10 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

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?

BR // Mats

> Thanks,
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-07 21:19 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-07 21:19 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

Hi Heikki,

On 05/07/2018 03:39 PM, Heikki Krogerus wrote:

> Hi Mats,
>
> On Fri, May 04, 2018 at 06:57:31PM +0200, Mats Karrman wrote:
>> On 2018-05-04 16:56, Heikki Krogerus wrote:
>>
>>> On Wed, May 02, 2018 at 03:13:44PM +0200, Mats Karrman wrote:
>>>> Hi Heikki,
>>>>
>>>> On 2018-05-02 10:25, Heikki Krogerus wrote:
>>>>
>>>>> On Wed, May 02, 2018 at 11:23:35AM +0300, Heikki Krogerus wrote:
>>>>>> Hi Mats,
>>>>>>
>>>>>> On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
>>>>>>> The current naming used for tcpc_mux_mode constants makes
>>>>>>> too much assumptioms about the usage of the signals.
>>>>>>> This patch replaces the names with generic names more closely
>>>>>>> tied to the Type-C specifications and also adds some new ones.
>>>>>>> At the same time TCPC_MUX_* defines are removed as they do not
>>>>>>> fit the new concept and currently have no in-tree users.
>>>>>> I'm afraid trying to generalize the modal connector states even like
>>>>>> this is not going to work. We can't make any assumptions about how the
>>>>>> alternate modes configure the pins, or the connector in general.
>>>>>>
>>>>>> The only way this will work is that every alternate mode has its own
>>>>>> configurations defined separately, and I'm talking about the actual
>>>>>> pin configurations that the specifications for each alternate mode
>>>>>> defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
>>>>>> work for sure.
>>>>>>
>>>>>> The connector states that are defined in USB Type-C specification (so
>>>>>> basically USB Operation and USB Safe State) can be generalized, but
>>>>>> those states just should not be defined in tcpm.h. We need to use
>>>>>> them in other drivers as well.
>>>>>>
>>>>>> I'm in the middle of preparing more complete support for alternate
>>>>>> modes. If you check the RFC [1] I send previously, in the first patch
>>>>>> of the series I'm adding documentation that should explain the
>>>>>> plan.
>>>>> Sorry, I forgot the link:
>>>>>
>>>>> [1] https://www.spinics.net/lists/linux-usb/msg166520.html
>>>> Oh, sorry I forgot about that post in the first place... I reread it now.
>>>>
>>>> Since the modal TYPEC_STATE_ are overlapping for each AM, this means that
>>>> the mux driver "set" must know which AM is active, right?
>>>>
>>>> And each mux driver also need to support all possible alt modes?
>>> There are two options for the mux drivers to link with the alternate
>>> modes. They can use the typec API where a single mux is linked to a
>>> single port. Alternatively they can use the notifiers from the
>>> alternate modes themselves, which allows a single mux to be liked to
>>> multiple alternate modes. Both methods will be available.
>>>
>>> Is this what you were asking?
>> Well, sort of... My angle is from the mux driver writers side. A hardware mux
>> device does not care what logical signals is being muxed and neither should the
>> mux driver I think. But the system designer must be able to make the mux driver
>> respond to the correct AM:STATE.
> So what you want is "generic" mux drivers?

What I really want is to understand where you're going with this.

I started to write a lot of answers to your statements below but then I
got to where you write that you are thinking about ditching the current
mux handling and that really changes a lot of things so I skip that.

>  The mux drivers are the
> ones that need to interpret the alternate mode specific connector
> states into actual pin muxing, not the type-c drivers. The type-c
> alternate mode drivers and frameworks should not even need to know if
> an alternate mode specific connector state requires pin muxing at all.
> They should be able quite simply pass forward the negotiated connector
> state, and be done with it. That is the only way we can keep this
> whole thing flexible enough to fit all kinds of platforms.
>
> Interpreting the alternate mode specific connector states is
> straightforward for the mux drivers, and that is all that they need to
> do in this case, but expecting the type-c drivers to supply the exact
> pin configuration to the mux drivers just so the mux driver can be a
> little bit more "dummy" does not only mean duplicated effort (the
> alternate mode drivers and the type-c frameworks still need to handle
> the actual alternate mode specific connector states), but also a
> roadmap to hacks like virtual/NOP mux drivers and platform specific
> quirks in the alternate mode drivers.
>
> The configuration of the type-c connector will simply not always
> happen the same way. We will not always have the muxes directly under
> operating system control. Sometimes the USB Type-C/PD controller
> handles them, or like in my case, a microcontroller on the system
> controls the muxes and the drivers need to communicate with the
> microcontroller. We don't know the requirements the various ways to
> configure the connectors have.
>
> So we don't want to end up in a scenario where we are uncertain about
> what does the underlying mux handling expect from the alternate mode
> driver (for example, do we need to inform about the Linux specific pin
> configuration value you are proposing, or the actual alternate mode
> specific connector state) but that is exactly what will happen.
>
>> So, do you think it would be a viable approach to let the mux driver be
>> configured through dt/acpi properties that link AM:STATE pairs to some internal
>> muxing mode of the hardware mux device?
> If the mux is able to route the pins to multiple locations on top of
> USB, the driver for it needs to get all the supported SVIDs, modes and
> their SVID specific connector states that require pin re-configuration
> from the hardware description. That should be enough for them.
>
>> 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?

> Btw. Could you please not use abbreviations like "AM". Please just say
> "alternate mode" when you want to talk about alternate modes.

I will try ;-)

> Thanks,
>
BR // Mats
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-07 13:39 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-07 13:39 UTC (permalink / raw)
  To: Mats Karrman
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

Hi Mats,

On Fri, May 04, 2018 at 06:57:31PM +0200, Mats Karrman wrote:
> 
> On 2018-05-04 16:56, Heikki Krogerus wrote:
> 
> > On Wed, May 02, 2018 at 03:13:44PM +0200, Mats Karrman wrote:
> > > Hi Heikki,
> > > 
> > > On 2018-05-02 10:25, Heikki Krogerus wrote:
> > > 
> > > > On Wed, May 02, 2018 at 11:23:35AM +0300, Heikki Krogerus wrote:
> > > > > Hi Mats,
> > > > > 
> > > > > On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
> > > > > > The current naming used for tcpc_mux_mode constants makes
> > > > > > too much assumptioms about the usage of the signals.
> > > > > > This patch replaces the names with generic names more closely
> > > > > > tied to the Type-C specifications and also adds some new ones.
> > > > > > At the same time TCPC_MUX_* defines are removed as they do not
> > > > > > fit the new concept and currently have no in-tree users.
> > > > > I'm afraid trying to generalize the modal connector states even like
> > > > > this is not going to work. We can't make any assumptions about how the
> > > > > alternate modes configure the pins, or the connector in general.
> > > > > 
> > > > > The only way this will work is that every alternate mode has its own
> > > > > configurations defined separately, and I'm talking about the actual
> > > > > pin configurations that the specifications for each alternate mode
> > > > > defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
> > > > > work for sure.
> > > > > 
> > > > > The connector states that are defined in USB Type-C specification (so
> > > > > basically USB Operation and USB Safe State) can be generalized, but
> > > > > those states just should not be defined in tcpm.h. We need to use
> > > > > them in other drivers as well.
> > > > > 
> > > > > I'm in the middle of preparing more complete support for alternate
> > > > > modes. If you check the RFC [1] I send previously, in the first patch
> > > > > of the series I'm adding documentation that should explain the
> > > > > plan.
> > > > Sorry, I forgot the link:
> > > > 
> > > > [1] https://www.spinics.net/lists/linux-usb/msg166520.html
> > > Oh, sorry I forgot about that post in the first place... I reread it now.
> > > 
> > > Since the modal TYPEC_STATE_ are overlapping for each AM, this means that
> > > the mux driver "set" must know which AM is active, right?
> > > 
> > > And each mux driver also need to support all possible alt modes?
> > There are two options for the mux drivers to link with the alternate
> > modes. They can use the typec API where a single mux is linked to a
> > single port. Alternatively they can use the notifiers from the
> > alternate modes themselves, which allows a single mux to be liked to
> > multiple alternate modes. Both methods will be available.
> > 
> > Is this what you were asking?
> 
> Well, sort of... My angle is from the mux driver writers side. A hardware mux
> device does not care what logical signals is being muxed and neither should the
> mux driver I think. But the system designer must be able to make the mux driver
> respond to the correct AM:STATE.

So what you want is "generic" mux drivers? The mux drivers are the
ones that need to interpret the alternate mode specific connector
states into actual pin muxing, not the type-c drivers. The type-c
alternate mode drivers and frameworks should not even need to know if
an alternate mode specific connector state requires pin muxing at all.
They should be able quite simply pass forward the negotiated connector
state, and be done with it. That is the only way we can keep this
whole thing flexible enough to fit all kinds of platforms.

Interpreting the alternate mode specific connector states is
straightforward for the mux drivers, and that is all that they need to
do in this case, but expecting the type-c drivers to supply the exact
pin configuration to the mux drivers just so the mux driver can be a
little bit more "dummy" does not only mean duplicated effort (the
alternate mode drivers and the type-c frameworks still need to handle
the actual alternate mode specific connector states), but also a
roadmap to hacks like virtual/NOP mux drivers and platform specific
quirks in the alternate mode drivers.

The configuration of the type-c connector will simply not always
happen the same way. We will not always have the muxes directly under
operating system control. Sometimes the USB Type-C/PD controller
handles them, or like in my case, a microcontroller on the system
controls the muxes and the drivers need to communicate with the
microcontroller. We don't know the requirements the various ways to
configure the connectors have.

So we don't want to end up in a scenario where we are uncertain about
what does the underlying mux handling expect from the alternate mode
driver (for example, do we need to inform about the Linux specific pin
configuration value you are proposing, or the actual alternate mode
specific connector state) but that is exactly what will happen.

> So, do you think it would be a viable approach to let the mux driver be
> configured through dt/acpi properties that link AM:STATE pairs to some internal
> muxing mode of the hardware mux device?

If the mux is able to route the pins to multiple locations on top of
USB, the driver for it needs to get all the supported SVIDs, modes and
their SVID specific connector states that require pin re-configuration
from the hardware description. That should be enough for them.

> 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.

Btw. Could you please not use abbreviations like "AM". Please just say
"alternate mode" when you want to talk about alternate modes.


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-04 16:57 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-04 16:57 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

On 2018-05-04 16:56, Heikki Krogerus wrote:

> On Wed, May 02, 2018 at 03:13:44PM +0200, Mats Karrman wrote:
>> Hi Heikki,
>>
>> On 2018-05-02 10:25, Heikki Krogerus wrote:
>>
>>> On Wed, May 02, 2018 at 11:23:35AM +0300, Heikki Krogerus wrote:
>>>> Hi Mats,
>>>>
>>>> On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
>>>>> The current naming used for tcpc_mux_mode constants makes
>>>>> too much assumptioms about the usage of the signals.
>>>>> This patch replaces the names with generic names more closely
>>>>> tied to the Type-C specifications and also adds some new ones.
>>>>> At the same time TCPC_MUX_* defines are removed as they do not
>>>>> fit the new concept and currently have no in-tree users.
>>>> I'm afraid trying to generalize the modal connector states even like
>>>> this is not going to work. We can't make any assumptions about how the
>>>> alternate modes configure the pins, or the connector in general.
>>>>
>>>> The only way this will work is that every alternate mode has its own
>>>> configurations defined separately, and I'm talking about the actual
>>>> pin configurations that the specifications for each alternate mode
>>>> defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
>>>> work for sure.
>>>>
>>>> The connector states that are defined in USB Type-C specification (so
>>>> basically USB Operation and USB Safe State) can be generalized, but
>>>> those states just should not be defined in tcpm.h. We need to use
>>>> them in other drivers as well.
>>>>
>>>> I'm in the middle of preparing more complete support for alternate
>>>> modes. If you check the RFC [1] I send previously, in the first patch
>>>> of the series I'm adding documentation that should explain the
>>>> plan.
>>> Sorry, I forgot the link:
>>>
>>> [1] https://www.spinics.net/lists/linux-usb/msg166520.html
>> Oh, sorry I forgot about that post in the first place... I reread it now.
>>
>> Since the modal TYPEC_STATE_ are overlapping for each AM, this means that
>> the mux driver "set" must know which AM is active, right?
>>
>> And each mux driver also need to support all possible alt modes?
> There are two options for the mux drivers to link with the alternate
> modes. They can use the typec API where a single mux is linked to a
> single port. Alternatively they can use the notifiers from the
> alternate modes themselves, which allows a single mux to be liked to
> multiple alternate modes. Both methods will be available.
>
> Is this what you were asking?

Well, sort of... My angle is from the mux driver writers side. A hardware mux
device does not care what logical signals is being muxed and neither should the
mux driver I think. But the system designer must be able to make the mux driver
respond to the correct AM:STATE.
So, do you think it would be a viable approach to let the mux driver be
configured through dt/acpi properties that link AM:STATE pairs to some internal
muxing mode of the hardware mux device?
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?

>> I was thinking that it must be a finite number of possible routes
>> between the local connections of a mux and the four available SS lanes of
>> the cable but of course there is no theoretical limit to the number of
>> local connections...
>>
>> Do we want to set a limitation of one mux device per port? I guess we
>> could and then let people write "composite" mux drivers if it should ever
>> be necessary.
> I think I answered to this one above.
>
>> Still it's difficult to write a mux device driver that support everything,
>> but I'm thinking that it might be possible to write a "mode agnostic" mux
>> driver that uses properties to match the "AM:STATE" pairs the board needs
>> to support to the hardware mux device specific muxing modes available?
>>
>> It would be very interesting to see a devicetree example on how you
>> picture things being connected to each other.
>>
>> Btw. You're using "mode" and "state" interchangeably which is a bit
>> confusing. Could you settle for one?
> OK, I'll try to be more consistent and pick one.
>
>
> Thanks,
>
>
BR // Mats
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-04 14:56 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-04 14:56 UTC (permalink / raw)
  To: Mats Karrman
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

On Wed, May 02, 2018 at 03:13:44PM +0200, Mats Karrman wrote:
> Hi Heikki,
> 
> On 2018-05-02 10:25, Heikki Krogerus wrote:
> 
> > On Wed, May 02, 2018 at 11:23:35AM +0300, Heikki Krogerus wrote:
> > > Hi Mats,
> > > 
> > > On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
> > > > The current naming used for tcpc_mux_mode constants makes
> > > > too much assumptioms about the usage of the signals.
> > > > This patch replaces the names with generic names more closely
> > > > tied to the Type-C specifications and also adds some new ones.
> > > > At the same time TCPC_MUX_* defines are removed as they do not
> > > > fit the new concept and currently have no in-tree users.
> > > I'm afraid trying to generalize the modal connector states even like
> > > this is not going to work. We can't make any assumptions about how the
> > > alternate modes configure the pins, or the connector in general.
> > > 
> > > The only way this will work is that every alternate mode has its own
> > > configurations defined separately, and I'm talking about the actual
> > > pin configurations that the specifications for each alternate mode
> > > defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
> > > work for sure.
> > > 
> > > The connector states that are defined in USB Type-C specification (so
> > > basically USB Operation and USB Safe State) can be generalized, but
> > > those states just should not be defined in tcpm.h. We need to use
> > > them in other drivers as well.
> > > 
> > > I'm in the middle of preparing more complete support for alternate
> > > modes. If you check the RFC [1] I send previously, in the first patch
> > > of the series I'm adding documentation that should explain the
> > > plan.
> > Sorry, I forgot the link:
> > 
> > [1] https://www.spinics.net/lists/linux-usb/msg166520.html
> 
> Oh, sorry I forgot about that post in the first place... I reread it now.
> 
> Since the modal TYPEC_STATE_ are overlapping for each AM, this means that
> the mux driver "set" must know which AM is active, right?
> 
> And each mux driver also need to support all possible alt modes?

There are two options for the mux drivers to link with the alternate
modes. They can use the typec API where a single mux is linked to a
single port. Alternatively they can use the notifiers from the
alternate modes themselves, which allows a single mux to be liked to
multiple alternate modes. Both methods will be available.

Is this what you were asking?

> I was thinking that it must be a finite number of possible routes
> between the local connections of a mux and the four available SS lanes of
> the cable but of course there is no theoretical limit to the number of
> local connections...
> 
> Do we want to set a limitation of one mux device per port? I guess we
> could and then let people write "composite" mux drivers if it should ever
> be necessary.

I think I answered to this one above.

> Still it's difficult to write a mux device driver that support everything,
> but I'm thinking that it might be possible to write a "mode agnostic" mux
> driver that uses properties to match the "AM:STATE" pairs the board needs
> to support to the hardware mux device specific muxing modes available?
> 
> It would be very interesting to see a devicetree example on how you
> picture things being connected to each other.
> 
> Btw. You're using "mode" and "state" interchangeably which is a bit
> confusing. Could you settle for one?

OK, I'll try to be more consistent and pick one.


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-02 13:13 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-02 13:13 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

Hi Heikki,

On 2018-05-02 10:25, Heikki Krogerus wrote:

> On Wed, May 02, 2018 at 11:23:35AM +0300, Heikki Krogerus wrote:
>> Hi Mats,
>>
>> On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
>>> The current naming used for tcpc_mux_mode constants makes
>>> too much assumptioms about the usage of the signals.
>>> This patch replaces the names with generic names more closely
>>> tied to the Type-C specifications and also adds some new ones.
>>> At the same time TCPC_MUX_* defines are removed as they do not
>>> fit the new concept and currently have no in-tree users.
>> I'm afraid trying to generalize the modal connector states even like
>> this is not going to work. We can't make any assumptions about how the
>> alternate modes configure the pins, or the connector in general.
>>
>> The only way this will work is that every alternate mode has its own
>> configurations defined separately, and I'm talking about the actual
>> pin configurations that the specifications for each alternate mode
>> defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
>> work for sure.
>>
>> The connector states that are defined in USB Type-C specification (so
>> basically USB Operation and USB Safe State) can be generalized, but
>> those states just should not be defined in tcpm.h. We need to use
>> them in other drivers as well.
>>
>> I'm in the middle of preparing more complete support for alternate
>> modes. If you check the RFC [1] I send previously, in the first patch
>> of the series I'm adding documentation that should explain the
>> plan.
> Sorry, I forgot the link:
>
> [1] https://www.spinics.net/lists/linux-usb/msg166520.html

Oh, sorry I forgot about that post in the first place... I reread it now.

Since the modal TYPEC_STATE_ are overlapping for each AM, this means that
the mux driver "set" must know which AM is active, right?

And each mux driver also need to support all possible alt modes?

I was thinking that it must be a finite number of possible routes
between the local connections of a mux and the four available SS lanes of
the cable but of course there is no theoretical limit to the number of
local connections...

Do we want to set a limitation of one mux device per port? I guess we
could and then let people write "composite" mux drivers if it should ever
be necessary.

Still it's difficult to write a mux device driver that support everything,
but I'm thinking that it might be possible to write a "mode agnostic" mux
driver that uses properties to match the "AM:STATE" pairs the board needs
to support to the hardware mux device specific muxing modes available?

It would be very interesting to see a devicetree example on how you
picture things being connected to each other.

Btw. You're using "mode" and "state" interchangeably which is a bit
confusing. Could you settle for one?

BR // Mats

> Cheers,
>
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-02  8:25 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-02  8:25 UTC (permalink / raw)
  To: Mats Karrman
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

On Wed, May 02, 2018 at 11:23:35AM +0300, Heikki Krogerus wrote:
> Hi Mats,
> 
> On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
> > The current naming used for tcpc_mux_mode constants makes
> > too much assumptioms about the usage of the signals.
> > This patch replaces the names with generic names more closely
> > tied to the Type-C specifications and also adds some new ones.
> > At the same time TCPC_MUX_* defines are removed as they do not
> > fit the new concept and currently have no in-tree users.
> 
> I'm afraid trying to generalize the modal connector states even like
> this is not going to work. We can't make any assumptions about how the
> alternate modes configure the pins, or the connector in general.
> 
> The only way this will work is that every alternate mode has its own
> configurations defined separately, and I'm talking about the actual
> pin configurations that the specifications for each alternate mode
> defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
> work for sure.
> 
> The connector states that are defined in USB Type-C specification (so
> basically USB Operation and USB Safe State) can be generalized, but
> those states just should not be defined in tcpm.h. We need to use
> them in other drivers as well.
> 
> I'm in the middle of preparing more complete support for alternate
> modes. If you check the RFC [1] I send previously, in the first patch
> of the series I'm adding documentation that should explain the
> plan.

Sorry, I forgot the link:

[1] https://www.spinics.net/lists/linux-usb/msg166520.html


Cheers,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-02  8:23 Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2018-05-02  8:23 UTC (permalink / raw)
  To: Mats Karrman
  Cc: Hans de Goede, Guenter Roeck, Greg Kroah-Hartman, Rob Herring, linux-usb

Hi Mats,

On Wed, May 02, 2018 at 12:21:07AM +0200, Mats Karrman wrote:
> The current naming used for tcpc_mux_mode constants makes
> too much assumptioms about the usage of the signals.
> This patch replaces the names with generic names more closely
> tied to the Type-C specifications and also adds some new ones.
> At the same time TCPC_MUX_* defines are removed as they do not
> fit the new concept and currently have no in-tree users.

I'm afraid trying to generalize the modal connector states even like
this is not going to work. We can't make any assumptions about how the
alternate modes configure the pins, or the connector in general.

The only way this will work is that every alternate mode has its own
configurations defined separately, and I'm talking about the actual
pin configurations that the specifications for each alternate mode
defines, so something like TYPEC_MUX_DP and TYPEC_MUX_DOCK will not
work for sure.

The connector states that are defined in USB Type-C specification (so
basically USB Operation and USB Safe State) can be generalized, but
those states just should not be defined in tcpm.h. We need to use
them in other drivers as well.

I'm in the middle of preparing more complete support for alternate
modes. If you check the RFC [1] I send previously, in the first patch
of the series I'm adding documentation that should explain the
plan.

> Signed-off-by: Mats Karrman <mats.dev.list@gmail.com>
> ---
>  drivers/usb/typec/mux/pi3usb30532.c |  7 ++++---
>  drivers/usb/typec/tcpm.c            |  2 +-
>  include/linux/usb/tcpm.h            | 21 ++++++++++-----------
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/mux/pi3usb30532.c b/drivers/usb/typec/mux/pi3usb30532.c
> index b0e88db..279f3c3 100644
> --- a/drivers/usb/typec/mux/pi3usb30532.c
> +++ b/drivers/usb/typec/mux/pi3usb30532.c
> @@ -83,18 +83,19 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int state)
>  	new_conf = pi->conf;
>  
>  	switch (state) {
> +	default:
>  	case TYPEC_MUX_NONE:
>  		new_conf = PI3USB30532_CONF_OPEN;
>  		break;
> -	case TYPEC_MUX_USB:
> +	case TYPEC_MUX_2CH_USBSS:
>  		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
>  			   PI3USB30532_CONF_USB3;
>  		break;
> -	case TYPEC_MUX_DP:
> +	case TYPEC_MUX_4CH_AM:
>  		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
>  			   PI3USB30532_CONF_4LANE_DP;
>  		break;
> -	case TYPEC_MUX_DOCK:
> +	case TYPEC_MUX_2CH_USBSS_2CH_AM:
>  		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
>  			   PI3USB30532_CONF_USB3_AND_2LANE_DP;
>  		break;
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 7ee417a..0451ea0 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -790,7 +790,7 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
>  	else
>  		usb_role = USB_ROLE_DEVICE;
>  
> -	ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role, orientation);
> +	ret = tcpm_mux_set(port, TYPEC_MUX_2CH_USBSS, usb_role, orientation);
>  	if (ret < 0)
>  		return ret;
>  
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index b231b93..3518965 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -93,20 +93,19 @@ struct tcpc_config {
>  	const struct typec_altmode_desc *alt_modes;
>  };
>  
> -/* Mux state attributes */
> -#define TCPC_MUX_USB_ENABLED		BIT(0)	/* USB enabled */
> -#define TCPC_MUX_DP_ENABLED		BIT(1)	/* DP enabled */
> -#define TCPC_MUX_POLARITY_INVERTED	BIT(2)	/* Polarity inverted */
> -
> -/* Mux modes, decoded to attributes */
> +/* Mux modes */
>  enum tcpc_mux_mode {
> -	TYPEC_MUX_NONE	= 0,				/* Open switch */
> -	TYPEC_MUX_USB	= TCPC_MUX_USB_ENABLED,		/* USB only */
> -	TYPEC_MUX_DP	= TCPC_MUX_DP_ENABLED,		/* DP only */
> -	TYPEC_MUX_DOCK	= TCPC_MUX_USB_ENABLED |	/* Both USB and DP */
> -			  TCPC_MUX_DP_ENABLED,
> +	TYPEC_MUX_NONE,				/* Open switch */
> +	TYPEC_MUX_2CH_USBSS,			/* 2ch USB SS */
> +	TYPEC_MUX_4CH_AM,			/* 4ch Alt Mode */
> +	TYPEC_MUX_2CH_USBSS_2CH_AM,		/* 2ch USB SS + 2ch Alt Mode */
> +
> +	// Example of additional modes that may be needed in future:
> +	TYPEC_MUX_4CH_USBSS,			/* 4ch USB SS */
> +	TYPEC_MUX_2CH_USBSS_2CH_AM_B,		/* 2ch USB SS + 2ch Alt Mode (e.g. DP GPU2) */
>  };
>  
> +
>  /**
>   * struct tcpc_dev - Port configuration and callback functions
>   * @config:	Pointer to port configuration


Thanks,

^ permalink raw reply	[flat|nested] 19+ messages in thread
* [RFC,1/7] usb: typec: Generalize mux mode names
@ 2018-05-01 22:21 Mats Karrman
  0 siblings, 0 replies; 19+ messages in thread
From: Mats Karrman @ 2018-05-01 22:21 UTC (permalink / raw)
  To: Heikki Krogerus, Hans de Goede, Guenter Roeck, Greg Kroah-Hartman
  Cc: Rob Herring, linux-usb

The current naming used for tcpc_mux_mode constants makes
too much assumptioms about the usage of the signals.
This patch replaces the names with generic names more closely
tied to the Type-C specifications and also adds some new ones.
At the same time TCPC_MUX_* defines are removed as they do not
fit the new concept and currently have no in-tree users.

Signed-off-by: Mats Karrman <mats.dev.list@gmail.com>
---
 drivers/usb/typec/mux/pi3usb30532.c |  7 ++++---
 drivers/usb/typec/tcpm.c            |  2 +-
 include/linux/usb/tcpm.h            | 21 ++++++++++-----------
 3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/typec/mux/pi3usb30532.c b/drivers/usb/typec/mux/pi3usb30532.c
index b0e88db..279f3c3 100644
--- a/drivers/usb/typec/mux/pi3usb30532.c
+++ b/drivers/usb/typec/mux/pi3usb30532.c
@@ -83,18 +83,19 @@ static int pi3usb30532_mux_set(struct typec_mux *mux, int state)
 	new_conf = pi->conf;
 
 	switch (state) {
+	default:
 	case TYPEC_MUX_NONE:
 		new_conf = PI3USB30532_CONF_OPEN;
 		break;
-	case TYPEC_MUX_USB:
+	case TYPEC_MUX_2CH_USBSS:
 		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
 			   PI3USB30532_CONF_USB3;
 		break;
-	case TYPEC_MUX_DP:
+	case TYPEC_MUX_4CH_AM:
 		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
 			   PI3USB30532_CONF_4LANE_DP;
 		break;
-	case TYPEC_MUX_DOCK:
+	case TYPEC_MUX_2CH_USBSS_2CH_AM:
 		new_conf = (new_conf & PI3USB30532_CONF_SWAP) |
 			   PI3USB30532_CONF_USB3_AND_2LANE_DP;
 		break;
diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
index 7ee417a..0451ea0 100644
--- a/drivers/usb/typec/tcpm.c
+++ b/drivers/usb/typec/tcpm.c
@@ -790,7 +790,7 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
 	else
 		usb_role = USB_ROLE_DEVICE;
 
-	ret = tcpm_mux_set(port, TYPEC_MUX_USB, usb_role, orientation);
+	ret = tcpm_mux_set(port, TYPEC_MUX_2CH_USBSS, usb_role, orientation);
 	if (ret < 0)
 		return ret;
 
diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
index b231b93..3518965 100644
--- a/include/linux/usb/tcpm.h
+++ b/include/linux/usb/tcpm.h
@@ -93,20 +93,19 @@ struct tcpc_config {
 	const struct typec_altmode_desc *alt_modes;
 };
 
-/* Mux state attributes */
-#define TCPC_MUX_USB_ENABLED		BIT(0)	/* USB enabled */
-#define TCPC_MUX_DP_ENABLED		BIT(1)	/* DP enabled */
-#define TCPC_MUX_POLARITY_INVERTED	BIT(2)	/* Polarity inverted */
-
-/* Mux modes, decoded to attributes */
+/* Mux modes */
 enum tcpc_mux_mode {
-	TYPEC_MUX_NONE	= 0,				/* Open switch */
-	TYPEC_MUX_USB	= TCPC_MUX_USB_ENABLED,		/* USB only */
-	TYPEC_MUX_DP	= TCPC_MUX_DP_ENABLED,		/* DP only */
-	TYPEC_MUX_DOCK	= TCPC_MUX_USB_ENABLED |	/* Both USB and DP */
-			  TCPC_MUX_DP_ENABLED,
+	TYPEC_MUX_NONE,				/* Open switch */
+	TYPEC_MUX_2CH_USBSS,			/* 2ch USB SS */
+	TYPEC_MUX_4CH_AM,			/* 4ch Alt Mode */
+	TYPEC_MUX_2CH_USBSS_2CH_AM,		/* 2ch USB SS + 2ch Alt Mode */
+
+	// Example of additional modes that may be needed in future:
+	TYPEC_MUX_4CH_USBSS,			/* 4ch USB SS */
+	TYPEC_MUX_2CH_USBSS_2CH_AM_B,		/* 2ch USB SS + 2ch Alt Mode (e.g. DP GPU2) */
 };
 
+
 /**
  * struct tcpc_dev - Port configuration and callback functions
  * @config:	Pointer to port configuration

^ permalink raw reply related	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-05-21 13:04 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-08 14:25 [RFC,1/7] usb: typec: Generalize mux mode names Heikki Krogerus
  -- strict thread matches above, loose matches on Subject: below --
2018-05-21 13:04 Heikki Krogerus
2018-05-18  5:26 Mats Karrman
2018-05-17 11:50 Heikki Krogerus
2018-05-16 21:11 Mats Karrman
2018-05-16 11:43 Heikki Krogerus
2018-05-15 21:24 Mats Karrman
2018-05-15  7:30 Heikki Krogerus
2018-05-11 19:28 Mats Karrman
2018-05-09 12:49 Heikki Krogerus
2018-05-08 19:10 Mats Karrman
2018-05-07 21:19 Mats Karrman
2018-05-07 13:39 Heikki Krogerus
2018-05-04 16:57 Mats Karrman
2018-05-04 14:56 Heikki Krogerus
2018-05-02 13:13 Mats Karrman
2018-05-02  8:25 Heikki Krogerus
2018-05-02  8:23 Heikki Krogerus
2018-05-01 22:21 Mats Karrman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.