All of lore.kernel.org
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	Oliver Neukum <oneukum@suse.com>,
	Felipe Balbi <felipe.balbi@linux.intel.com>,
	Roger Quadros <rogerq@ti.com>,
	Rajaram R <rajaram.officemail@gmail.com>,
	linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCHv4 1/2] usb: USB Type-C connector class
Date: Fri, 1 Jul 2016 07:41:40 -0700	[thread overview]
Message-ID: <20160701144140.GA15242@roeck-us.net> (raw)
In-Reply-To: <20160701073803.GB21901@kuha.fi.intel.com>

On Fri, Jul 01, 2016 at 10:38:03AM +0300, Heikki Krogerus wrote:
> On Thu, Jun 30, 2016 at 10:10:25AM -0700, Guenter Roeck wrote:
> > On Wed, Jun 29, 2016 at 04:38:37PM +0300, Heikki Krogerus wrote:
> > > The purpose of USB Type-C connector class is to provide
> > > unified interface for the user space to get the status and
> > > basic information about USB Type-C connectors on a system,
> > > control over data role swapping, and when the port supports
> > > USB Power Delivery, also control over power role swapping
> > > and Alternate Modes.
> > > 
> > > Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > ---
> > [ ... ]
> > 
> > > +
> > > +What:		/sys/class/typec/<port>-partner/supports_usb_power_delivery
> > > +Date:		June 2016
> > > +Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > +Description:
> > > +		Shows if the partner supports USB Power Delivery.
> > > +		- 1 if USB Power Delivery is supported
> > > +		- 0 when it's not
> > > +
> > > +
> > > +What:		/sys/class/typec/<port>-partner/id_header_vdo
> > > +Date:		June 2016
> > > +Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > > +Description:
> > > +		If the partner supports USB Power Deliver, shows the VDO
> > > +		returned from Discover Identity USB Power Delivery command.
> > > +
> > > +		If the partner does not support USB Power Delivery, the
> > > +		attribute is hidden.
> > > +
> > On second thought, and after merging the code (and realizing that I don't get
> > the raw data from the Type-C Port Manager), I am not sure if a raw attribute
> > is that useful here. It also doesn't provide all information either.
> 
> Yeah, I don't think it's available with UCSI either..
> 
> > Would it make sense to split it into multiple decoded attributes ?
> > 
> > - vendor-id
> >   [bit 0..15 of ID header VDO]
> > - product-type (undefined, hub, peripheral, alternate mode adapter for ufp;
> > 	passive/active for cable plugs)
> >   Might map into typec_partner_type, but I don't see a 1:1 match.
> >   [bit 27..29 of ID header VDO]
> > - alternate-mode-supported
> >   [bit 26 of ID header VDO]
> > - capabilities (ufp, dfp, drp, none (?))
> >   [bit 30/31 of ID header VDO]
> > - product-id
> >   [bit 16..31 of Product VDO]
> > 
> > Does this make any sense ?
> 
> I feel a bit uncomfortable exposing so many attribute like that which
> will give details that we can only know when both ends support USB
> PD...
> 
> Here's my proposal for this:
> There has to be a special group for these devices, partners and
> cables, a directory named "pd" or "power_deliver" (or something like
> that), which exposes those. That group will not be responsibility of
> this class, but instead the PD framework that you are working on
> (right?).
> 
> So basically, in this class we will not expose those attributes, and
> we'll also get rid of the "supports_usb_power_delivery" attribute (the
> "pd" groups should only exist when USB PD is actually supported).
> 
> Initially that group needs to be assigned to the "groups" member of
> struct device of the partners in the drivers before they are
> registered. That member is meant for optional attribute groups, but we
> can later think of something better, a way perhaps to bind that group
> the device types "groups" member for partners, cables, etc. or
> something similar in case using the "groups" member of struct device
> is not ideal. But initially we can use that.
> 
> How would that sound to you?
> 
Quite frankly I don't know. At first glance it sounds like a bad idea - it means
that PD specific atttributes would not be standardized, which would reduce the
valus of the class code substantially. I'll have to think about this.

At the same time I'll have to find a somewhat working solution for the locking
issues. I'll have to focus on that for the time being, because it affects
the core architecture of our code. So I don't really have the time to look
into attribute support right now.

Guenter

  reply	other threads:[~2016-07-01 14:42 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-29 13:38 [PATCHv4 0/2] USB Type-C Connector class Heikki Krogerus
2016-06-29 13:38 ` [PATCHv4 1/2] usb: USB Type-C connector class Heikki Krogerus
2016-06-30 17:10   ` Guenter Roeck
2016-07-01  7:38     ` Heikki Krogerus
2016-07-01 14:41       ` Guenter Roeck [this message]
2016-06-30 22:02   ` Guenter Roeck
2016-07-01  7:13     ` Heikki Krogerus
2016-07-01 12:05       ` Heikki Krogerus
2016-07-01 14:33         ` Guenter Roeck
2016-07-03 19:38           ` Heikki Krogerus
2016-07-03 21:28             ` Guenter Roeck
2016-07-04 17:11               ` Heikki Krogerus
2016-07-04 17:45                 ` Guenter Roeck
2016-07-05 18:47                   ` Heikki Krogerus
2016-06-29 13:38 ` [PATCHv4 2/2] usb: typec: add driver for Intel Whiskey Cove PMIC USB Type-C PHY Heikki Krogerus
2016-08-09 14:01 ` [PATCHv4 0/2] USB Type-C Connector class Greg KH
2016-08-09 16:23   ` Guenter Roeck
2016-08-10  8:19     ` Oliver Neukum
2016-08-16  7:38       ` Heikki Krogerus
2016-08-16 16:40         ` Guenter Roeck
2016-08-17  7:44           ` Heikki Krogerus
2016-08-10  9:20     ` Felipe Balbi

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=20160701144140.GA15242@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=felipe.balbi@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=heikki.krogerus@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=oneukum@suse.com \
    --cc=rajaram.officemail@gmail.com \
    --cc=rogerq@ti.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 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.