| Message ID | 20170905164221.11266-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
On 2017-09-05 18:42, Hans de Goede wrote: > Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by > USB device/host, resp. Type-C polarity/role/altmode mux drivers and > consumers to ensure that they agree on the meaning of the > mux_control_select() state argument. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Changes in v2: > -Start numbering of defines at 0 not 1 > -Use a new usb.h header, rather then adding these to consumer.h > -Add separate MUX_USB_* and MUX_TYPEC_* defines > --- > include/linux/mux/usb.h | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > create mode 100644 include/linux/mux/usb.h > > diff --git a/include/linux/mux/usb.h b/include/linux/mux/usb.h > new file mode 100644 > index 000000000000..44df5eca5256 > --- /dev/null > +++ b/include/linux/mux/usb.h > @@ -0,0 +1,32 @@ > +/* > + * mux/usb.h - definitions for USB multiplexers > + * > + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > +#ifndef _LINUX_MUX_USB_H > +#define _LINUX_MUX_USB_H > + > +/* Mux state values for USB device/host role muxes */ > +#define MUX_USB_DEVICE (0) /* USB device mode */ > +#define MUX_USB_HOST (1) /* USB host mode */ > +#define MUX_USB_STATES (2) > + > +/* > + * Mux state values for Type-C polarity/role/altmode muxes. > + * > + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as > + * inverted-polarity (Type-C plugged in upside down) can happen with any > + * other mux-state. > + */ > +#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ > +#define MUX_TYPEC_DEVICE (0 << 1) /* USB device mode */ > +#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */ > +#define MUX_TYPEC_HOST_AND_DP_SRC (2 << 1) /* USB host + 2 lanes DP src */ > +#define MUX_TYPEC_DP_SRC (3 << 1) /* 4 lanes Display Port src */ > +#define MUX_TYPEC_STATES (4 << 1) But USB Type-C muxes need not support just these states If I read it right? USB Type-C seems to be usable for a variety of protocols and the above list seems pretty much like a special case for this mux (and perhaps a set of other similar muxes). But when someone with a USB Type-C mux for different protocols shows up, that person will probably be frustrated by these defines, no? Or is there something I don't see that limits USB-C to DP? Cheers, Peter > + > +#endif /* _LINUX_MUX_TYPEC_H */ >
Hi, On 08-09-17 17:47, Peter Rosin wrote: > On 2017-09-05 18:42, Hans de Goede wrote: >> Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by >> USB device/host, resp. Type-C polarity/role/altmode mux drivers and >> consumers to ensure that they agree on the meaning of the >> mux_control_select() state argument. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Changes in v2: >> -Start numbering of defines at 0 not 1 >> -Use a new usb.h header, rather then adding these to consumer.h >> -Add separate MUX_USB_* and MUX_TYPEC_* defines >> --- >> include/linux/mux/usb.h | 32 ++++++++++++++++++++++++++++++++ >> 1 file changed, 32 insertions(+) >> create mode 100644 include/linux/mux/usb.h >> >> diff --git a/include/linux/mux/usb.h b/include/linux/mux/usb.h >> new file mode 100644 >> index 000000000000..44df5eca5256 >> --- /dev/null >> +++ b/include/linux/mux/usb.h >> @@ -0,0 +1,32 @@ >> +/* >> + * mux/usb.h - definitions for USB multiplexers >> + * >> + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> +#ifndef _LINUX_MUX_USB_H >> +#define _LINUX_MUX_USB_H >> + >> +/* Mux state values for USB device/host role muxes */ >> +#define MUX_USB_DEVICE (0) /* USB device mode */ >> +#define MUX_USB_HOST (1) /* USB host mode */ >> +#define MUX_USB_STATES (2) >> + >> +/* >> + * Mux state values for Type-C polarity/role/altmode muxes. >> + * >> + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as >> + * inverted-polarity (Type-C plugged in upside down) can happen with any >> + * other mux-state. >> + */ >> +#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ >> +#define MUX_TYPEC_DEVICE (0 << 1) /* USB device mode */ >> +#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */ >> +#define MUX_TYPEC_HOST_AND_DP_SRC (2 << 1) /* USB host + 2 lanes DP src */ >> +#define MUX_TYPEC_DP_SRC (3 << 1) /* 4 lanes Display Port src */ >> +#define MUX_TYPEC_STATES (4 << 1) > > But USB Type-C muxes need not support just these states If I read it right? > USB Type-C seems to be usable for a variety of protocols and the above list > seems pretty much like a special case for this mux (and perhaps a set of > other similar muxes). But when someone with a USB Type-C mux for different > protocols shows up, that person will probably be frustrated by these > defines, no? Or is there something I don't see that limits USB-C to DP? In general almost all hardware is limited to the above (+ analog audio over the 2 Sideband use pins, but I expect that to have a separate mux). You're right, theoretically there might be other cases, e.g. there is a spec for HDMI over Type-C (wishful thinking from the HDMI group, no one uses this), but: 1) I expect most muxes to implement the above set, that is what all hardware out there supports (well that or less). 2) We can always add extra defines here, that means that a Type-C mux may not implement all states and return -EINVAL when asked for something it does not implement, which I understand is a bit weird from a mux subsys pov. But that can be the case anyways because even though the mux supports these options, the board it is used on does no necessarily have to support these options, e.g. there may be only 2 lanes of DP hooked up to the mux (or no DP at all, but then I would them to expect a different mux). So the Type-C Port Manager already needs to be passed some platform data describing which features the board has and keep that in mind when negotiation with the dongle attached to the Type-C port, so if we do get boards which do HDMI and no DP, then the TCPM would simply never use the MUX_TYPEC_HOST_AND_DP_SRC and MUX_TYPEC_DP_SRC states. Regards, Hans
On 2017-09-08 19:07, Hans de Goede wrote: > Hi, > > On 08-09-17 17:47, Peter Rosin wrote: >> On 2017-09-05 18:42, Hans de Goede wrote: >>> Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by >>> USB device/host, resp. Type-C polarity/role/altmode mux drivers and >>> consumers to ensure that they agree on the meaning of the >>> mux_control_select() state argument. >>> >>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>> --- *snip* >>> +/* >>> + * Mux state values for Type-C polarity/role/altmode muxes. >>> + * >>> + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as >>> + * inverted-polarity (Type-C plugged in upside down) can happen with any >>> + * other mux-state. >>> + */ >>> +#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ >>> +#define MUX_TYPEC_DEVICE (0 << 1) /* USB device mode */ >>> +#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */ >>> +#define MUX_TYPEC_HOST_AND_DP_SRC (2 << 1) /* USB host + 2 lanes DP src */ >>> +#define MUX_TYPEC_DP_SRC (3 << 1) /* 4 lanes Display Port src */ >>> +#define MUX_TYPEC_STATES (4 << 1) >> >> But USB Type-C muxes need not support just these states If I read it right? >> USB Type-C seems to be usable for a variety of protocols and the above list >> seems pretty much like a special case for this mux (and perhaps a set of >> other similar muxes). But when someone with a USB Type-C mux for different >> protocols shows up, that person will probably be frustrated by these >> defines, no? Or is there something I don't see that limits USB-C to DP? > > In general almost all hardware is limited to the above (+ analog audio over > the 2 Sideband use pins, but I expect that to have a separate mux). > > You're right, theoretically there might be other cases, e.g. there is a spec > for HDMI over Type-C (wishful thinking from the HDMI group, no one uses this), > but: > > 1) I expect most muxes to implement the above set, that is what all > hardware out there supports (well that or less). > > 2) We can always add extra defines here, that means that a Type-C mux may > not implement all states and return -EINVAL when asked for something it > does not implement, which I understand is a bit weird from a mux subsys > pov. But that can be the case anyways because even though the mux supports > these options, the board it is used on does no necessarily have to support > these options, e.g. there may be only 2 lanes of DP hooked up to the mux > (or no DP at all, but then I would them to expect a different mux). > > So the Type-C Port Manager already needs to be passed some platform > data describing which features the board has and keep that in mind > when negotiation with the dongle attached to the Type-C port, so if > we do get boards which do HDMI and no DP, then the TCPM would simply > never use the MUX_TYPEC_HOST_AND_DP_SRC and MUX_TYPEC_DP_SRC states. Ok, I googled "usb type c mux" and came up with HD3SS460 from Texas as the first hit. http://www.ti.com/lit/ds/symlink/hd3ss460.pdf That one has three control pins, but two of them (AMSEL and EN) are tri-state. So 18 states in theory. However, if EN is low everything is HighZ, so that collapses 6 states into 1, and 2 other states are reserved. Still 11 states, which is two more than what you have implemented for PI3USB30532. If we ignore polarity switching, it's only a one state diff. However, when I try to make sense of the states for the HD3SS460, I don't see anything that selects USB device or host. And I don't really see why a Type C mux has to know that; in my head the mux should just route the signals. And then when I look in your PI3USB30532 driver I don't seen any such difference either. Along the same lines, the Type C mux does not know/care if DP is source or sink. Or? How about: #define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ #define MUX_TYPEC_USB (0 << 1) /* USB only mode */ #define MUX_TYPEC_USB_AND_DP (1 << 1) /* USB host + 2 lanes DP */ #define MUX_TYPEC_DP (2 << 1) /* 4 lanes Display Port */ #define MUX_TYPEC_STATES (3 << 1) I'm not sure what 2 states the HS3SS460 have in addition to the above, but the way I read the spec those to are variations on the MUX_TYPEC_USB_AND_DP state, but routing the DP signals to alternate pins. Which suggests that more documentation is needed to describe exactly what is meant when someone selects MUX_TYPEC_USB_AND_DP? Cheers, Peter
Hi, On 10-09-17 23:36, Peter Rosin wrote: > On 2017-09-08 19:07, Hans de Goede wrote: >> Hi, >> >> On 08-09-17 17:47, Peter Rosin wrote: >>> On 2017-09-05 18:42, Hans de Goede wrote: >>>> Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by >>>> USB device/host, resp. Type-C polarity/role/altmode mux drivers and >>>> consumers to ensure that they agree on the meaning of the >>>> mux_control_select() state argument. >>>> >>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >>>> --- > > *snip* > >>>> +/* >>>> + * Mux state values for Type-C polarity/role/altmode muxes. >>>> + * >>>> + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as >>>> + * inverted-polarity (Type-C plugged in upside down) can happen with any >>>> + * other mux-state. >>>> + */ >>>> +#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ >>>> +#define MUX_TYPEC_DEVICE (0 << 1) /* USB device mode */ >>>> +#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */ >>>> +#define MUX_TYPEC_HOST_AND_DP_SRC (2 << 1) /* USB host + 2 lanes DP src */ >>>> +#define MUX_TYPEC_DP_SRC (3 << 1) /* 4 lanes Display Port src */ >>>> +#define MUX_TYPEC_STATES (4 << 1) >>> >>> But USB Type-C muxes need not support just these states If I read it right? >>> USB Type-C seems to be usable for a variety of protocols and the above list >>> seems pretty much like a special case for this mux (and perhaps a set of >>> other similar muxes). But when someone with a USB Type-C mux for different >>> protocols shows up, that person will probably be frustrated by these >>> defines, no? Or is there something I don't see that limits USB-C to DP? >> >> In general almost all hardware is limited to the above (+ analog audio over >> the 2 Sideband use pins, but I expect that to have a separate mux). >> >> You're right, theoretically there might be other cases, e.g. there is a spec >> for HDMI over Type-C (wishful thinking from the HDMI group, no one uses this), >> but: >> >> 1) I expect most muxes to implement the above set, that is what all >> hardware out there supports (well that or less). >> >> 2) We can always add extra defines here, that means that a Type-C mux may >> not implement all states and return -EINVAL when asked for something it >> does not implement, which I understand is a bit weird from a mux subsys >> pov. But that can be the case anyways because even though the mux supports >> these options, the board it is used on does no necessarily have to support >> these options, e.g. there may be only 2 lanes of DP hooked up to the mux >> (or no DP at all, but then I would them to expect a different mux). >> >> So the Type-C Port Manager already needs to be passed some platform >> data describing which features the board has and keep that in mind >> when negotiation with the dongle attached to the Type-C port, so if >> we do get boards which do HDMI and no DP, then the TCPM would simply >> never use the MUX_TYPEC_HOST_AND_DP_SRC and MUX_TYPEC_DP_SRC states. > > Ok, I googled "usb type c mux" and came up with HD3SS460 from Texas as > the first hit. > > http://www.ti.com/lit/ds/symlink/hd3ss460.pdf > > That one has three control pins, but two of them (AMSEL and EN) are > tri-state. So 18 states in theory. However, if EN is low everything is > HighZ, so that collapses 6 states into 1, and 2 other states are reserved. > Still 11 states, which is two more than what you have implemented for > PI3USB30532. If we ignore polarity switching, it's only a one state diff. > However, when I try to make sense of the states for the HD3SS460, I don't > see anything that selects USB device or host. And I don't really see why > a Type C mux has to know that; in my head the mux should just route the > signals. And then when I look in your PI3USB30532 driver I don't seen any > such difference either. Along the same lines, the Type C mux does not > know/care if DP is source or sink. Or? > > How about: > > #define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ > #define MUX_TYPEC_USB (0 << 1) /* USB only mode */ > #define MUX_TYPEC_USB_AND_DP (1 << 1) /* USB host + 2 lanes DP */ > #define MUX_TYPEC_DP (2 << 1) /* 4 lanes Display Port */ > #define MUX_TYPEC_STATES (3 << 1) > Sure that works for me, I will switch over to this for v3 of the patch-set. One note though, compared to my list, this changes DEVICE / HOST to just a single _USB entry. That works fine for my purpose, but typically USB host and device controllers are 2 separate blocks with a mux in between them. Now most current hardware have that mux in the SoC and then an external mux to mux the USB3 lines and optionally also DP lines to the Type-C connector and the above table does is correct (as the Type-C mux only has 1 USB state not separate host / device states as my proposal was). But my reason for having separate DEVICE / HOST states (and treating those identical in the pi3usb30532 driver) was to future proof things a bit. > I'm not sure what 2 states the HS3SS460 have in addition to the above, but > the way I read the spec those to are variations on the MUX_TYPEC_USB_AND_DP > state, but routing the DP signals to alternate pins. Which suggests that > more documentation is needed to describe exactly what is meant when someone > selects MUX_TYPEC_USB_AND_DP? The DP over Type-C spec unfortunately is not open, but the slva844a.pdf TI appnote has a table listing the possible pin permutations which can be used with DP over Type-C (Table 1. page 5) which always has all the superspeed USB pins in the same place. Regards, Hans
diff --git a/include/linux/mux/usb.h b/include/linux/mux/usb.h new file mode 100644 index 000000000000..44df5eca5256 --- /dev/null +++ b/include/linux/mux/usb.h @@ -0,0 +1,32 @@ +/* + * mux/usb.h - definitions for USB multiplexers + * + * Copyright (C) 2017 Hans de Goede <hdegoede@redhat.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ +#ifndef _LINUX_MUX_USB_H +#define _LINUX_MUX_USB_H + +/* Mux state values for USB device/host role muxes */ +#define MUX_USB_DEVICE (0) /* USB device mode */ +#define MUX_USB_HOST (1) /* USB host mode */ +#define MUX_USB_STATES (2) + +/* + * Mux state values for Type-C polarity/role/altmode muxes. + * + * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as + * inverted-polarity (Type-C plugged in upside down) can happen with any + * other mux-state. + */ +#define MUX_TYPEC_POLARITY_INV BIT(0) /* Polarity inverted bit */ +#define MUX_TYPEC_DEVICE (0 << 1) /* USB device mode */ +#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */ +#define MUX_TYPEC_HOST_AND_DP_SRC (2 << 1) /* USB host + 2 lanes DP src */ +#define MUX_TYPEC_DP_SRC (3 << 1) /* 4 lanes Display Port src */ +#define MUX_TYPEC_STATES (4 << 1) + +#endif /* _LINUX_MUX_TYPEC_H */
Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by USB device/host, resp. Type-C polarity/role/altmode mux drivers and consumers to ensure that they agree on the meaning of the mux_control_select() state argument. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Changes in v2: -Start numbering of defines at 0 not 1 -Use a new usb.h header, rather then adding these to consumer.h -Add separate MUX_USB_* and MUX_TYPEC_* defines --- include/linux/mux/usb.h | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 include/linux/mux/usb.h