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

Groups > linux.kernel > #1394841 > unrolled thread

Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Guenter Roeck <linux@roeck-us.net> - 2016-05-05 05:10 +0200
    Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Felipe Balbi <balbi@kernel.org> - 2016-05-06 09:00 +0200
      Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Guenter Roeck <linux@roeck-us.net> - 2016-05-06 10:10 +0200
        Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-06 10:30 +0200
          Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Guenter Roeck <linux@roeck-us.net> - 2016-05-06 16:20 +0200
      Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-06 10:30 +0200
    Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-06 10:10 +0200
      Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Guenter Roeck <linux@roeck-us.net> - 2016-05-06 16:10 +0200
      Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Guenter Roeck <linux@roeck-us.net> - 2016-05-11 05:20 +0200
        Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Heikki Krogerus <heikki.krogerus@linux.intel.com> - 2016-05-11 11:50 +0200
          Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Guenter Roeck <linux@roeck-us.net> - 2016-05-11 16:50 +0200

#1394841 — Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI

On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
> Hi,
> 
> The OS, or more precisely the user space, needs to be able to control
> a few things regarding USB Type-C ports. The first thing that must be
> allowed to be controlled is the data role. USB Type-C ports will
> select the data role randomly with DRP ports. When USB PD is
> supported, also independent (from data role) power role swapping can
> be supported together with Alternate Mode control.
> 
> I'm proposing with this set a Class for the Type-C connectors that
> gives the user space control over those things on top of getting basic
> details about the USB Type-C connectors and also partners. The details
> include the capabilities of the port, the supported data and power
> roles, supported accessories (audio and debug), supported Alternate
> Modes, USB PD support and of course the type of the partner (USB, Alt
> Mode, Accessory or Charger), and more or less the same details about
> the partner.
> 
> I'm not considering cables with this Class, and I have deliberately
> left out some more technical details, like cable orientation, firstly
> because I did not see much use for the user space from knowing that
> an secondly because that kind of details are not always available for
> example with UCSI.
> 
> So the interface to the user space is kept as simple as I dared to
> make it.
> 
> NOTE: In case there is somebody wondering, this is not adding USB PD
> support to Linux kernel. This is just about USB Type-C.
> 

Hello Heikki,

we have implemented a prototype TCPM (USB Type-C Protocol Manager)
software on top of your patch set. It will support TCPCI as well
as other USB-C controllers such as FUSB302. The plan is to use
this software in systems where no separate controller is available.

Is there any chance to advance this patch set ? It would be instrumental
to get a unified interface to user space.

Thanks,
Guenter

[toc] | [next] | [standalone]

#1395646

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

Hi Guenter,

Guenter Roeck <linux@roeck-us.net> writes:
> On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
>> Hi,
>> 
>> The OS, or more precisely the user space, needs to be able to control
>> a few things regarding USB Type-C ports. The first thing that must be
>> allowed to be controlled is the data role. USB Type-C ports will
>> select the data role randomly with DRP ports. When USB PD is
>> supported, also independent (from data role) power role swapping can
>> be supported together with Alternate Mode control.
>> 
>> I'm proposing with this set a Class for the Type-C connectors that
>> gives the user space control over those things on top of getting basic
>> details about the USB Type-C connectors and also partners. The details
>> include the capabilities of the port, the supported data and power
>> roles, supported accessories (audio and debug), supported Alternate
>> Modes, USB PD support and of course the type of the partner (USB, Alt
>> Mode, Accessory or Charger), and more or less the same details about
>> the partner.
>> 
>> I'm not considering cables with this Class, and I have deliberately
>> left out some more technical details, like cable orientation, firstly
>> because I did not see much use for the user space from knowing that
>> an secondly because that kind of details are not always available for
>> example with UCSI.
>> 
>> So the interface to the user space is kept as simple as I dared to
>> make it.
>> 
>> NOTE: In case there is somebody wondering, this is not adding USB PD
>> support to Linux kernel. This is just about USB Type-C.
>> 
>
> Hello Heikki,
>
> we have implemented a prototype TCPM (USB Type-C Protocol Manager)
> software on top of your patch set. It will support TCPCI as well
> as other USB-C controllers such as FUSB302. The plan is to use
> this software in systems where no separate controller is available.
>
> Is there any chance to advance this patch set ? It would be instrumental
> to get a unified interface to user space.

