2022-03-01 04:20:01

by Won Chung

[permalink] [raw]
Subject: [PATCH v2] usb:typec: Add sysfs support for Type C connector's physical location

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 determine
which specific device at which location is doing what.

_PLD output includes much more fields, but only generic fields are added
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 <[email protected]>
---

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/Documentation/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 <[email protected]>
+Description:
+ Describes which panel surface of the system’s housing the
+ port resides on.
+
+What: /sys/class/typec/<port>/location/vertical_position
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ Describes vertical position of the port on the panel surface.
+ Valid values: upper, center, lower
+
+What: /sys/class/typec/<port>/location/horizontal_position
+Date: March 2022
+Contact: Won Chung <[email protected]>
+Description:
+ Describes horizontal position of the port on the panel surface.
+ Valid values: left, center, right
+
+What: /sys/class/typec/<port>/location/dock
+Date: March 2022
+Contact: Won Chung <[email protected]>
+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 <[email protected]>
+Description:
+ Set as "yes" if the port resides on the lid of laptop system,
+ otherwise set as "no".
+
USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)

What: /sys/class/typec/<port>-partner/accessory_mode
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 45a6f0c807cb..5a52cb6728ec 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1579,8 +1579,101 @@ static const struct attribute_group typec_group = {
.attrs = typec_attrs,
};

+static const char * const typec_location_panel[] = {
+ "top",
+ "bottom",
+ "left",
+ "right",
+ "front",
+ "back",
+ "unknown",
+};
+
+static const char * const typec_location_vertical_position[] = {
+ "upper", "center", "lower"
+};
+
+static const char * const typec_location_horizontal_position[] = {
+ "left", "center", "right"
+};
+
+static ssize_t panel_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct typec_port *port = to_typec_port(dev);
+
+ if (port->pld)
+ return sprintf(buf, "%s\n",
+ typec_location_panel[port->pld->panel]);
+ return 0;
+};
+static DEVICE_ATTR_RO(panel);
+
+static ssize_t vertical_position_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct typec_port *port = to_typec_port(dev);
+
+ if (port->pld)
+ return sprintf(buf, "%s\n",
+ typec_location_vertical_position[
+ port->pld->vertical_position]);
+ return 0;
+};
+static DEVICE_ATTR_RO(vertical_position);
+
+static ssize_t horizontal_position_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct typec_port *port = to_typec_port(dev);
+
+ if (port->pld)
+ return sprintf(buf, "%s\n",
+ typec_location_horizontal_position[
+ port->pld->horizontal_position]);
+ return 0;
+};
+static DEVICE_ATTR_RO(horizontal_position);
+
+static ssize_t dock_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct typec_port *port = to_typec_port(dev);
+
+ if (port->pld)
+ return sprintf(buf, "%s\n", port->pld->dock ? "yes" : "no");
+ return 0;
+};
+static DEVICE_ATTR_RO(dock);
+
+static ssize_t lid_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct typec_port *port = to_typec_port(dev);
+
+ if (port->pld)
+ return sprintf(buf, "%s\n", port->pld->lid ? "yes" : "no");
+ return 0;
+};
+static DEVICE_ATTR_RO(lid);
+
+static struct attribute *typec_location_attrs[] = {
+ &dev_attr_panel.attr,
+ &dev_attr_vertical_position.attr,
+ &dev_attr_horizontal_position.attr,
+ &dev_attr_dock.attr,
+ &dev_attr_lid.attr,
+ NULL,
+};
+
+static const struct attribute_group typec_location_group = {
+ .name = "location",
+ .attrs = typec_location_attrs,
+};
+
static const struct attribute_group *typec_groups[] = {
&typec_group,
+ &typec_location_group,
NULL
};

@@ -1614,6 +1707,24 @@ const struct device_type typec_port_dev_type = {
.release = typec_release,
};

+static struct acpi_pld_info *get_acpi_pld_info(struct device *dev)
+{
+#if defined(CONFIG_ACPI)
+ struct acpi_pld_info *pld;
+ acpi_status status;
+
+ if (!has_acpi_companion(dev))
+ return NULL;
+
+ status = acpi_get_physical_device_location(ACPI_HANDLE(dev), &pld);
+ if (ACPI_FAILURE(status))
+ return NULL;
+ return pld;
+#else
+ return NULL;
+#endif
+}
+
/* --------------------------------------- */
/* Driver callbacks to report role updates */

@@ -2073,6 +2184,8 @@ struct typec_port *typec_register_port(struct device *parent,
return ERR_PTR(ret);
}

