[PATCH 0/2] platform/chrome: typec: xHCI DbC

[PATCH 0/2] platform/chrome: typec: xHCI DbC

[Heikki Krogerus]

Hi,

In order to use xHCI DbC we need to allow the USB to be muxed to xHCI
even when the connector is in device role. That's because in DbC mode
the xHCI is the USB device controller.

In the first patch I'm just adding symlinks between the USB role
switches and their USB Type-C connectors. That way the user space can
find the correct role switch simply by following the symlink.

The second patch modifies cros_ec_typec.c. I'm assigning the PLD
(Physical Location of Device) hash of the port to the USB role switch
when it's missing from the ACPI tables. That should make sure the
first patch always works.


Heikki Krogerus (2):
  usb: roles: Link the switch to its connector
  platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

 .../ABI/testing/sysfs-class-usb_role          |  6 +++
 drivers/platform/chrome/cros_ec_typec.c       | 11 +++++
 drivers/usb/roles/class.c                     | 40 ++++++++++++++++++-
 3 files changed, 55 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH 1/2] usb: roles: Link the switch to its connector

[Heikki Krogerus]

This is probable useful information to have in user space in
general, but it's primarily needed for the xHCI DbC (Debug
Capability). When xHCI DbC is being used, the USB port needs
to be muxed to the xHCI even in device role. In xHCI DbC mode,
the xHCI is the USB device controller.

Tested-by: Uday Bhat <uday.m.bhat@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 .../ABI/testing/sysfs-class-usb_role          |  6 +++
 drivers/usb/roles/class.c                     | 40 ++++++++++++++++++-
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-usb_role b/Documentation/ABI/testing/sysfs-class-usb_role
index 3b810a425a52..9fab3f06679e 100644
--- a/Documentation/ABI/testing/sysfs-class-usb_role
+++ b/Documentation/ABI/testing/sysfs-class-usb_role
@@ -19,3 +19,9 @@ Description:
 		- none
 		- host
 		- device
+
+What:		/sys/class/usb_role/<switch>/connector
+Date:		Feb 2024
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Optional symlink to the USB Type-C connector.
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index ae41578bd014..4ad03c93c17f 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -7,6 +7,7 @@
  *         Hans de Goede <hdegoede@redhat.com>
  */
 
+#include <linux/component.h>
 #include <linux/usb/role.h>
 #include <linux/property.h>
 #include <linux/device.h>
