Message-ID: <CAGCq0LZGz04JCTEJXrBqs4ENybQih6zKWTacq9T9DKPNOQAfMw@mail.gmail.com>
Date:   Thu, 24 Oct 2019 17:02:18 +0800
From:   Puma Hsu <pumahsu@...gle.com>
To:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc:     Guenter Roeck <linux@...ck-us.net>,
        Greg KH <gregkh@...uxfoundation.org>,
        Badhri Jagan Sridharan <badhri@...gle.com>,
        Kyle Tso <kyletso@...gle.com>,
        Albert Wang <albertccwang@...gle.com>,
        Chien Kun Niu <rickyniu@...gle.com>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] usb: typec: Add sysfs node to show connector orientation

Yes, generally this might be purely informational or be a dynamically
debuggable
mechanism for end user as I mentioned in previous discussion
thread(https://lkml.org/lkml/2019/10/22/198).
Could I know if it is not suitable that we expose a file for
informational usage?


If everyone agreed above, about the definition of “unknown” and the condition
“don’t know the orientation”, what about adding additional return value?
  1. For original “unknown”, it is a generic unknown state which can
indicate no
      matter connector is disconnected, cannot specify which cc side
is configured(such as Ra-Ra),
      or even driver can not know the orientation.
  2. New additional value “unavailable”, it can be used to
specifically explicate that
      driver can not know the orientation.
Take UCSI as example, it can use generic “unknown” or “unavailable” if
it wants.
But if it exposes “unavailable”, then application in user space can
know that this attribute is not useful.



I summarize the proposal definition below:
 - unknown (generic unknown. driver don’t or can’t know the polarity,
                      e.g. disconnected, both cc1 and cc2 are the same, )
 - normal (configured in cc1 side)
 - reversed (configured in cc2 side)
 - unavailable (not support the polarity detection)


Thanks in advance.
Puma Hsu


On Wed, Oct 23, 2019 at 11:58 PM Heikki Krogerus
<heikki.krogerus@...ux.intel.com> wrote:
>
> 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@...gle.com>
> > > > > > ---
> > > > > >   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@...ux.intel.com>
> > > > > >   Description:
> > > > > >               Revision number of the supported USB Type-C specification.
> > > > > > +What:                /sys/class/typec/<port>/connector_orientation
> > > > > > +Date:                October 2019
> > > > > > +Contact:     Puma Hsu <pumahsu@...gle.com>
> > > > > > +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

Powered by blists - more mailing lists