+ port->pld = get_acpi_pld_info(&port->dev);
+
ret = device_add(&port->dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index 0f1bd6d19d67..1b52633400c8 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -3,6 +3,7 @@
#ifndef __USB_TYPEC_CLASS__
#define __USB_TYPEC_CLASS__

+#include <linux/acpi.h>
#include <linux/device.h>
#include <linux/usb/typec.h>

@@ -54,6 +55,8 @@ struct typec_port {

const struct typec_capability *cap;
const struct typec_operations *ops;
+
+ struct acpi_pld_info *pld;
};

#define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
--
2.35.1.574.g5d30c73bfb-goog


2022-03-01 20:21:27

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH v2] usb:typec: Add sysfs support for Type C connector's physical location

On Tue, Mar 1, 2022 at 7:57 PM Won Chung <[email protected]> wrote:
>
> On Tue, Mar 1, 2022 at 1:33 AM Heikki Krogerus
> <[email protected]> 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 determine
> > > which specific device at which location is doing what.
> > >
> > > _PLD output includes much more fields, but only generic fields are added
> > > 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 <[email protected]>
> > > ---
> > >
> > > 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/Documentation/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 <[email protected]>
> > > +Description:
> > > + Describes which panel surface of the system’s housing the
> > > + port resides on.
> > > +
> > > +What: /sys/class/typec/<port>/location/vertical_position
> > > +Date: March 2022
> > > +Contact: Won Chung <[email protected]>
> > > +Description:
> > > + Describes vertical position of the port on the panel surface.
> > > + Valid values: upper, center, lower
> > > +
> > > +What: /sys/class/typec/<port>/location/horizontal_position
> > > +Date: March 2022
> > > +Contact: Won Chung <[email protected]>
> > > +Description:
> > > + Describes horizontal position of the port on the panel surface.
> > > + Valid values: left, center, right
> > > +
> > > +What: /sys/class/typec/<port>/location/dock
> > > +Date: March 2022
> > > +Contact: Won Chung <[email protected]>
> > > +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 <[email protected]>
> > > +Description:
> > > + Set as "yes" if the port resides on the lid of laptop system,
> > > + 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.

Well, this doesn't have to be /sys/class/typec/<port>/location/ though.

For example, the device location information can be exposed in a more
generic way is /sys/devices/.../location/ for all devices for which it
is available, somewhat in analogy to /sys/devices/.../power/.

2022-03-01 20:26:54

by Won Chung

[permalink] [raw]
Subject: Re: [PATCH v2] usb:typec: Add sysfs support for Type C connector's physical location

On Tue, Mar 1, 2022 at 1:33 AM Heikki Krogerus
<[email protected]> 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 determine
> > which specific device at which location is doing what.
> >
> > _PLD output includes much more fields, but only generic fields are added
> > 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 <[email protected]>
> > ---
> >
> > 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/Documentation/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 <[email protected]>
> > +Description:
> > + Describes which panel surface of the system’s housing the
> > + port resides on.
> > +
> > +What: /sys/class/typec/<port>/location/vertical_position
> > +Date: March 2022
> > +Contact: Won Chung <[email protected]>
> > +Description:
> > + Describes vertical position of the port on the panel surface.
> > + Valid values: upper, center, lower
> > +
> > +What: /sys/class/typec/<port>/location/horizontal_position
> > +Date: March 2022
> > +Contact: Won Chung <[email protected]>
> > +Description:
> > + Describes horizontal position of the port on the panel surface.
> > + Valid values: left, center, right
> > +
> > +What: /sys/class/typec/<port>/location/dock
> > +Date: March 2022
> > +Contact: Won Chung <[email protected]>
> > +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 <[email protected]>
> > +Description:
> > + Set as "yes" if the port resides on the lid of laptop system,
> > + 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/CAOvb9yh7uNg9ZU3RsieGChsjLCfKQhHhipBi4RMuQYKEA4fu9A@mail.gmail.com/.
For your reference, in a diverged email thread, Greg also gave a
feedback: https://lore.kernel.org/all/[email protected]/.

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

2022-03-01 21:05:55

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2] usb:typec: Add sysfs support for Type C connector's physical location

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 determine
> which specific device at which location is doing what.
>
> _PLD output includes much more fields, but only generic fields are added
> 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 <[email protected]>
> ---
>
> 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/Documentation/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 <[email protected]>
> +Description:
> + Describes which panel surface of the system’s housing the
> + port resides on.
> +
> +What: /sys/class/typec/<port>/location/vertical_position
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + Describes vertical position of the port on the panel surface.
> + Valid values: upper, center, lower
> +
> +What: /sys/class/typec/<port>/location/horizontal_position
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +Description:
> + Describes horizontal position of the port on the panel surface.
> + Valid values: left, center, right
> +
> +What: /sys/class/typec/<port>/location/dock
> +Date: March 2022
> +Contact: Won Chung <[email protected]>
> +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 <[email protected]>
> +Description:
> + Set as "yes" if the port resides on the lid of laptop system,
> + 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

