From: Guenter Roeck <linux@roeck-us.net>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Mathias Nyman <mathias.nyman@linux.intel.com>,
Felipe Balbi <balbi@kernel.org>
Subject: Re: [PATCH 0/3] usb: USB Type-C Class and driver for UCSI
Date: Fri, 13 May 2016 10:48:10 -0700 [thread overview]
Message-ID: <20160513174810.GA1999@roeck-us.net> (raw)
In-Reply-To: <20160513142321.GA21576@kuha.fi.intel.com>
Hi,
On Fri, May 13, 2016 at 05:23:21PM +0300, Heikki Krogerus wrote:
> Hi,
>
> On Wed, May 11, 2016 at 07:47:18AM -0700, Guenter Roeck wrote:
> > 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.
>
> I'm actually struggling with this same question. I decided to protect
> the whole struct typec_port by not allowing the drivers to touch it,
> but I'm still working on it..
>
Sounds good to me, as long as you provide APIs to change the values.
> > 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 partner types don't map directly to the USB PD Product Types. We
> need to describe the partner even when USB PD is not available.
> Accessory Modes are for example out side the scope of USB PD.
>
> But I'll try to propose something for those.
>
Ok.
> > 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.
>
> OK. I'll check these.
>
> So I failed to finish my proposal for v2 this week. On top of the
> sync/protection problems, I'm also still trying to tune my AltMode
> bus. I'm going to attempt to send it as an RFC on Monday or Tuesday in
> any case.
>
No worries. It turns out the PD code in existing type-c devices isn't
exactly stable or predictable, so I end up spending a lot of time
trying to stabilize my state machine anyway.
Thanks,
Guenter
prev parent reply other threads:[~2016-05-13 17:48 UTC|newest]
Thread overview: 90+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-09 17:01 [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Heikki Krogerus
2016-02-09 17:01 ` [PATCH 1/3] usb: USB Type-C Connector Class Heikki Krogerus
2016-02-09 18:20 ` Greg KH
2016-02-10 10:38 ` Heikki Krogerus
2016-02-10 17:26 ` Greg KH
2016-02-11 14:07 ` Heikki Krogerus
2016-02-10 10:49 ` Oliver Neukum
2016-02-10 11:05 ` Andy Shevchenko
2016-02-10 11:11 ` Heikki Krogerus
2016-02-10 11:14 ` Andy Shevchenko
2016-02-10 11:23 ` Heikki Krogerus
2016-02-15 15:16 ` Oliver Neukum
2016-02-11 8:55 ` Felipe Balbi
2016-02-11 9:08 ` Oliver Neukum
2016-02-11 14:51 ` Heikki Krogerus
2016-02-11 14:36 ` Heikki Krogerus
2016-02-11 14:56 ` Oliver Neukum
2016-02-17 14:07 ` Oliver Neukum
2016-02-18 8:47 ` Heikki Krogerus
2016-02-18 9:21 ` Oliver Neukum
2016-02-18 13:09 ` Heikki Krogerus
2016-02-18 9:35 ` Oliver Neukum
2016-02-18 13:25 ` Heikki Krogerus
2016-02-18 13:44 ` Oliver Neukum
2016-02-18 15:13 ` Heikki Krogerus
2016-02-26 13:09 ` Heikki Krogerus
2016-02-09 17:01 ` [PATCH 2/3] usb: type-c: USB Type-C Connector System Software Interface Heikki Krogerus
2016-02-09 18:21 ` Greg KH
2016-02-10 10:30 ` Heikki Krogerus
2016-02-10 17:20 ` Greg KH
2016-02-11 13:50 ` Heikki Krogerus
2016-02-15 15:30 ` Oliver Neukum
2016-02-16 9:22 ` Heikki Krogerus
2016-02-16 13:39 ` Oliver Neukum
2016-02-17 7:58 ` Heikki Krogerus
2016-02-17 9:03 ` Oliver Neukum
2016-02-17 10:29 ` Felipe Balbi
2016-02-17 10:36 ` Oliver Neukum
2016-02-17 11:11 ` Heikki Krogerus
2016-02-17 13:36 ` Felipe Balbi
2016-02-17 14:28 ` Heikki Krogerus
2016-02-18 9:07 ` Peter Chen
2016-02-18 10:44 ` Heikki Krogerus
2016-02-18 10:37 ` Rajaram R
2016-02-18 10:47 ` Heikki Krogerus
2016-02-18 11:06 ` Rajaram R
2016-02-17 13:34 ` Felipe Balbi
2016-02-17 13:51 ` Oliver Neukum
2016-02-18 7:08 ` Felipe Balbi
2016-02-18 10:18 ` Oliver Neukum
2016-02-18 10:30 ` Felipe Balbi
2016-02-18 10:40 ` Oliver Neukum
2016-02-18 9:29 ` Peter Chen
2016-02-18 9:44 ` Oliver Neukum
2016-02-10 11:19 ` Oliver Neukum
2016-02-10 12:04 ` Heikki Krogerus
2016-02-10 11:56 ` Andy Shevchenko
2016-02-10 13:21 ` Oliver Neukum
2016-02-10 14:02 ` Andy Shevchenko
2016-02-10 15:11 ` Bjørn Mork
2016-02-11 8:26 ` Andy Shevchenko
2016-02-11 8:59 ` Bjørn Mork
2016-02-10 14:15 ` Oliver Neukum
2016-02-10 14:24 ` Andy Shevchenko
2016-02-10 15:08 ` Oliver Neukum
[not found] ` <CAHp75VfmGsskf7Cmni3b4=tCbkPsR8d3jPYiv93Lm6DM9gq1-g@mail.gmail.com>
2016-02-11 8:13 ` Fwd: " Andy Shevchenko
2016-02-11 14:10 ` Heikki Krogerus
2016-02-10 13:04 ` Oliver Neukum
2016-02-11 14:08 ` Heikki Krogerus
2016-02-09 17:01 ` [PATCH 3/3] usb: type-c: UCSI ACPI driver Heikki Krogerus
2016-02-09 18:22 ` Greg KH
2016-02-10 10:23 ` Heikki Krogerus
2016-02-17 18:53 ` [PATCH 0/3] usb: USB Type-C Class and driver for UCSI Oliver Neukum
2016-02-18 9:21 ` Heikki Krogerus
2016-02-17 19:34 ` Rajaram R
2016-02-18 11:05 ` Heikki Krogerus
2016-02-18 11:15 ` Oliver Neukum
2016-05-05 3:05 ` Guenter Roeck
2016-05-06 6:50 ` Felipe Balbi
2016-05-06 8:05 ` Guenter Roeck
2016-05-06 8:29 ` Heikki Krogerus
2016-05-06 14:10 ` Guenter Roeck
2016-05-06 8:23 ` Heikki Krogerus
2016-05-06 8:08 ` Heikki Krogerus
2016-05-06 14:08 ` Guenter Roeck
2016-05-11 3:14 ` Guenter Roeck
2016-05-11 9:40 ` Heikki Krogerus
2016-05-11 14:47 ` Guenter Roeck
2016-05-13 14:23 ` Heikki Krogerus
2016-05-13 17:48 ` Guenter Roeck [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160513174810.GA1999@roeck-us.net \
--to=linux@roeck-us.net \
--cc=balbi@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mathias.nyman@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).