Re: [PATCH V2] usb: typec: Add sysfs node to show connector orientation

[Heikki Krogerus]

On Wed, Oct 23, 2019 at 08:01:26AM -0700, Guenter Roeck wrote:
> On Wed, Oct 23, 2019 at 05:29:00PM +0300, Heikki Krogerus wrote:
> > On Wed, Oct 23, 2019 at 06:44:39AM -0700, Guenter Roeck wrote:
> > > On 10/23/19 1:32 AM, Heikki Krogerus wrote:
> > > > +Guenter
> > > > 
> > > > On Tue, Oct 22, 2019 at 04:59:24PM +0800, Puma Hsu wrote:
> > > > > Export the Type-C connector orientation so that user space
> > > > > can get this information.
> > > > > 
> > > > > Signed-off-by: Puma Hsu <pumahsu@xxxxxxxxxx>
> > > > > ---
> > > > >   Documentation/ABI/testing/sysfs-class-typec | 11 +++++++++++
> > > > >   drivers/usb/typec/class.c                   | 18 ++++++++++++++++++
> > > > >   2 files changed, 29 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > > > index d7647b258c3c..b22f71801671 100644
> > > > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > > > @@ -108,6 +108,17 @@ Contact:	Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > > > >   Description:
> > > > >   		Revision number of the supported USB Type-C specification.
> > > > > +What:		/sys/class/typec/<port>/connector_orientation
> > > > > +Date:		October 2019
> > > > > +Contact:	Puma Hsu <pumahsu@xxxxxxxxxx>
> > > > > +Description:
> > > > > +		Indicates which typec connector orientation is configured now.
> > > > > +		cc1 is defined as "normal" and cc2 is defined as "reversed".
> > > > > +
> > > > > +		Valid value:
> > > > > +		- unknown (nothing configured)
> > > > 
> > > > "unknown" means we do not know the orientation.
> > > > 
> > > > > +		- normal (configured in cc1 side)
> > > > > +		- reversed (configured in cc2 side)
> > > > 
> > > > Guenter, do you think "connector_orientation" OK. I proposed it, but
> > > > I'm now wondering if something like "polarity" would be better?
> > > > 
> > > 
> > > Yes, or just "orientation". I don't see the value in the "connector_" prefix.
> > > I also wonder if "unknown" is really correct. Is it really unknown, or
> > > does it mean that the port is disconnected ?
> > 
> > Unknown means we don't know the orientation. We don't always have that
> > information available to us. With UCSI we simply do not know it.
> > 
> > I think this file needs to be hidden after all if we don't know the
> > cable plug orientation.
> > 
> Making the attribute appear and disappear may cause difficulties for
> userspace.
> 
> > How about empty string instead of "unknown"?
> > 
> An empty string might also be challenging for userspace.
> 
> "unknown" is fine if it is really unknown.

That's what I was thinking, but I realised that since the value may be
"unknown" even when the driver is able to tell the cable plug
orientation, there is no way for the userspace to know is the driver
able to supply the information or not. That is why I say the attribute
has to be hidden in cases where the driver really does not know the
orientation (like UCSI).

I'm really not a big fan of hidden attribute files, as they do make
things unpredictable for the userspace, but with information like
this, either we simply do not provide it to the userspace at all -
option that I'm all for if there is no real need for this - or we
hide the file with drivers that can not supply the information.

> With that in mind, I wonder what value that attribute has for
> userspace, but presumably there must be some use case. I assume it
> is purely informational.

Puma actually already answered to this one:
https://lkml.org/lkml/2019/10/22/198

If I understood correctly, it would be purely informational. Puma,
please correct me if I'm wrong!

But if that is the case, and it is purely informational, then I don't
think we should add this attribute at all.


thanks,

-- 
heikki