2022-03-02 23:47:17

by Heikki Krogerus

[permalink] [raw]
Subject: Re: [PATCH v2] usb:typec: Add sysfs support for Type C connector's physical location

On Tue, Mar 01, 2022 at 08:11:06PM +0100, Rafael J. Wysocki wrote:
> On Tue, Mar 1, 2022 at 7:57 PM Won Chung <[email protected]> wrote:
> >
> > On Tue, Mar 1, 2022 at 1:33 AM Heikki Krogerus
> > <[email protected]> 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 determine
> > > > which specific device at which location is doing what.
> > > >
> > > > _PLD output includes much more fields, but only generic fields are added
> > > > 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 <[email protected]>
> > > > ---
> > > >
> > > > 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/Documentation/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 <[email protected]>
> > > > +Description:
> > > > + Describes which panel surface of the system’s housing the
> > > > + port resides on.
> > > > +
> > > > +What: /sys/class/typec/<port>/location/vertical_position
> > > > +Date: March 2022
> > > > +Contact: Won Chung <[email protected]>
> > > > +Description:
> > > > + Describes vertical position of the port on the panel surface.
> > > > + Valid values: upper, center, lower
> > > > +
> > > > +What: /sys/class/typec/<port>/location/horizontal_position
> > > > +Date: March 2022
> > > > +Contact: Won Chung <[email protected]>
> > > > +Description:
> > > > + Describes horizontal position of the port on the panel surface.
> > > > + Valid values: left, center, right
> > > > +
> > > > +What: /sys/class/typec/<port>/location/dock
> > > > +Date: March 2022
> > > > +Contact: Won Chung <[email protected]>
> > > > +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 <[email protected]>
> > > > +Description:
> > > > + Set as "yes" if the port resides on the lid of laptop system,
> > > > + 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.
>
> Well, this doesn't have to be /sys/class/typec/<port>/location/ though.
>
> For example, the device location information can be exposed in a more
> generic way is /sys/devices/.../location/ for all devices for which it
> is available, somewhat in analogy to /sys/devices/.../power/.

Right, that's what I meant. These can be made generic.

thanks,

--
heikki

2022-03-03 18:37:27

by Won Chung

[permalink] [raw]
Subject: Re: [PATCH v2] usb:typec: Add sysfs support for Type C connector's physical location

On Wed, Mar 2, 2022 at 3:21 AM Heikki Krogerus
<[email protected]> wrote:
>
> On Tue, Mar 01, 2022 at 08:11:06PM +0100, Rafael J. Wysocki wrote:
> > On Tue, Mar 1, 2022 at 7:57 PM Won Chung <[email protected]> wrote:
> > >
> > > On Tue, Mar 1, 2022 at 1:33 AM Heikki Krogerus
> > > <[email protected]> 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 determine
> > > > > which specific device at which location is doing what.
> > > > >
> > > > > _PLD output includes much more fields, but only generic fields are added
> > > > > 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 <[email protected]>
> > > > > ---
> > > > >
> > > > > 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/Documentation/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 <[email protected]>
> > > > > +Description:
> > > > > + Describes which panel surface of the system’s housing the
> > > > > + port resides on.
> > > > > +
> > > > > +What: /sys/class/typec/<port>/location/vertical_position
> > > > > +Date: March 2022
> > > > > +Contact: Won Chung <[email protected]>
> > > > > +Description:
> > > > > + Describes vertical position of the port on the panel surface.
> > > > > + Valid values: upper, center, lower
> > > > > +
> > > > > +What: /sys/class/typec/<port>/location/horizontal_position
> > > > > +Date: March 2022
> > > > > +Contact: Won Chung <[email protected]>
> > > > > +Description:
> > > > > + Describes horizontal position of the port on the panel surface.
> > > > > + Valid values: left, center, right
> > > > > +
> > > > > +What: /sys/class/typec/<port>/location/dock
> > > > > +Date: March 2022
> > > > > +Contact: Won Chung <[email protected]>
> > > > > +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 <[email protected]>
> > > > > +Description:
> > > > > + Set as "yes" if the port resides on the lid of laptop system,
> > > > > + 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.
> >
> > Well, this doesn't have to be /sys/class/typec/<port>/location/ though.
> >
> > For example, the device location information can be exposed in a more
> > generic way is /sys/devices/.../location/ for all devices for which it
> > is available, somewhat in analogy to /sys/devices/.../power/.
>
> Right, that's what I meant. These can be made generic.

> thanks,
>
> --
> heikki

Hi Heikki and Rafael,

Thank you for clarification and guidance. I created and sent a new
patch on driver core for /sys/devices/.../location/.

Won