All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] usb:typec: Add sysfs support for Type C connector's physical location
@ 2022-02-28 19:06 Won Chung
  2022-02-28 21:06 ` Greg Kroah-Hartman
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Won Chung @ 2022-02-28 19:06 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman, Rafael J . Wysocki,
	Benson Leung, Prashant Malani, linux-usb, linux-kernel
  Cc: 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 <wonchung@google.com>
---
 Documentation/ABI/testing/sysfs-class-typec | 43 +++++++++++++++++
 drivers/usb/typec/class.c                   | 52 +++++++++++++++++++++
 drivers/usb/typec/class.h                   |  3 ++
 3 files changed, 98 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 75088ecad202..2879bc6e6ad2 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -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 <wonchung@google.com>
+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 <wonchung@google.com>
+Description:
+		0 - Upper
+		1 - Center
+		2 - Lower
+
+What:		/sys/class/typec/<port>/location/horizontal_position
+Date:		Feb, 2022
+Contact:	Won Chung <wonchung@google.com>
+Description:
+		0 - Left
+		1 - Center
+		2 - Right
+
+What:		/sys/class/typec/<port>/location/dock
+Date:		Feb, 2022
+Contact:	Won Chung <wonchung@google.com>
+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 <wonchung@google.com>
+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
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 45a6f0c807cb..43b23c221f95 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -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
 };
 
@@ -1614,6 +1646,24 @@ const struct device_type typec_port_dev_type = {
 	.release = typec_release,
 };
 
+void *get_pld(struct device *dev)
+{
+#ifdef 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 +2123,8 @@ struct typec_port *typec_register_port(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
+	port->pld = get_pld(&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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb:typec: Add sysfs support for Type C connector's physical location
  2022-02-28 19:06 [PATCH] usb:typec: Add sysfs support for Type C connector's physical location Won Chung
@ 2022-02-28 21:06 ` Greg Kroah-Hartman
  2022-02-28 21:51 ` kernel test robot
  2022-02-28 22:52 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2022-02-28 21:06 UTC (permalink / raw)
  To: Won Chung
  Cc: Heikki Krogerus, Rafael J . Wysocki, Benson Leung,
	Prashant Malani, linux-usb, linux-kernel

On Mon, Feb 28, 2022 at 07:06:49PM +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
> - horizontal_position
> - vertical_position
> - dock
> - lid
> 
> Signed-off-by: Won Chung <wonchung@google.com>
> ---
>  Documentation/ABI/testing/sysfs-class-typec | 43 +++++++++++++++++
>  drivers/usb/typec/class.c                   | 52 +++++++++++++++++++++
>  drivers/usb/typec/class.h                   |  3 ++
>  3 files changed, 98 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index 75088ecad202..2879bc6e6ad2 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -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 <wonchung@google.com>
> +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)

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


> +
> +What:		/sys/class/typec/<port>/location/vertical_position
> +Date:		February 2022
> +Contact:	Won Chung <wonchung@google.com>
> +Description:
> +		0 - Upper
> +		1 - Center
> +		2 - Lower

Same here.


> +
> +What:		/sys/class/typec/<port>/location/horizontal_position
> +Date:		Feb, 2022
> +Contact:	Won Chung <wonchung@google.com>
> +Description:
> +		0 - Left
> +		1 - Center
> +		2 - Right

And here.

> +
> +What:		/sys/class/typec/<port>/location/dock
> +Date:		Feb, 2022

Note that date ends in a few hours :(

> +Contact:	Won Chung <wonchung@google.com>
> +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 <wonchung@google.com>
> +Description:
> +		Set if the port resides on the lid of laptop system.

"set"?  What does that mean?



> +
>  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..43b23c221f95 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -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
>  };
>  
> @@ -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.

> +{
> +#ifdef CONFIG_ACPI

No #ifdefs in .c files please.

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb:typec: Add sysfs support for Type C connector's physical location
  2022-02-28 19:06 [PATCH] usb:typec: Add sysfs support for Type C connector's physical location Won Chung
  2022-02-28 21:06 ` Greg Kroah-Hartman
@ 2022-02-28 21:51 ` kernel test robot
  2022-02-28 22:52 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-02-28 21:51 UTC (permalink / raw)
  To: Won Chung, Heikki Krogerus, Greg Kroah-Hartman,
	Rafael J . Wysocki, Benson Leung, Prashant Malani, linux-usb,
	linux-kernel
  Cc: kbuild-all, Won Chung

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/202203010527.LaNAQMXX-lkp@intel.com/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 <lkp@intel.com>

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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] usb:typec: Add sysfs support for Type C connector's physical location
  2022-02-28 19:06 [PATCH] usb:typec: Add sysfs support for Type C connector's physical location Won Chung
  2022-02-28 21:06 ` Greg Kroah-Hartman
  2022-02-28 21:51 ` kernel test robot
@ 2022-02-28 22:52 ` kernel test robot
  2 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2022-02-28 22:52 UTC (permalink / raw)
  To: Won Chung, Heikki Krogerus, Greg Kroah-Hartman,
	Rafael J . Wysocki, Benson Leung, Prashant Malani, linux-usb,
	linux-kernel
  Cc: llvm, kbuild-all, Won Chung

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: hexagon-randconfig-r041-20220227 (https://download.01.org/0day-ci/archive/20220301/202203010628.Aac5rBDB-lkp@intel.com/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 <lkp@intel.com>

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)
  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/kbuild-all@lists.01.org

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-02-28 22:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-28 19:06 [PATCH] usb:typec: Add sysfs support for Type C connector's physical location Won Chung
2022-02-28 21:06 ` Greg Kroah-Hartman
2022-02-28 21:51 ` kernel test robot
2022-02-28 22:52 ` kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.