usb:typec: Add sysfs support for Type C connector's physical location

PATCH Superseded Won Chung 3 participants 4 messages Feb 28, 2022

Subsystems (2)

USB USB Type-C class

lore

Share & tools

lore

Won Chung

author

2022-02-28 19:06 UTC #0 lore raw

PATCH 1/1 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
- horizontal_position
- vertical_position
- dock
- lid

Signed-off-by Won Chung <[email protected]>

---
 Documentation/ABI/testing/sysfs-class-typec | 43 +++++++++++++++++
 drivers/usb/typec/class.c                   | 52 +++++++++++++++++++++
 drivers/usb/typec/class.h                   |  3 ++
 3 files changed, 98 insertions(+)

Documentation/ABI/testing/sysfs-class-typec +43 -0

index 75088ecad202..2879bc6e6ad2 100644
@@ -141,6 +141,49 @@ Description:
 		- "reverse": CC2 orientation
 		- "unknown": Orientation cannot be determined.
 
+What:		/sys/class/typec/<port>/location/panel
+Date:		February 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		Describes which panel surface of the system’s housing the
+		Type C port resides on:
+		0 - Top
+		1 - Bottom
+		2 - Left
+		3 - Right
+		4 - Front
+		5 - Back
+		6 - Unknown (Vertical Position and Horizontal Position will be
+		ignored)
+
+What:		/sys/class/typec/<port>/location/vertical_position
+Date:		February 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		0 - Upper
+		1 - Center
+		2 - Lower
+
+What:		/sys/class/typec/<port>/location/horizontal_position
+Date:		Feb, 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		0 - Left
+		1 - Center
+		2 - Right
+
+What:		/sys/class/typec/<port>/location/dock
+Date:		Feb, 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		Set if the port resides in a docking station or a port replicator.
+
+What:		/sys/class/typec/<port>/location/lid
+Date:		Feb, 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		Set if the port resides on the lid of laptop system.
+
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
 What:		/sys/class/typec/<port>-partner/accessory_mode

drivers/usb/typec/class.c +52 -0

72 lines not on this page · raw message

drivers/usb/typec/class.h +3 -0

index 0f1bd6d19d67..1b52633400c8 100644
@@ -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)

Signature · 1 line

2.35.1.574.g5d30c73bfb-goog

Greg Kroah-Hartman

maintainer

2022-02-28 21:06 UTC #1 lore raw

Quoted 40 lines · Won Chung

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
- horizontal_position
- vertical_position
- dock
- lid
Signed-off-by Won Chung <[email protected]>
---
 Documentation/ABI/testing/sysfs-class-typec | 43 +++++++++++++++++
 drivers/usb/typec/class.c                   | 52 +++++++++++++++++++++
 drivers/usb/typec/class.h                   |  3 ++
 3 files changed, 98 insertions(+)
Documentation/ABI/testing/sysfs-class-typec +4 -0
index 75088ecad202..2879bc6e6ad2 100644
@@ -141,6 +141,49 @@ Description:
 		- "reverse": CC2 orientation
 		- "unknown": Orientation cannot be determined.
 
+What:		/sys/class/typec/<port>/location/panel
+Date:		February 2022
+Contact:	Won Chung <[email protected]>
+Description:

Documentation/ABI/testing/sysfs-class-typec Description: · line 157

Quoted 10 lines

Documentation/ABI/testing/sysfs-class-typec +10 -0

+		Describes which panel surface of the system’s housing the
+		Type C port resides on:
+		0 - Top
+		1 - Bottom
+		2 - Left
+		3 - Right
+		4 - Front
+		5 - Back
+		6 - Unknown (Vertical Position and Horizontal Position will be
+		ignored)

This is text files, why not say "top", "bottom", and so on? Why use a number that means nothing?

Documentation/ABI/testing/sysfs-class-typec Description: · line 165

Quoted 8 lines

Documentation/ABI/testing/sysfs-class-typec +8 -0

+
+What:		/sys/class/typec/<port>/location/vertical_position
+Date:		February 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		0 - Upper
+		1 - Center
+		2 - Lower

Same here.

Documentation/ABI/testing/sysfs-class-typec Description: · line 173

Quoted 8 lines

Documentation/ABI/testing/sysfs-class-typec +8 -0

+
+What:		/sys/class/typec/<port>/location/horizontal_position
+Date:		Feb, 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		0 - Left
+		1 - Center
+		2 - Right

And here.

Documentation/ABI/testing/sysfs-class-typec Description: · line 176

Quoted 3 lines

Documentation/ABI/testing/sysfs-class-typec +3 -0
+
+What:		/sys/class/typec/<port>/location/dock
+Date:		Feb, 2022

