| Message ID | 20220301022625.469446-1-wonchung@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] usb:typec: Add sysfs support for Type C connector's physical location | expand |
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 <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/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 <wonchung@google.com> > +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 <wonchung@google.com> > +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 <wonchung@google.com> > +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 <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 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,
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 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 <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/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 <wonchung@google.com> > > +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 <wonchung@google.com> > > +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 <wonchung@google.com> > > +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 <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 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/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
On Tue, Mar 1, 2022 at 7:57 PM Won Chung <wonchung@google.com> wrote: > > 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 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 <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/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 <wonchung@google.com> > > > +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 <wonchung@google.com> > > > +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 <wonchung@google.com> > > > +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 <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 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/.
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 <wonchung@google.com> +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 <wonchung@google.com> +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 <wonchung@google.com> +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 <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 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)
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 <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(+)