On Wed, Oct 16, 2019 at 03:58:50PM +0300, Heikki Krogerus wrote:
> On Wed, Oct 16, 2019 at 11:43:14AM +0800, pumahsu wrote:
> > Export the Type-C cc orientation so that user space can
> > get this information.
>
> For what do you need this information in user space? I'm guessing you
> have something else in mind besides exposing this as just generic
> information, or debugging purposes, no?
>
> Please keep in mind that we do not always know the cable orientation.
> UCSI for example does not give any clues about which way the cable
> plug was connected to the connector. That means this sysfs file will
> most likely need to be hidden in those cases, which I guess is
> acceptable, but definitely not ideal.
>
> > Signed-off-by: pumahsu <pumahsu@xxxxxxxxxx>
> > ---
> > Documentation/ABI/testing/sysfs-class-typec | 7 +++++++
> > drivers/usb/typec/class.c | 11 +++++++++++
> > 2 files changed, 18 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index d7647b258c3c..419f952c991d 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -108,6 +108,13 @@ Contact: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> > Description:
> > Revision number of the supported USB Type-C specification.
> >
> > +What: /sys/class/typec/<port>/cc_orientation
> > +Date: September 2019
> > +Contact: Puma Hsu <pumahsu@xxxxxxxxxx>
> > +Description:
> > + Indicates which cc orientation is active now, or 0 when
> > + nothing is connected.
>
> cc_orientation is a bit cryptic. I think if this is part of the port
> ABI, then we should talk about something like "connector_orientation".
>
> > USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 7d8805d9bf37..00edae63da8e 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
> > }
> > static DEVICE_ATTR_RO(usb_power_delivery_revision);
> >
> > +static ssize_t cc_orientation_show(struct device *dev,
> > + struct device_attribute *attr,
> > + char *buf)
> > +{
> > + struct typec_port *p = to_typec_port(dev);
> > +
> > + return sprintf(buf, "%d\n", typec_get_orientation(p));
> > +}
> > +static DEVICE_ATTR_RO(cc_orientation);
>
> Now you are returning 0, 1 or 2 which to me is not ideal. This really
> should return a string, something like "normal" / "reversed", and in
> case the orientation is TYPEC_ORIENTATION_NONE, empty string.
Or maybe TYPEC_ORIENTATION_NONE could be handle with "unknown" string.
That way we may not need to hide the file.
thanks,
--
heikki