Note that date ends in a few hours :(

Documentation/ABI/testing/sysfs-class-typec Description: · line 185

Quoted 9 lines

Documentation/ABI/testing/sysfs-class-typec +9 -0

+Contact:	Won Chung <[email protected]>
+Description:
+		Set if the port resides in a docking station or a port replicator.
+
+What:		/sys/class/typec/<port>/location/lid
+Date:		Feb, 2022
+Contact:	Won Chung <[email protected]>
+Description:
+		Set if the port resides on the lid of laptop system.

"set"? What does that mean?

Quoted 50 lines

Documentation/ABI/testing/sysfs-class-typec +1 -0

+
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
 What:		/sys/class/typec/<port>-partner/accessory_mode

drivers/usb/typec/class.c +32 -0

index 45a6f0c807cb..43b23c221f95 100644
@@ -1579,8 +1579,40 @@ static const struct attribute_group typec_group = {
 	.attrs = typec_attrs,
 };
 
+#define DEV_ATTR_LOCATION_PROP(prop) \
+	static ssize_t prop##_show(struct device *dev, struct device_attribute *attr, \
+		char *buf) \
+	{ \
+		struct typec_port *port = to_typec_port(dev); \
+		if (port->pld) \
+			return sprintf(buf, "%u\n", port->pld->prop); \
+		return 0; \
+	}; \
+static DEVICE_ATTR_RO(prop)
+
+DEV_ATTR_LOCATION_PROP(panel);
+DEV_ATTR_LOCATION_PROP(vertical_position);
+DEV_ATTR_LOCATION_PROP(horizontal_position);
+DEV_ATTR_LOCATION_PROP(dock);
+DEV_ATTR_LOCATION_PROP(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
 };

drivers/usb/typec/class.c const struct device_type typec_port_dev_type = { · line 1649

Quoted 4 lines

drivers/usb/typec/class.c +1 -0

@@ -1614,6 +1646,24 @@ const struct device_type typec_port_dev_type = {
 	.release = typec_release,
 };
 
+void *get_pld(struct device *dev)

That is a horrible global function name :(

And why a void pointer? We have real types in the kernel, please use them.

drivers/usb/typec/class.c const struct device_type typec_port_dev_type = { · line 1651

Quoted 2 lines

drivers/usb/typec/class.c +2 -0
+{
+#ifdef CONFIG_ACPI

No #ifdefs in .c files please.

drivers/usb/typec/class.c const struct device_type typec_port_dev_type = { · line 1661

Quoted 10 lines

drivers/usb/typec/class.c +10 -0

+	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;

See, you return a real type, don't throw that information away. This isn't Windows :)

thanks,

gre gk-h

kernel test robot

build bot

2022-02-28 21:52 UTC #2 lore raw

In reply to Won Chung

Thank you for the patch! Perhaps something to improve:

Hi Won,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on usb/usb-testing]
[also build test WARNING on chrome-platform/for-next linus/master v5.17-rc6 next-20220228] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Won-Chung/usb-typec-Add-sysfs-support-for-Type-C-connector-s-physical-location/20220301-030738
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm-randconfig-c002-20220228 (https://download.01.org/0day-ci/archive/20220301/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/93d7a0fa1ed009ae6cc98fe5039cec8c9c77609f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Won-Chung/usb-typec-Add-sysfs-support-for-Type-C-connector-s-physical-location/20220301-030738 git checkout 93d7a0fa1ed009ae6cc98fe5039cec8c9c77609f
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash drivers/usb/typec/

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/usb/typec/class.c:1649:7: warning: no previous prototype for 'get_pld' [-Wmissing-prototypes]

1649 | void *get_pld(struct device *dev)
| ^~~~~~~

vim +/get_pld +1649 drivers/usb/typec/class.c

1648

> 1649	void *get_pld(struct device *dev)

1650 {
1651 #ifdef CONFIG_ACPI
1652 struct acpi_pld_info *pld;
1653 acpi_status status;
1654
1655 if (!has_acpi_companion(dev))
1656 return NULL;
1657
1658 status = acpi_get_physical_device_location(ACPI_HANDLE(dev), &pld);
1659 if (ACPI_FAILURE(status))
1660 return NULL;
1661 return pld;
1662 #else
1663 return NULL;
1664 #endif
1665 }
1666

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

kernel test robot

build bot

2022-02-28 22:53 UTC #3 lore raw

In reply to Won Chung

Thank you for the patch! Perhaps something to improve:

Hi Won,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Won-Chung/usb-typec-Add-sysfs-support-for-Type-C-connector-s-physical-location/20220301-030738
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: hexagon-randconfig-r041-20220227 (https://download.01.org/0day-ci/archive/20220301/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d271fc04d5b97b12e6b797c6067d3c96a8d7470e)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/93d7a0fa1ed009ae6cc98fe5039cec8c9c77609f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Won-Chung/usb-typec-Add-sysfs-support-for-Type-C-connector-s-physical-location/20220301-030738 git checkout 93d7a0fa1ed009ae6cc98fe5039cec8c9c77609f
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/usb/typec/

If you fix the issue, kindly add following tag as appropriate

Reported-by kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> drivers/usb/typec/class.c:1649:7: warning: no previous prototype for function 'get_pld' [-Wmissing-prototypes]
   void *get_pld(struct device *dev)
         ^

drivers/usb/typec/class.c:1649:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void *get_pld(struct device *dev)
^
static
1 warning generated.

vim +/get_pld +1649 drivers/usb/typec/class.c

1648

> 1649	void *get_pld(struct device *dev)

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]