@@ -34,6 +35,32 @@ struct usb_role_switch {
 
 #define to_role_switch(d)	container_of(d, struct usb_role_switch, dev)
 
+static int connector_bind(struct device *dev, struct device *connector, void *data)
+{
+	int ret;
+
+	ret = sysfs_create_link(&dev->kobj, &connector->kobj, "connector");
+	if (ret)
+		return ret;
+
+	ret = sysfs_create_link(&connector->kobj, &dev->kobj, "usb-role-switch");
+	if (ret)
+		sysfs_remove_link(&dev->kobj, "connector");
+
+	return ret;
+}
+
+static void connector_unbind(struct device *dev, struct device *connector, void *data)
+{
+	sysfs_remove_link(&connector->kobj, "usb-role-switch");
+	sysfs_remove_link(&dev->kobj, "connector");
+}
+
+static const struct component_ops connector_ops = {
+	.bind = connector_bind,
+	.unbind = connector_unbind,
+};
+
 /**
  * usb_role_switch_set_role - Set USB role for a switch
  * @sw: USB role switch
@@ -352,6 +379,12 @@ usb_role_switch_register(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
+	if (dev_fwnode(&sw->dev)) {
+		ret = component_add(&sw->dev, &connector_ops);
+		if (ret)
+			dev_warn(&sw->dev, "failed to add component\n");
+	}
+
 	/* TODO: Symlinks for the host port and the device controller. */
 
 	return sw;
@@ -366,8 +399,11 @@ EXPORT_SYMBOL_GPL(usb_role_switch_register);
  */
 void usb_role_switch_unregister(struct usb_role_switch *sw)
 {
-	if (!IS_ERR_OR_NULL(sw))
-		device_unregister(&sw->dev);
+	if (IS_ERR_OR_NULL(sw))
+		return;
+	if (dev_fwnode(&sw->dev))
+		component_del(&sw->dev, &connector_ops);
+	device_unregister(&sw->dev);
 }
 EXPORT_SYMBOL_GPL(usb_role_switch_unregister);
 
-- 
2.43.0

[PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

[Heikki Krogerus]

The USB role switch does not always have the _PLD (Physical
Location of Device) in ACPI tables. If it's missing,
assigning the PLD hash of the port to the switch. That
should guarantee that the USB Type-C port mapping code is
always able to find the connection between the two (the port
and the switch).

Tested-by: Uday Bhat <uday.m.bhat@intel.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 2b2f14a1b711..5c14e8db08b5 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -28,6 +28,7 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
 				       struct fwnode_handle *fwnode,
 				       struct device *dev)
 {
+	struct fwnode_handle *sw_fwnode;
 	const char *buf;
 	int ret;
 
@@ -66,6 +67,16 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
 		cap->prefer_role = ret;
 	}
 
+	/* Assing the USB role switch the correct pld_crc if it's missing. */
+	sw_fwnode = fwnode_find_reference(fwnode, "usb-role-switch", 0);
+	if (!IS_ERR_OR_NULL(sw_fwnode)) {
+		struct acpi_device *adev = to_acpi_device_node(sw_fwnode);
+
+		if (adev && !adev->pld_crc)
+			adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
+		fwnode_handle_put(sw_fwnode);
+	}
+
 	cap->fwnode = fwnode;
 
 	return 0;
-- 
2.43.0

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

[Sergei Shtylyov]

On 2/7/24 5:58 PM, Heikki Krogerus wrote:

> The USB role switch does not always have the _PLD (Physical
> Location of Device) in ACPI tables. If it's missing,
> assigning the PLD hash of the port to the switch. That
> should guarantee that the USB Type-C port mapping code is
> always able to find the connection between the two (the port
> and the switch).
> 
> Tested-by: Uday Bhat <uday.m.bhat@intel.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 2b2f14a1b711..5c14e8db08b5 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
[...]
> @@ -66,6 +67,16 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
>  		cap->prefer_role = ret;
>  	}
>  
> +	/* Assing the USB role switch the correct pld_crc if it's missing. */

   Doing what?! :-P Maybe assign?

[...]

MBR, Sergey

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

[kernel test robot]

Hi Heikki,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus chrome-platform/for-next chrome-platform/for-firmware-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.8-rc3 next-20240207]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Heikki-Krogerus/usb-roles-Link-the-switch-to-its-connector/20240207-230017
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20240207145851.1603237-3-heikki.krogerus%40linux.intel.com
patch subject: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD
config: arm-defconfig (https://download.01.org/0day-ci/archive/20240208/202402080600.zOq5UvYq-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240208/202402080600.zOq5UvYq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402080600.zOq5UvYq-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/platform/chrome/cros_ec_typec.c:75:20: error: incomplete definition of type 'struct acpi_device'
                   if (adev && !adev->pld_crc)
                                ~~~~^
   include/linux/acpi.h:795:8: note: forward declaration of 'struct acpi_device'
   struct acpi_device;
          ^
   drivers/platform/chrome/cros_ec_typec.c:76:8: error: incomplete definition of type 'struct acpi_device'
                           adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
                           ~~~~^
   include/linux/acpi.h:795:8: note: forward declaration of 'struct acpi_device'
   struct acpi_device;
          ^
   drivers/platform/chrome/cros_ec_typec.c:76:47: error: incomplete definition of type 'struct acpi_device'
                           adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
                                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~^
   include/linux/acpi.h:795:8: note: forward declaration of 'struct acpi_device'
   struct acpi_device;
          ^
   3 errors generated.


vim +75 drivers/platform/chrome/cros_ec_typec.c

    23	
    24	#define DP_PORT_VDO	(DP_CONF_SET_PIN_ASSIGN(BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D)) | \
    25					DP_CAP_DFP_D | DP_CAP_RECEPTACLE)
    26	
    27	static int cros_typec_parse_port_props(struct typec_capability *cap,
    28					       struct fwnode_handle *fwnode,
    29					       struct device *dev)
    30	{
    31		struct fwnode_handle *sw_fwnode;
    32		const char *buf;
    33		int ret;
    34	
    35		memset(cap, 0, sizeof(*cap));
    36		ret = fwnode_property_read_string(fwnode, "power-role", &buf);
    37		if (ret) {
    38			dev_err(dev, "power-role not found: %d\n", ret);
    39			return ret;
    40		}
    41	
    42		ret = typec_find_port_power_role(buf);
    43		if (ret < 0)
    44			return ret;
    45		cap->type = ret;
    46	
    47		ret = fwnode_property_read_string(fwnode, "data-role", &buf);
    48		if (ret) {
    49			dev_err(dev, "data-role not found: %d\n", ret);
    50			return ret;
    51		}
    52	
    53		ret = typec_find_port_data_role(buf);
    54		if (ret < 0)
    55			return ret;
    56		cap->data = ret;
    57	
    58		/* Try-power-role is optional. */
    59		ret = fwnode_property_read_string(fwnode, "try-power-role", &buf);
    60		if (ret) {
    61			dev_warn(dev, "try-power-role not found: %d\n", ret);
    62			cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
    63		} else {
    64			ret = typec_find_power_role(buf);
    65			if (ret < 0)
    66				return ret;
    67			cap->prefer_role = ret;
    68		}
    69	
    70		/* Assing the USB role switch the correct pld_crc if it's missing. */
    71		sw_fwnode = fwnode_find_reference(fwnode, "usb-role-switch", 0);
    72		if (!IS_ERR_OR_NULL(sw_fwnode)) {
    73			struct acpi_device *adev = to_acpi_device_node(sw_fwnode);
    74	
  > 75			if (adev && !adev->pld_crc)
    76				adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
    77			fwnode_handle_put(sw_fwnode);
    78		}
    79	
    80		cap->fwnode = fwnode;
    81	
    82		return 0;
    83	}
    84	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

