Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1726854 > unrolled thread

[PATCH v2 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 00/11] mux/typec: Add USB / TypeC mux drivers and hook them up on some x86 systems Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
    [PATCH v2 05/11] mux: Add Intel Cherrytrail USB mux driver Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
      Re: [PATCH v2 05/11] mux: Add Intel Cherrytrail USB mux driver Hans de Goede <hdegoede@redhat.com> - 2017-09-19 18:40 +0200
      Re: [PATCH 1/2] mux: add mux_control_get_optional() API Hans de Goede <hdegoede@redhat.com> - 2017-09-19 20:40 +0200
        Re: [PATCH 1/2] mux: add mux_control_get_optional() API Stephen Boyd <stephen.boyd@linaro.org> - 2017-09-20 18:20 +0200
    [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support using tcpc_gen_mux support Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
      Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Rob Herring <robh@kernel.org> - 2017-09-13 00:30 +0200
        Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Hans de Goede <hdegoede@redhat.com> - 2017-09-13 11:00 +0200
          Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Rob Herring <robh@kernel.org> - 2017-09-13 15:40 +0200
            Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Hans de Goede <hdegoede@redhat.com> - 2017-09-13 16:10 +0200
              Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Rob Herring <robh@kernel.org> - 2017-09-13 17:10 +0200
                Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Hans de Goede <hdegoede@redhat.com> - 2017-09-13 17:50 +0200
                  Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Guenter Roeck <linux@roeck-us.net> - 2017-09-13 18:20 +0200
                  Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Hans de Goede <hdegoede@redhat.com> - 2017-09-25 13:40 +0200
                    Re: [PATCH v2 10/11] staging: typec: fusb302: Hook up mux support  using tcpc_gen_mux support Hans de Goede <hdegoede@redhat.com> - 2017-09-25 16:20 +0200
    [PATCH v2 06/11] mux: Add Pericom PI3USB30532 Type-C mux driver Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
    [PATCH v2 09/11] staging: typec: Add Generic TCPC mux driver using the mux subsys Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
    [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
      Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap /  otg phy mux handling Mathias Nyman <mathias.nyman@linux.intel.com> - 2017-09-07 15:20 +0200
        Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap /  otg phy mux handling Hans de Goede <hdegoede@redhat.com> - 2017-09-07 17:50 +0200
          Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap /  otg phy mux handling Mathias Nyman <mathias.nyman@linux.intel.com> - 2017-09-19 14:40 +0200
            Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap /  otg phy mux handling Hans de Goede <hdegoede@redhat.com> - 2017-09-21 14:00 +0200
    [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
      Re: [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and  and MUX_TYPEC_* state constants Hans de Goede <hdegoede@redhat.com> - 2017-09-08 19:10 +0200
        Re: [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and  and MUX_TYPEC_* state constants Hans de Goede <j.w.r.degoede@gmail.com> - 2017-09-21 14:10 +0200
    [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode when configured as such Hans de Goede <hdegoede@redhat.com> - 2017-09-05 18:50 +0200
      Re: [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode  when configured as such Guenter Roeck <linux@roeck-us.net> - 2017-09-11 01:00 +0200
        Re: [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode  when configured as such Hans de Goede <hdegoede@redhat.com> - 2017-09-22 16:10 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1734874 — Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

[Multipart message — attachments visible in raw view] — view raw

Hi,

sorry about the long delay

On 07.09.2017 18:49, Hans de Goede wrote:
> Hi,
>
> On 07-09-17 15:14, Mathias Nyman wrote:
>> On 05.09.2017 19:42, Hans de Goede wrote:
>>> The Intel cherrytrail xhci controller has an extended cap mmio-range
>>> which contains registers to control the muxing to the xhci (host mode)
>>> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>>>
>>> Having a mux driver included in the xhci code (or under drivers/usb/host)
>>> is not desirable. So this commit adds a simple handler for this extended
>>> capability, which creates a platform device with the caps mmio region as
>>> resource, this allows us to write a separate platform mux driver for the
>>> mux.
>>>
>> I think it would be better to have one place where we add handlers for
>> vendor specific extended capabilities.
>>
>> Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
>> there's a xhci-ext-caps.h header already
>>
>> We could walk through the capability list once and add the needed handlers.
>> Something like:
>>
>> +int xhci_ext_cap_init(void __iomem *base)
>
> This will need to take a struct xhci_hcd *xhci param instead
> as some of the ext_cap handling (including the cht mux code)
> will need access to this.
>

yes, sample code added in second patch for reference/testing.

>
> So I see 2 options here (without making this function PCI specific)
> 1) Add an u32 product_id field to struct xhci_hcd; or
> 2) Use a quirk flag as my current code is doing.
>
> I'm fine with doing this either way, please let me know your preference.

Lets go with the quirk for now, I'll sort that out later

>
> Can you do a "git format-patch" of that and send it to me? If you
> can give me that + your preference for how to check if we're
> dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
> with your suggestions applied.

Ended up modifying xhci_find_next_ext_cap() using id = 0 for
the next capability in list. Patch attached,

Second patch is just for reference how to use it.

Thanks
-Mathias

[toc] | [prev] | [next] | [standalone]

#1736601 — Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

Hi,

On 19-09-17 14:40, Mathias Nyman wrote:
> Hi,
> 
> sorry about the long delay
> 
> On 07.09.2017 18:49, Hans de Goede wrote:
>> Hi,
>>
>> On 07-09-17 15:14, Mathias Nyman wrote:
>>> On 05.09.2017 19:42, Hans de Goede wrote:
>>>> The Intel cherrytrail xhci controller has an extended cap mmio-range
>>>> which contains registers to control the muxing to the xhci (host mode)
>>>> or the dwc3 (device mode) and vbus-detection for the otg usb-phy.
>>>>
>>>> Having a mux driver included in the xhci code (or under drivers/usb/host)
>>>> is not desirable. So this commit adds a simple handler for this extended
>>>> capability, which creates a platform device with the caps mmio region as
>>>> resource, this allows us to write a separate platform mux driver for the
>>>> mux.
>>>>
>>> I think it would be better to have one place where we add handlers for
>>> vendor specific extended capabilities.
>>>
>>> Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
>>> there's a xhci-ext-caps.h header already
>>>
>>> We could walk through the capability list once and add the needed handlers.
>>> Something like:
>>>
>>> +int xhci_ext_cap_init(void __iomem *base)
>>
>> This will need to take a struct xhci_hcd *xhci param instead
>> as some of the ext_cap handling (including the cht mux code)
>> will need access to this.
>>
> 
> yes, sample code added in second patch for reference/testing.
> 
>>
>> So I see 2 options here (without making this function PCI specific)
>> 1) Add an u32 product_id field to struct xhci_hcd; or
>> 2) Use a quirk flag as my current code is doing.
>>
>> I'm fine with doing this either way, please let me know your preference.
> 
> Lets go with the quirk for now, I'll sort that out later
> 
>>
>> Can you do a "git format-patch" of that and send it to me? If you
>> can give me that + your preference for how to check if we're
>> dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
>> with your suggestions applied.
> 
> Ended up modifying xhci_find_next_ext_cap() using id = 0 for
> the next capability in list. Patch attached,
> 
> Second patch is just for reference how to use it.

Thank you for the patches, I'm working on prepping a v3 of
this series which includes and uses the first patch.

Regards,

Hans

[toc] | [prev] | [next] | [standalone]

#1726865 — [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants

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)
+
+#endif /* _LINUX_MUX_TYPEC_H */
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1729091 — Re: [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants

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

[toc] | [prev] | [next] | [standalone]

#1736604 — Re: [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants

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

[toc] | [prev] | [next] | [standalone]

#1726866 — [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode when configured as such

Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
data-role is device is not correct. Plenty of devices support operating
as USB device through a (separate) USB device controller.

So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
accordingly.

Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
this is for configuring the Type-C port as a Display Port source, not a
sink.

Last this commit makes tcpm_reset_port() to set the mux to
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT so that it does not and up
staying in host (and with this commit also device) mode after a detach.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/staging/typec/tcpm.c |  7 ++++---
 drivers/staging/typec/tcpm.h | 22 ++++++++++++++--------
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
index 8af62e74d54c..ffe7e26d4ed3 100644
--- a/drivers/staging/typec/tcpm.c
+++ b/drivers/staging/typec/tcpm.c
@@ -752,11 +752,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
 	int ret;
 
 	if (data == TYPEC_HOST)
-		ret = tcpm_mux_set(port, TYPEC_MUX_USB,
+		ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
 				   TCPC_USB_SWITCH_CONNECT);
 	else
-		ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
-				   TCPC_USB_SWITCH_DISCONNECT);
+		ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
+				   TCPC_USB_SWITCH_CONNECT);
 	if (ret < 0)
 		return ret;
 
@@ -2025,6 +2025,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
 	tcpm_init_vconn(port);
 	tcpm_set_current_limit(port, 0, 0);
 	tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
+	tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
 	tcpm_set_attached_state(port, false);
 	port->try_src_count = 0;
 	port->try_snk_count = 0;
diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
index 7e9a6b7b5cd6..f662eed48c86 100644
--- a/drivers/staging/typec/tcpm.h
+++ b/drivers/staging/typec/tcpm.h
@@ -83,17 +83,23 @@ enum tcpc_usb_switch {
 };
 
 /* 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 */
+#define TCPC_MUX_USB_DEVICE_ENABLED		BIT(0)	/* USB device enabled */
+#define TCPC_MUX_USB_HOST_ENABLED		BIT(1)	/* USB host enabled */
+#define TCPC_MUX_DP_SRC_ENABLED			BIT(2)	/* DP enabled */
+#define TCPC_MUX_POLARITY_INVERTED		BIT(3)	/* Polarity inverted */
 
 /* Mux modes, decoded to attributes */
 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,
+	/* Open switch */
+	TYPEC_MUX_NONE = 0,
+	/* USB device only */
+	TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
+	/* USB host only */
+	TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
+	/* DP source only */
+	TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
+	/* Both USB host and DP source */
+	TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
 };
 
 struct tcpc_mux_dev {
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]

#1730081 — Re: [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode when configured as such

On 09/05/2017 09:42 AM, Hans de Goede wrote:
> Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
> data-role is device is not correct. Plenty of devices support operating
> as USB device through a (separate) USB device controller.
> 
> So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
> and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
> accordingly.
> 
> Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
> this is for configuring the Type-C port as a Display Port source, not a
> sink.
> 
> Last this commit makes tcpm_reset_port() to set the mux to
> TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT so that it does not and up
> staying in host (and with this commit also device) mode after a detach.
> 
This sentence is hard to understand.

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

Otherwise

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   drivers/staging/typec/tcpm.c |  7 ++++---
>   drivers/staging/typec/tcpm.h | 22 ++++++++++++++--------
>   2 files changed, 18 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
> index 8af62e74d54c..ffe7e26d4ed3 100644
> --- a/drivers/staging/typec/tcpm.c
> +++ b/drivers/staging/typec/tcpm.c
> @@ -752,11 +752,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
>   	int ret;
>   
>   	if (data == TYPEC_HOST)
> -		ret = tcpm_mux_set(port, TYPEC_MUX_USB,
> +		ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
>   				   TCPC_USB_SWITCH_CONNECT);
>   	else
> -		ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
> -				   TCPC_USB_SWITCH_DISCONNECT);
> +		ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
> +				   TCPC_USB_SWITCH_CONNECT);
>   	if (ret < 0)
>   		return ret;
>   
> @@ -2025,6 +2025,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>   	tcpm_init_vconn(port);
>   	tcpm_set_current_limit(port, 0, 0);
>   	tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
> +	tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
>   	tcpm_set_attached_state(port, false);
>   	port->try_src_count = 0;
>   	port->try_snk_count = 0;
> diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
> index 7e9a6b7b5cd6..f662eed48c86 100644
> --- a/drivers/staging/typec/tcpm.h
> +++ b/drivers/staging/typec/tcpm.h
> @@ -83,17 +83,23 @@ enum tcpc_usb_switch {
>   };
>   
>   /* 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 */
> +#define TCPC_MUX_USB_DEVICE_ENABLED		BIT(0)	/* USB device enabled */
> +#define TCPC_MUX_USB_HOST_ENABLED		BIT(1)	/* USB host enabled */
> +#define TCPC_MUX_DP_SRC_ENABLED			BIT(2)	/* DP enabled */
> +#define TCPC_MUX_POLARITY_INVERTED		BIT(3)	/* Polarity inverted */
>   
>   /* Mux modes, decoded to attributes */
>   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,
> +	/* Open switch */
> +	TYPEC_MUX_NONE = 0,
> +	/* USB device only */
> +	TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
> +	/* USB host only */
> +	TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
> +	/* DP source only */
> +	TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
> +	/* Both USB host and DP source */
> +	TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
>   };
>   
>   struct tcpc_mux_dev {
> 

[toc] | [prev] | [next] | [standalone]

#1737518 — Re: [PATCH v2 08/11] staging: typec: tcpm: Set mux to device mode when configured as such

Hi,

On 09/11/2017 12:56 AM, Guenter Roeck wrote:
> On 09/05/2017 09:42 AM, Hans de Goede wrote:
>> Setting the mux to TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT when the
>> data-role is device is not correct. Plenty of devices support operating
>> as USB device through a (separate) USB device controller.
>>
>> So this commit instead splits out TYPEC_MUX_USB into TYPEC_MUX_USB_HOST
>> and TYPEC_MUX_USB_DEVICE and makes tcpm_set_roles() set the mux
>> accordingly.
>>
>> Likewise TCPC_MUX_DP gets renamed to TCPC_MUX_DP_SRC to make clear that
>> this is for configuring the Type-C port as a Display Port source, not a
>> sink.
>>
>> Last this commit makes tcpm_reset_port() to set the mux to
>> TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT so that it does not and up
>> staying in host (and with this commit also device) mode after a detach.
>>
> This sentence is hard to understand.

Ok, changed to:

"Last this commit makes tcpm_reset_port() call tcpm_mux_set(port,
TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT) so that the mux does _not_
stay in its last mode after a detach."

