Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220301022625.469446-1-wonchung@google.com> <Yh3ofnlEx0bT/R6E@kuha.fi.intel.com>
In-Reply-To: <Yh3ofnlEx0bT/R6E@kuha.fi.intel.com>
From:   Won Chung <wonchung@google.com>
Date:   Tue, 1 Mar 2022 10:57:41 -0800
Message-ID: <CAOvb9yi4oi=KegV1d8MvxWPhStf5jL01CbdM_pAdLqKNAocJ1Q@mail.gmail.com>
Subject: Re: [PATCH v2] usb:typec: Add sysfs support for Type C connector's
 physical location
To:     Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J . Wysocki" <rafael@kernel.org>,
        Benson Leung <bleung@chromium.org>,
        Prashant Malani <pmalani@chromium.org>,
        linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Mar 1, 2022 at 1:33 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi Won,
>
> On Tue, Mar 01, 2022 at 02:26:25AM +0000, Won Chung wrote:
> > When ACPI table includes _PLD field for a Type C connector, share _PLD
> > values in its sysfs. _PLD stands for physical location of device.
> >
> > Currently without connector's location information, when there are
> > multiple Type C ports, it is hard to distinguish which connector
> > corresponds to which physical port at which location. For example, when
> > there are two Type C connectors, it is hard to find out which connector
> > corresponds to the Type C port on the left panel versus the Type C port
> > on the right panel. With location information provided, we can determin=
e
> > which specific device at which location is doing what.
> >
> > _PLD output includes much more fields, but only generic fields are adde=
d
> > and exposed to sysfs, so that non-ACPI devices can also support it in
> > the future. The minimal generic fields needed for locating a port are
> > the following.
> > - panel
> > - vertical_position
> > - horizontal_position
> > - dock
> > - lid
> >
> > Signed-off-by: Won Chung <wonchung@google.com>
> > ---
> >
> > Changes in v2:
> > - Use string for location.
> > - Clarify get_pld() with naming and return type.
> >
> >  Documentation/ABI/testing/sysfs-class-typec |  35 ++++++
> >  drivers/usb/typec/class.c                   | 113 ++++++++++++++++++++
> >  drivers/usb/typec/class.h                   |   3 +
> >  3 files changed, 151 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentatio=
n/ABI/testing/sysfs-class-typec
> > index 75088ecad202..4497a5aeb063 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -141,6 +141,41 @@ Description:
> >               - "reverse": CC2 orientation
> >               - "unknown": Orientation cannot be determined.
> >
> > +What:                /sys/class/typec/<port>/location/panel
> > +Date:                March 2022
> > +Contact:     Won Chung <wonchung@google.com>
> > +Description:
> > +             Describes which panel surface of the system=E2=80=99s hou=
sing the
> > +             port resides on.
> > +
> > +What:                /sys/class/typec/<port>/location/vertical_positio=
n
> > +Date:                March 2022
> > +Contact:     Won Chung <wonchung@google.com>
> > +Description:
> > +             Describes vertical position of the port on the panel surf=
ace.
> > +             Valid values: upper, center, lower
> > +
> > +What:                /sys/class/typec/<port>/location/horizontal_posit=
ion
> > +Date:                March 2022
> > +Contact:     Won Chung <wonchung@google.com>
> > +Description:
> > +             Describes horizontal position of the port on the panel su=
rface.
> > +             Valid values: left, center, right
> > +
> > +What:                /sys/class/typec/<port>/location/dock
> > +Date:                March 2022
> > +Contact:     Won Chung <wonchung@google.com>
> > +Description:
> > +             Set as "yes" if the port resides in a docking station or =
a port
> > +             replicator, otherwise set as "no".
> > +
> > +What:                /sys/class/typec/<port>/location/lid
> > +Date:                March 2022
> > +Contact:     Won Chung <wonchung@google.com>
> > +Description:
> > +             Set as "yes" if the port resides on the lid of laptop sys=
tem,
> > +             otherwise set as "no".
> > +
>
> I've probable lost track of the topic during my winter break, I'm
> sorry about that, but why are you proposing now that this should be
> made Type-C specific?
> This information is not Type-C specific, so it definitely does not
> belong here.
>
> Br,
>
> --
> heikki

Hi Heikki,

Thank you for the comment. Sorry that my description was not clear.
This is follow up from "[PATCH v6] ACPI: device_sysfs: Add sysfs
support for _PLD" in which Rafael suggested to have generic location
in Type C connector than adding PLD specifically to ACPI device. I was
also convinced by Rafael since userspace code would also be quite
ACPI-specific to access PLD in ACPI device sysfs. The discussion can
be found in https://lore.kernel.org/all/CAOvb9yh7uNg9ZU3RsieGChsjLCfKQhHhip=
Bi4RMuQYKEA4fu9A@mail.gmail.com/.
For your reference, in a diverged email thread, Greg also gave a
feedback: https://lore.kernel.org/all/Yg01TjB6DGYeMxjI@kroah.com/.

Do you think it is still better to have this location information in
ACPI device? I would appreciate it if you can share your thoughts in
the original thread where the discussion is. Thank you very much!

Won