[kernel test robot]

Hi Heikki,

kernel test robot noticed the following build errors:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on usb/usb-next usb/usb-linus chrome-platform/for-next chrome-platform/for-firmware-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.8-rc3 next-20240207]
[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#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Heikki-Krogerus/usb-roles-Link-the-switch-to-its-connector/20240207-230017
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link:    https://lore.kernel.org/r/20240207145851.1603237-3-heikki.krogerus%40linux.intel.com
patch subject: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD
config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240208/202402081136.sve0cViZ-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240208/202402081136.sve0cViZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202402081136.sve0cViZ-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/platform/chrome/cros_ec_typec.c: In function 'cros_typec_parse_port_props':
>> drivers/platform/chrome/cros_ec_typec.c:75:34: error: invalid use of undefined type 'struct acpi_device'
      75 |                 if (adev && !adev->pld_crc)
         |                                  ^~
   drivers/platform/chrome/cros_ec_typec.c:76:29: error: invalid use of undefined type 'struct acpi_device'
      76 |                         adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
         |                             ^~
   drivers/platform/chrome/cros_ec_typec.c:76:68: error: invalid use of undefined type 'struct acpi_device'
      76 |                         adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
         |                                                                    ^~


vim +75 drivers/platform/chrome/cros_ec_typec.c

    23	
    24	#define DP_PORT_VDO	(DP_CONF_SET_PIN_ASSIGN(BIT(DP_PIN_ASSIGN_C) | BIT(DP_PIN_ASSIGN_D)) | \
    25					DP_CAP_DFP_D | DP_CAP_RECEPTACLE)
    26	
    27	static int cros_typec_parse_port_props(struct typec_capability *cap,
    28					       struct fwnode_handle *fwnode,
    29					       struct device *dev)
    30	{
    31		struct fwnode_handle *sw_fwnode;
    32		const char *buf;
    33		int ret;
    34	
    35		memset(cap, 0, sizeof(*cap));
    36		ret = fwnode_property_read_string(fwnode, "power-role", &buf);
    37		if (ret) {
    38			dev_err(dev, "power-role not found: %d\n", ret);
    39			return ret;
    40		}
    41	
    42		ret = typec_find_port_power_role(buf);
    43		if (ret < 0)
    44			return ret;
    45		cap->type = ret;
    46	
    47		ret = fwnode_property_read_string(fwnode, "data-role", &buf);
    48		if (ret) {
    49			dev_err(dev, "data-role not found: %d\n", ret);
    50			return ret;
    51		}
    52	
    53		ret = typec_find_port_data_role(buf);
    54		if (ret < 0)
    55			return ret;
    56		cap->data = ret;
    57	
    58		/* Try-power-role is optional. */
    59		ret = fwnode_property_read_string(fwnode, "try-power-role", &buf);
    60		if (ret) {
    61			dev_warn(dev, "try-power-role not found: %d\n", ret);
    62			cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
    63		} else {
    64			ret = typec_find_power_role(buf);
    65			if (ret < 0)
    66				return ret;
    67			cap->prefer_role = ret;
    68		}
    69	
    70		/* Assing the USB role switch the correct pld_crc if it's missing. */
    71		sw_fwnode = fwnode_find_reference(fwnode, "usb-role-switch", 0);
    72		if (!IS_ERR_OR_NULL(sw_fwnode)) {
    73			struct acpi_device *adev = to_acpi_device_node(sw_fwnode);
    74	
  > 75			if (adev && !adev->pld_crc)
    76				adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
    77			fwnode_handle_put(sw_fwnode);
    78		}
    79	
    80		cap->fwnode = fwnode;
    81	
    82		return 0;
    83	}
    84	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