A newer version of $subject is already in Greg's queue [1]

[1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=0c1849a8c7af652c92ad0265a7ca5934fd773c69

-- 
balbi

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

#1395682

Felipe,

On 05/05/2016 11:50 PM, Felipe Balbi wrote:
>
> Hi Guenter,
>
> Guenter Roeck <linux@roeck-us.net> writes:
>> On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
>>> Hi,
>>>
>>> The OS, or more precisely the user space, needs to be able to control
>>> a few things regarding USB Type-C ports. The first thing that must be
>>> allowed to be controlled is the data role. USB Type-C ports will
>>> select the data role randomly with DRP ports. When USB PD is
>>> supported, also independent (from data role) power role swapping can
>>> be supported together with Alternate Mode control.
>>>
>>> I'm proposing with this set a Class for the Type-C connectors that
>>> gives the user space control over those things on top of getting basic
>>> details about the USB Type-C connectors and also partners. The details
>>> include the capabilities of the port, the supported data and power
>>> roles, supported accessories (audio and debug), supported Alternate
>>> Modes, USB PD support and of course the type of the partner (USB, Alt
>>> Mode, Accessory or Charger), and more or less the same details about
>>> the partner.
>>>
>>> I'm not considering cables with this Class, and I have deliberately
>>> left out some more technical details, like cable orientation, firstly
>>> because I did not see much use for the user space from knowing that
>>> an secondly because that kind of details are not always available for
>>> example with UCSI.
>>>
>>> So the interface to the user space is kept as simple as I dared to
>>> make it.
>>>
>>> NOTE: In case there is somebody wondering, this is not adding USB PD
>>> support to Linux kernel. This is just about USB Type-C.
>>>
>>
>> Hello Heikki,
>>
>> we have implemented a prototype TCPM (USB Type-C Protocol Manager)
>> software on top of your patch set. It will support TCPCI as well
>> as other USB-C controllers such as FUSB302. The plan is to use
>> this software in systems where no separate controller is available.
>>
>> Is there any chance to advance this patch set ? It would be instrumental
>> to get a unified interface to user space.
>
> A newer version of $subject is already in Greg's queue [1]
>
> [1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=0c1849a8c7af652c92ad0265a7ca5934fd773c69
>
I am aware of that patch.

Unfortunately, unlike the original submission, the new patch is not an
infrastructure, it is just a driver supporting Intel's UCSI. Unlike the
original series, it does not provide an infrastructure, and it does not
support other implementations of USB Type-C port management systems.

In our system, we'll have (at least) three such implementations:

- TCPM and TCPC implemented in EC and/or microcontrollers.
   This is currently implemented and shipping with some Chromebooks.
- TCPM implemented in Linux, interfacing to a standard TCPC, using TCPCI
   for TCPM-TCPC communication
   This will be needed for systems with no EC and a standard Type-C port
   controller.
- TCPM implemented in Linux, interfacing to FUSB302.
   This will be needed for systems with no EC, utilizing a FUSB302
   port controller.

All those fit nicely into the infrastructure provided by the original
patch series, where UCSI was just one possible implementation of a
USB Type-C port management system.

The original patch series had the tremendous advantage of presenting a
unified ABI to user space. With the new patch, this is no longer the case.
All implementations would be completely separate and thus effectively
guarantee ABI fragmentation (Fairchild's code supporting FUSB302 in Linux
is a good example. The existing implementation of Type-C support in the
Chromebooks mentioned above is another).

I know there has been a lengthy discussion about the patch set, but I may
have missed the conclusion. Is there some reason to _not_ advance it
that I may have missed ?

Thanks,
Guenter

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

#1395689

On Fri, May 06, 2016 at 01:05:05AM -0700, Guenter Roeck wrote:
> Felipe,
> 
> On 05/05/2016 11:50 PM, Felipe Balbi wrote:
> > 
> > Hi Guenter,
> > 
> > Guenter Roeck <linux@roeck-us.net> writes:
> > > On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
> > > > Hi,
> > > > 
> > > > The OS, or more precisely the user space, needs to be able to control
> > > > a few things regarding USB Type-C ports. The first thing that must be
> > > > allowed to be controlled is the data role. USB Type-C ports will
> > > > select the data role randomly with DRP ports. When USB PD is
> > > > supported, also independent (from data role) power role swapping can
> > > > be supported together with Alternate Mode control.
> > > > 
> > > > I'm proposing with this set a Class for the Type-C connectors that
> > > > gives the user space control over those things on top of getting basic
> > > > details about the USB Type-C connectors and also partners. The details
> > > > include the capabilities of the port, the supported data and power
> > > > roles, supported accessories (audio and debug), supported Alternate
> > > > Modes, USB PD support and of course the type of the partner (USB, Alt
> > > > Mode, Accessory or Charger), and more or less the same details about
> > > > the partner.
> > > > 
> > > > I'm not considering cables with this Class, and I have deliberately
> > > > left out some more technical details, like cable orientation, firstly
> > > > because I did not see much use for the user space from knowing that
> > > > an secondly because that kind of details are not always available for
> > > > example with UCSI.
> > > > 
> > > > So the interface to the user space is kept as simple as I dared to
> > > > make it.
> > > > 
> > > > NOTE: In case there is somebody wondering, this is not adding USB PD
> > > > support to Linux kernel. This is just about USB Type-C.
> > > > 
> > > 
> > > Hello Heikki,
> > > 
> > > we have implemented a prototype TCPM (USB Type-C Protocol Manager)
> > > software on top of your patch set. It will support TCPCI as well
> > > as other USB-C controllers such as FUSB302. The plan is to use
> > > this software in systems where no separate controller is available.
> > > 
> > > Is there any chance to advance this patch set ? It would be instrumental
> > > to get a unified interface to user space.
> > 
> > A newer version of $subject is already in Greg's queue [1]
> > 
> > [1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=0c1849a8c7af652c92ad0265a7ca5934fd773c69
> > 
> I am aware of that patch.
> 
> Unfortunately, unlike the original submission, the new patch is not an
> infrastructure, it is just a driver supporting Intel's UCSI. Unlike the
> original series, it does not provide an infrastructure, and it does not
> support other implementations of USB Type-C port management systems.
> 
> In our system, we'll have (at least) three such implementations:
> 
> - TCPM and TCPC implemented in EC and/or microcontrollers.
>   This is currently implemented and shipping with some Chromebooks.
> - TCPM implemented in Linux, interfacing to a standard TCPC, using TCPCI
>   for TCPM-TCPC communication
>   This will be needed for systems with no EC and a standard Type-C port
>   controller.
> - TCPM implemented in Linux, interfacing to FUSB302.
>   This will be needed for systems with no EC, utilizing a FUSB302
>   port controller.
> 
> All those fit nicely into the infrastructure provided by the original
> patch series, where UCSI was just one possible implementation of a
> USB Type-C port management system.
> 
> The original patch series had the tremendous advantage of presenting a
> unified ABI to user space. With the new patch, this is no longer the case.
> All implementations would be completely separate and thus effectively
> guarantee ABI fragmentation (Fairchild's code supporting FUSB302 in Linux
> is a good example. The existing implementation of Type-C support in the
> Chromebooks mentioned above is another).
> 
> I know there has been a lengthy discussion about the patch set, but I may
> have missed the conclusion. Is there some reason to _not_ advance it
> that I may have missed ?

No, we are still continuing with the class driver. We just descided to
split the UCIS into separate driver for now, just because we needed it
to be supported fast. But I did mention in the commit message of the
UCSI patch that the goal is to merge that into a Type-C framework once
it's awailable.


Thanks,

-- 
heikki

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

#1395885

Hello Heikki,

On 05/06/2016 01:29 AM, Heikki Krogerus wrote:
> On Fri, May 06, 2016 at 01:05:05AM -0700, Guenter Roeck wrote:
[ ... ]
>> I know there has been a lengthy discussion about the patch set, but I may
>> have missed the conclusion. Is there some reason to _not_ advance it
>> that I may have missed ?
>
> No, we are still continuing with the class driver. We just descided to
> split the UCIS into separate driver for now, just because we needed it
> to be supported fast. But I did mention in the commit message of the
> UCSI patch that the goal is to merge that into a Type-C framework once
> it's awailable.
>
Yes, I noticed. I had suspected that your need for a driver now was the
reason for the stand-alone driver. Felipe's response got me concerned
for a minute, though.

Thanks,
Guenter

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

#1395687

On Fri, May 06, 2016 at 09:50:00AM +0300, Felipe Balbi wrote:
> 
> Hi Guenter,
> 
> Guenter Roeck <linux@roeck-us.net> writes:
> > On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
> >> Hi,
> >> 
> >> The OS, or more precisely the user space, needs to be able to control
> >> a few things regarding USB Type-C ports. The first thing that must be
> >> allowed to be controlled is the data role. USB Type-C ports will
> >> select the data role randomly with DRP ports. When USB PD is
> >> supported, also independent (from data role) power role swapping can
> >> be supported together with Alternate Mode control.
> >> 
> >> I'm proposing with this set a Class for the Type-C connectors that
> >> gives the user space control over those things on top of getting basic
> >> details about the USB Type-C connectors and also partners. The details
> >> include the capabilities of the port, the supported data and power
> >> roles, supported accessories (audio and debug), supported Alternate
> >> Modes, USB PD support and of course the type of the partner (USB, Alt
> >> Mode, Accessory or Charger), and more or less the same details about
> >> the partner.
> >> 
> >> I'm not considering cables with this Class, and I have deliberately
> >> left out some more technical details, like cable orientation, firstly
> >> because I did not see much use for the user space from knowing that
> >> an secondly because that kind of details are not always available for
> >> example with UCSI.
> >> 
> >> So the interface to the user space is kept as simple as I dared to
> >> make it.
> >> 
> >> NOTE: In case there is somebody wondering, this is not adding USB PD
> >> support to Linux kernel. This is just about USB Type-C.
> >> 
> >
> > Hello Heikki,
> >
> > we have implemented a prototype TCPM (USB Type-C Protocol Manager)
> > software on top of your patch set. It will support TCPCI as well
> > as other USB-C controllers such as FUSB302. The plan is to use
> > this software in systems where no separate controller is available.
> >
> > Is there any chance to advance this patch set ? It would be instrumental
> > to get a unified interface to user space.
> 
> A newer version of $subject is already in Greg's queue [1]
> 
> [1] https://git.kernel.org/cgit/linux/kernel/git/gregkh/usb.git/commit/?h=usb-next&id=0c1849a8c7af652c92ad0265a7ca5934fd773c69

No that only has the separated UCSI driver, not the class.



-- 
heikki

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

#1395683

Hi,

On Wed, May 04, 2016 at 08:05:44PM -0700, Guenter Roeck wrote:
> On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
> > Hi,
> > 
> > The OS, or more precisely the user space, needs to be able to control
> > a few things regarding USB Type-C ports. The first thing that must be
> > allowed to be controlled is the data role. USB Type-C ports will
> > select the data role randomly with DRP ports. When USB PD is
> > supported, also independent (from data role) power role swapping can
> > be supported together with Alternate Mode control.
> > 
> > I'm proposing with this set a Class for the Type-C connectors that
> > gives the user space control over those things on top of getting basic
> > details about the USB Type-C connectors and also partners. The details
> > include the capabilities of the port, the supported data and power
> > roles, supported accessories (audio and debug), supported Alternate
> > Modes, USB PD support and of course the type of the partner (USB, Alt
> > Mode, Accessory or Charger), and more or less the same details about
> > the partner.
> > 
> > I'm not considering cables with this Class, and I have deliberately
> > left out some more technical details, like cable orientation, firstly
> > because I did not see much use for the user space from knowing that
> > an secondly because that kind of details are not always available for
> > example with UCSI.
> > 
> > So the interface to the user space is kept as simple as I dared to
> > make it.
> > 
> > NOTE: In case there is somebody wondering, this is not adding USB PD
> > support to Linux kernel. This is just about USB Type-C.
> > 
> 
> Hello Heikki,
> 
> we have implemented a prototype TCPM (USB Type-C Protocol Manager)
> software on top of your patch set. It will support TCPCI as well
> as other USB-C controllers such as FUSB302. The plan is to use
> this software in systems where no separate controller is available.

Interesting. So I'm guessing TCPM is actually an implementation USB PD
software stack, no? AFAIK FUSB302 has USB PD transceiver, right?

> Is there any chance to advance this patch set ? It would be instrumental
> to get a unified interface to user space.

I don't have not made any new code for the class driver yet, but I'm
attempting to prepare v2 next week.


Thanks,

-- 
heikki

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

#1395883

On 05/06/2016 01:08 AM, Heikki Krogerus wrote:
> Hi,
>
> On Wed, May 04, 2016 at 08:05:44PM -0700, Guenter Roeck wrote:
>> On Tue, Feb 09, 2016 at 07:01:20PM +0200, Heikki Krogerus wrote:
>>> Hi,
>>>
>>> The OS, or more precisely the user space, needs to be able to control
>>> a few things regarding USB Type-C ports. The first thing that must be
>>> allowed to be controlled is the data role. USB Type-C ports will
>>> select the data role randomly with DRP ports. When USB PD is
>>> supported, also independent (from data role) power role swapping can
>>> be supported together with Alternate Mode control.
>>>
>>> I'm proposing with this set a Class for the Type-C connectors that
>>> gives the user space control over those things on top of getting basic
>>> details about the USB Type-C connectors and also partners. The details
>>> include the capabilities of the port, the supported data and power
>>> roles, supported accessories (audio and debug), supported Alternate
>>> Modes, USB PD support and of course the type of the partner (USB, Alt
>>> Mode, Accessory or Charger), and more or less the same details about
>>> the partner.
>>>
>>> I'm not considering cables with this Class, and I have deliberately
>>> left out some more technical details, like cable orientation, firstly
>>> because I did not see much use for the user space from knowing that
>>> an secondly because that kind of details are not always available for
>>> example with UCSI.
>>>
>>> So the interface to the user space is kept as simple as I dared to
>>> make it.
>>>
>>> NOTE: In case there is somebody wondering, this is not adding USB PD
>>> support to Linux kernel. This is just about USB Type-C.
>>>
>>
>> Hello Heikki,
>>
>> we have implemented a prototype TCPM (USB Type-C Protocol Manager)
>> software on top of your patch set. It will support TCPCI as well
>> as other USB-C controllers such as FUSB302. The plan is to use
>> this software in systems where no separate controller is available.
>
> Interesting. So I'm guessing TCPM is actually an implementation USB PD
> software stack, no? AFAIK FUSB302 has USB PD transceiver, right?
>
Yes, both correct.

>> Is there any chance to advance this patch set ? It would be instrumental
>> to get a unified interface to user space.
>
> I don't have not made any new code for the class driver yet, but I'm
> attempting to prepare v2 next week.
>

Excellent! Please cc me - either this e-mail address or groeck@chromium.org.

Thanks,
Guenter

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

#1398652

Heikki,

On 05/06/2016 01:08 AM, Heikki Krogerus wrote:
> Hi,
>
[ ... ]
>
> I don't have not made any new code for the class driver yet, but I'm
> attempting to prepare v2 next week.
>
Would it make sense to send feedback about v1 now, or should I wait for v2 ?

Thanks,
Guenter

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

#1398875

On Tue, May 10, 2016 at 08:14:34PM -0700, Guenter Roeck wrote:
> Heikki,
> 
> On 05/06/2016 01:08 AM, Heikki Krogerus wrote:
> > Hi,
> > 
> [ ... ]
> > 
> > I don't have not made any new code for the class driver yet, but I'm
> > attempting to prepare v2 next week.
> > 
> Would it make sense to send feedback about v1 now, or should I wait for v2 ?

I don't think I'm able to send v2 today, or even tomorrow, so feel
free to give the feedback. Just be aware that I've rewritten the
alternate mode part completely.

I'm creating a separate device for the partner and also the cable
during connection. I'm also already going to introduce a small bus for
the AltModes. It's clear that we need to have AltMode specific
drivers. The generic parts can't take care of all the AltMode specific
requirements and VDMs. The bus will give us a nice way to bind those
drivers to the actual AltModes a partner and the cable plugs offer.

So if there are dependencies between the altmodes, for example if the
cable plugs needs to be in a certain mode in order for the partner to
be able to function in some specific mode, the responsibility of
taking care of those will fall primarily to in the AltMode drivers.
So not userspace.

The AltMode drivers actually are useful also as they can be part of
the relevant frameworks, for example DP in some graphics framework.
For example in case of DP, the number of lanes (I guess 2 or 4) should
be ideally known if I have understood correctly. Knowledge about the
connection seems to also be needed, and I've so far seen some pretty
weird solutions for hotplug events with the DP AltMode. With the
driver we should be able to avoid those.

But in any case, every SVIDs a partner (or plug) offers will have
their own device registered with the partner (or cable) itself as
parent in this design. I'm expecting a little bit of conversation
about this plan, but right now I feel confident about it.

How does this sound to you?


Cheers,

-- 
heikki

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

#1399204

Hi,

On Wed, May 11, 2016 at 12:40:11PM +0300, Heikki Krogerus wrote:
> On Tue, May 10, 2016 at 08:14:34PM -0700, Guenter Roeck wrote:
> > Heikki,
> > 
> > On 05/06/2016 01:08 AM, Heikki Krogerus wrote:
> > > Hi,
> > > 
> > [ ... ]
> > > 
> > > I don't have not made any new code for the class driver yet, but I'm
> > > attempting to prepare v2 next week.
> > > 
> > Would it make sense to send feedback about v1 now, or should I wait for v2 ?
> 
> I don't think I'm able to send v2 today, or even tomorrow, so feel
> free to give the feedback. Just be aware that I've rewritten the
> alternate mode part completely.
> 
Alternate mode handling was my major concern, actually.

> I'm creating a separate device for the partner and also the cable
> during connection. I'm also already going to introduce a small bus for
> the AltModes. It's clear that we need to have AltMode specific
> drivers. The generic parts can't take care of all the AltMode specific
> requirements and VDMs. The bus will give us a nice way to bind those
> drivers to the actual AltModes a partner and the cable plugs offer.
> 
> So if there are dependencies between the altmodes, for example if the
> cable plugs needs to be in a certain mode in order for the partner to
> be able to function in some specific mode, the responsibility of
> taking care of those will fall primarily to in the AltMode drivers.
> So not userspace.
> 
> The AltMode drivers actually are useful also as they can be part of
> the relevant frameworks, for example DP in some graphics framework.
> For example in case of DP, the number of lanes (I guess 2 or 4) should
> be ideally known if I have understood correctly. Knowledge about the
> connection seems to also be needed, and I've so far seen some pretty
> weird solutions for hotplug events with the DP AltMode. With the
> driver we should be able to avoid those.
> 
> But in any case, every SVIDs a partner (or plug) offers will have
> their own device registered with the partner (or cable) itself as
> parent in this design. I'm expecting a little bit of conversation
> about this plan, but right now I feel confident about it.
> 
> How does this sound to you?
> 
Looking forward to it. My major problem so far was that alternate mode
handling is very platform specific, which didn't seem to be well supported
in v1 of your patch. I thought about implementing a hierarchy of drivers
below the type-c class to solve that problem. Looks like you just solved
it for me.

Other than that, my major concern is the lack of synchronization/protection
between the type-c class and the drivers. Setting port parameters (data role,
power role, operational power role, partner alternate modes, partner type)
from registered drivers may need to be synchronzed/protected. For example,
data and power role are set during connection establishment, but can be
overwritten from the typec class code. Right now I am just setting the
respective variables in struct typec_port directly, but that doesn't seem
right.

For partner_type, I don't really know how to map the options to the identity
reported by the partner. The reported product types are unknown / hub /
peripheral / passive cable / active cable / alternate mode adapter.
The available partner types are unknown / USB / Charger / Alternate Mode /
Audio Accessory / Debug Accessory. What am I missing here ?

The rest is just nitpicks.

- alternate_modes_show() and partner_alt_modes_show() discard the last byte
  of the generated string and replace it with \0.
- s/Accessroy/Accessory/
- typec_connect() and typec_disconnect() should probably also set
  port->connected.

Thanks,
Guenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web