For v3 of this patchset.

> 
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> Otherwise
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

And added your Reviewed-by.

Thanks & Regards,

Hans




>
> 
>> ---
>>   drivers/staging/typec/tcpm.c |  7 ++++---
>>   drivers/staging/typec/tcpm.h | 22 ++++++++++++++--------
>>   2 files changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/typec/tcpm.c b/drivers/staging/typec/tcpm.c
>> index 8af62e74d54c..ffe7e26d4ed3 100644
>> --- a/drivers/staging/typec/tcpm.c
>> +++ b/drivers/staging/typec/tcpm.c
>> @@ -752,11 +752,11 @@ static int tcpm_set_roles(struct tcpm_port *port, bool attached,
>>       int ret;
>>       if (data == TYPEC_HOST)
>> -        ret = tcpm_mux_set(port, TYPEC_MUX_USB,
>> +        ret = tcpm_mux_set(port, TYPEC_MUX_USB_HOST,
>>                      TCPC_USB_SWITCH_CONNECT);
>>       else
>> -        ret = tcpm_mux_set(port, TYPEC_MUX_NONE,
>> -                   TCPC_USB_SWITCH_DISCONNECT);
>> +        ret = tcpm_mux_set(port, TYPEC_MUX_USB_DEVICE,
>> +                   TCPC_USB_SWITCH_CONNECT);
>>       if (ret < 0)
>>           return ret;
>> @@ -2025,6 +2025,7 @@ static void tcpm_reset_port(struct tcpm_port *port)
>>       tcpm_init_vconn(port);
>>       tcpm_set_current_limit(port, 0, 0);
>>       tcpm_set_polarity(port, TYPEC_POLARITY_CC1);
>> +    tcpm_mux_set(port, TYPEC_MUX_NONE, TCPC_USB_SWITCH_DISCONNECT);
>>       tcpm_set_attached_state(port, false);
>>       port->try_src_count = 0;
>>       port->try_snk_count = 0;
>> diff --git a/drivers/staging/typec/tcpm.h b/drivers/staging/typec/tcpm.h
>> index 7e9a6b7b5cd6..f662eed48c86 100644
>> --- a/drivers/staging/typec/tcpm.h
>> +++ b/drivers/staging/typec/tcpm.h
>> @@ -83,17 +83,23 @@ enum tcpc_usb_switch {
>>   };
>>   /* 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 */
>> +#define TCPC_MUX_USB_DEVICE_ENABLED        BIT(0)    /* USB device enabled */
>> +#define TCPC_MUX_USB_HOST_ENABLED        BIT(1)    /* USB host enabled */
>> +#define TCPC_MUX_DP_SRC_ENABLED            BIT(2)    /* DP enabled */
>> +#define TCPC_MUX_POLARITY_INVERTED        BIT(3)    /* Polarity inverted */
>>   /* Mux modes, decoded to attributes */
>>   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,
>> +    /* Open switch */
>> +    TYPEC_MUX_NONE = 0,
>> +    /* USB device only */
>> +    TYPEC_MUX_USB_DEVICE = TCPC_MUX_USB_DEVICE_ENABLED,
>> +    /* USB host only */
>> +    TYPEC_MUX_USB_HOST = TCPC_MUX_USB_HOST_ENABLED,
>> +    /* DP source only */
>> +    TYPEC_MUX_DP = TCPC_MUX_DP_SRC_ENABLED,
>> +    /* Both USB host and DP source */
>> +    TYPEC_MUX_DOCK = TCPC_MUX_USB_HOST_ENABLED | TCPC_MUX_DP_SRC_ENABLED,
>>   };
>>   struct tcpc_mux_dev {
>>
> 

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web