[Heikki Krogerus]

On Thu, Feb 08, 2024 at 12:07:40PM +0800, kernel test robot wrote:
> Hi Heikki,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on usb/usb-testing]
> [also build test ERROR on usb/usb-next usb/usb-linus chrome-platform/for-next chrome-platform/for-firmware-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.8-rc3 next-20240207]
> [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#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Heikki-Krogerus/usb-roles-Link-the-switch-to-its-connector/20240207-230017
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> patch link:    https://lore.kernel.org/r/20240207145851.1603237-3-heikki.krogerus%40linux.intel.com
> patch subject: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD
> config: m68k-allmodconfig (https://download.01.org/0day-ci/archive/20240208/202402081136.sve0cViZ-lkp@intel.com/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240208/202402081136.sve0cViZ-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202402081136.sve0cViZ-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/platform/chrome/cros_ec_typec.c: In function 'cros_typec_parse_port_props':
> >> drivers/platform/chrome/cros_ec_typec.c:75:34: error: invalid use of undefined type 'struct acpi_device'
>       75 |                 if (adev && !adev->pld_crc)
>          |                                  ^~
>    drivers/platform/chrome/cros_ec_typec.c:76:29: error: invalid use of undefined type 'struct acpi_device'
>       76 |                         adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
>          |                             ^~
>    drivers/platform/chrome/cros_ec_typec.c:76:68: error: invalid use of undefined type 'struct acpi_device'
>       76 |                         adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
>          |                                                                    ^~

Oh, so this has to be wrapped in ifdef CONFIG_ACPI :(

-- 
heikki

Re: [PATCH 2/2] platform/chrome: cros_ec_typec: Make sure the USB role switch has PLD

[Prashant Malani]

Hi Heikki,

On Wed, Feb 7, 2024 at 6:59 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> The USB role switch does not always have the _PLD (Physical
> Location of Device) in ACPI tables. If it's missing,
> assigning the PLD hash of the port to the switch. That
> should guarantee that the USB Type-C port mapping code is
> always able to find the connection between the two (the port
> and the switch).
>
> Tested-by: Uday Bhat <uday.m.bhat@intel.com>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  drivers/platform/chrome/cros_ec_typec.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
> index 2b2f14a1b711..5c14e8db08b5 100644
> --- a/drivers/platform/chrome/cros_ec_typec.c
> +++ b/drivers/platform/chrome/cros_ec_typec.c
> @@ -28,6 +28,7 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
>                                        struct fwnode_handle *fwnode,
>                                        struct device *dev)
>  {
> +       struct fwnode_handle *sw_fwnode;
>         const char *buf;
>         int ret;
>
> @@ -66,6 +67,16 @@ static int cros_typec_parse_port_props(struct typec_capability *cap,
>                 cap->prefer_role = ret;
>         }
>
> +       /* Assing the USB role switch the correct pld_crc if it's missing. */
> +       sw_fwnode = fwnode_find_reference(fwnode, "usb-role-switch", 0);
> +       if (!IS_ERR_OR_NULL(sw_fwnode)) {
> +               struct acpi_device *adev = to_acpi_device_node(sw_fwnode);
> +
> +               if (adev && !adev->pld_crc)
> +                       adev->pld_crc = to_acpi_device_node(fwnode)->pld_crc;
> +               fwnode_handle_put(sw_fwnode);
Can this be in common Type-C code (maybe typec_register_port())?
It doesn't strike me as ChromeOS specific, but perhaps I am missing something.

Thanks,