* [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops
@ 2025-07-09 13:22 Radu Vele
2025-07-09 13:22 ` [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port Radu Vele
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Radu Vele @ 2025-07-09 13:22 UTC (permalink / raw)
To: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies,
Andrei Kuchynski, Tzung-Bi Shih
Cc: chrome-platform, linux-kernel
From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Add the pr_set and dr_set typec_operations to registered typec ports.
This enables sysfs to control power and data role when the port is
capable of doing so.
Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Co-developed-by: Radu Vele <raduvele@google.com>
Signed-off-by: Radu Vele <raduvele@google.com>
---
drivers/platform/chrome/cros_ec_typec.c | 77 ++++++++++++++++++++++++-
1 file changed, 76 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 7678e3d05fd3..289429ef959f 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -58,8 +58,83 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m
&req, sizeof(req), NULL, 0);
}
+static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type)
+{
+ struct cros_typec_port *port = typec_get_drvdata(tc_port);
+ struct cros_typec_data *data = port->typec_data;
+ struct ec_response_usb_pd_control_v2 resp;
+ struct ec_params_usb_pd_control req;
+ int role, ret;
+
+ /* Must be at least v1 to support role swap. */
+ if (!data->pd_ctrl_ver)
+ return -EOPNOTSUPP;
+
+ /* First query the state */
+ req.port = port->port_num;
+ req.role = USB_PD_CTRL_ROLE_NO_CHANGE;
+ req.mux = USB_PD_CTRL_MUX_NO_CHANGE;
+ req.swap = USB_PD_CTRL_SWAP_NONE;
+
+ ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL,
+ &req, sizeof(req), &resp, sizeof(resp));
+ if (ret < 0)
+ return ret;
+
+ switch (swap_type) {
+ case USB_PD_CTRL_SWAP_DATA:
+ role = (resp.role & PD_CTRL_RESP_ROLE_DATA) ? TYPEC_HOST :
+ TYPEC_DEVICE;
+ break;
+ case USB_PD_CTRL_SWAP_POWER:
+ role = (resp.role & PD_CTRL_RESP_ROLE_POWER) ? TYPEC_SOURCE :
+ TYPEC_SINK;
+ break;
+ default:
+ dev_warn(data->dev, "Unsupported role swap type %d", swap_type);
+ return -EOPNOTSUPP;
+ }
+
+ if (role == target_role)
+ return 0;
+
+ req.swap = swap_type;
+ ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL,
+ &req, sizeof(req), &resp, sizeof(resp));
+
+ if (ret < 0)
+ return ret;
+
+ switch (swap_type) {
+ case USB_PD_CTRL_SWAP_DATA:
+ typec_set_data_role(tc_port, resp.role & PD_CTRL_RESP_ROLE_DATA ?
+ TYPEC_HOST :
+ TYPEC_DEVICE);
+ break;
+ case USB_PD_CTRL_SWAP_POWER:
+ typec_set_pwr_role(tc_port, resp.role & PD_CTRL_RESP_ROLE_POWER ?
+ TYPEC_SOURCE :
+ TYPEC_SINK);
+ break;
+ }
+
+ return 0;
+}
+
+static int cros_typec_dr_swap(struct typec_port *port, enum typec_data_role role)
+{
+ return cros_typec_perform_role_swap(port, role, USB_PD_CTRL_SWAP_DATA);
+}
+
+static int cros_typec_pr_swap(struct typec_port *port, enum typec_role role)
+{
+ return cros_typec_perform_role_swap(port, role, USB_PD_CTRL_SWAP_POWER);
+}
+
static const struct typec_operations cros_typec_usb_mode_ops = {
- .enter_usb_mode = cros_typec_enter_usb_mode
+ .enter_usb_mode = cros_typec_enter_usb_mode,
+ .dr_set = cros_typec_dr_swap,
+ .pr_set = cros_typec_pr_swap
};
static int cros_typec_parse_port_props(struct typec_capability *cap,
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port 2025-07-09 13:22 [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Radu Vele @ 2025-07-09 13:22 ` Radu Vele 2025-07-09 18:54 ` Benson Leung 2025-07-10 4:06 ` Tzung-Bi Shih 2025-07-09 18:45 ` [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Benson Leung 2025-07-10 4:06 ` Tzung-Bi Shih 2 siblings, 2 replies; 6+ messages in thread From: Radu Vele @ 2025-07-09 13:22 UTC (permalink / raw) To: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies, Andrei Kuchynski, Tzung-Bi Shih Cc: chrome-platform, linux-kernel Add a lock associated to each port to protect port data against concurrent access. Concurrency may result from sysfs commands and ec events. Signed-off-by: Radu Vele <raduvele@google.com> --- drivers/platform/chrome/cros_ec_typec.c | 38 +++++++++++++++++++------ drivers/platform/chrome/cros_ec_typec.h | 3 ++ 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c index 289429ef959f..bbb9e02624b8 100644 --- a/drivers/platform/chrome/cros_ec_typec.c +++ b/drivers/platform/chrome/cros_ec_typec.c @@ -8,6 +8,7 @@ #include <linux/acpi.h> #include <linux/module.h> +#include <linux/mutex.h> #include <linux/of.h> #include <linux/platform_data/cros_ec_commands.h> #include <linux/platform_data/cros_usbpd_notify.h> @@ -54,8 +55,11 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m .mode_to_enter = CROS_EC_ALTMODE_USB4 }; - return cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL, + mutex_lock(&port->lock); + int ret = cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL, &req, sizeof(req), NULL, 0); + mutex_unlock(&port->lock); + return ret; } static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type) @@ -70,6 +74,8 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r if (!data->pd_ctrl_ver) return -EOPNOTSUPP; + mutex_lock(&port->lock); + /* First query the state */ req.port = port->port_num; req.role = USB_PD_CTRL_ROLE_NO_CHANGE; @@ -79,7 +85,7 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, &req, sizeof(req), &resp, sizeof(resp)); if (ret < 0) - return ret; + goto unlock; switch (swap_type) { case USB_PD_CTRL_SWAP_DATA: @@ -92,18 +98,21 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r break; default: dev_warn(data->dev, "Unsupported role swap type %d", swap_type); - return -EOPNOTSUPP; + ret = -EOPNOTSUPP; + goto unlock; } - if (role == target_role) - return 0; + if (role == target_role) { + ret = 0; + goto unlock; + } req.swap = swap_type; ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, &req, sizeof(req), &resp, sizeof(resp)); if (ret < 0) - return ret; + goto unlock; switch (swap_type) { case USB_PD_CTRL_SWAP_DATA: @@ -117,8 +126,11 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r TYPEC_SINK); break; } + ret = 0; - return 0; +unlock: + mutex_unlock(&port->lock); + return ret; } static int cros_typec_dr_swap(struct typec_port *port, enum typec_data_role role) @@ -370,6 +382,7 @@ static void cros_unregister_ports(struct cros_typec_data *typec) if (!typec->ports[i]) continue; + mutex_lock(&typec->ports[i]->lock); cros_typec_remove_partner(typec, i); cros_typec_remove_cable(typec, i); @@ -378,6 +391,8 @@ static void cros_unregister_ports(struct cros_typec_data *typec) typec_mux_put(typec->ports[i]->mux); cros_typec_unregister_port_altmodes(typec->ports[i]); typec_unregister_port(typec->ports[i]->port); + mutex_unlock(&typec->ports[i]->lock); + mutex_destroy(&typec->ports[i]->lock); } } @@ -472,6 +487,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec) goto unregister_ports; } + mutex_init(&cros_port->lock); cros_port->port_num = port_num; cros_port->typec_data = typec; typec->ports[port_num] = cros_port; @@ -1232,6 +1248,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) return -EINVAL; } + mutex_lock(&typec->ports[port_num]->lock); req.port = port_num; req.role = USB_PD_CTRL_ROLE_NO_CHANGE; req.mux = USB_PD_CTRL_MUX_NO_CHANGE; @@ -1241,7 +1258,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) EC_CMD_USB_PD_CONTROL, &req, sizeof(req), &resp, sizeof(resp)); if (ret < 0) - return ret; + goto unlock; /* Update the switches if they exist, according to requested state */ ret = cros_typec_configure_mux(typec, port_num, &resp); @@ -1263,7 +1280,10 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) if (typec->typec_cmd_supported) cros_typec_handle_status(typec, port_num); - return 0; + ret = 0; +unlock: + mutex_unlock(&typec->ports[port_num]->lock); + return ret; } static int cros_typec_get_cmd_version(struct cros_typec_data *typec) diff --git a/drivers/platform/chrome/cros_ec_typec.h b/drivers/platform/chrome/cros_ec_typec.h index f9c31f04c102..d0a8a12ec91a 100644 --- a/drivers/platform/chrome/cros_ec_typec.h +++ b/drivers/platform/chrome/cros_ec_typec.h @@ -82,6 +82,9 @@ struct cros_typec_port { struct usb_power_delivery_capabilities *partner_sink_caps; struct cros_typec_data *typec_data; + + /* Mutex to protect port data against concurrent access */ + struct mutex lock; }; #endif /* __CROS_EC_TYPEC__ */ -- 2.50.0.727.gbf7dc18ff4-goog ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port 2025-07-09 13:22 ` [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port Radu Vele @ 2025-07-09 18:54 ` Benson Leung 2025-07-10 4:06 ` Tzung-Bi Shih 1 sibling, 0 replies; 6+ messages in thread From: Benson Leung @ 2025-07-09 18:54 UTC (permalink / raw) To: Radu Vele Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies, Andrei Kuchynski, Tzung-Bi Shih, chrome-platform, linux-kernel Hi Radu, On Wed, Jul 09, 2025 at 01:22:32PM +0000, Radu Vele wrote: > Add a lock associated to each port to protect port data against > concurrent access. Concurrency may result from sysfs commands > and ec events. > > Signed-off-by: Radu Vele <raduvele@google.com> > --- > drivers/platform/chrome/cros_ec_typec.c | 38 +++++++++++++++++++------ > drivers/platform/chrome/cros_ec_typec.h | 3 ++ > 2 files changed, 32 insertions(+), 9 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > index 289429ef959f..bbb9e02624b8 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -8,6 +8,7 @@ > > #include <linux/acpi.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/of.h> > #include <linux/platform_data/cros_ec_commands.h> > #include <linux/platform_data/cros_usbpd_notify.h> > @@ -54,8 +55,11 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m > .mode_to_enter = CROS_EC_ALTMODE_USB4 > }; > > - return cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL, > + mutex_lock(&port->lock); > + int ret = cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL, > &req, sizeof(req), NULL, 0); Please declare "int ret" at the top of the block, as per other examples of it in this file. You have it in the middle of a block right now. > + mutex_unlock(&port->lock); > + return ret; > } > > static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type) > @@ -70,6 +74,8 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r > if (!data->pd_ctrl_ver) > return -EOPNOTSUPP; > > + mutex_lock(&port->lock); > + > /* First query the state */ > req.port = port->port_num; > req.role = USB_PD_CTRL_ROLE_NO_CHANGE; > @@ -79,7 +85,7 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r > ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, > &req, sizeof(req), &resp, sizeof(resp)); > if (ret < 0) > - return ret; > + goto unlock; > > switch (swap_type) { > case USB_PD_CTRL_SWAP_DATA: > @@ -92,18 +98,21 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r > break; > default: > dev_warn(data->dev, "Unsupported role swap type %d", swap_type); > - return -EOPNOTSUPP; > + ret = -EOPNOTSUPP; > + goto unlock; > } > > - if (role == target_role) > - return 0; > + if (role == target_role) { > + ret = 0; > + goto unlock; > + } > > req.swap = swap_type; > ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, > &req, sizeof(req), &resp, sizeof(resp)); > > if (ret < 0) > - return ret; > + goto unlock; > > switch (swap_type) { > case USB_PD_CTRL_SWAP_DATA: > @@ -117,8 +126,11 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r > TYPEC_SINK); > break; > } > + ret = 0; > > - return 0; > +unlock: > + mutex_unlock(&port->lock); > + return ret; > } > > static int cros_typec_dr_swap(struct typec_port *port, enum typec_data_role role) > @@ -370,6 +382,7 @@ static void cros_unregister_ports(struct cros_typec_data *typec) > if (!typec->ports[i]) > continue; > > + mutex_lock(&typec->ports[i]->lock); > cros_typec_remove_partner(typec, i); > cros_typec_remove_cable(typec, i); > > @@ -378,6 +391,8 @@ static void cros_unregister_ports(struct cros_typec_data *typec) > typec_mux_put(typec->ports[i]->mux); > cros_typec_unregister_port_altmodes(typec->ports[i]); > typec_unregister_port(typec->ports[i]->port); > + mutex_unlock(&typec->ports[i]->lock); > + mutex_destroy(&typec->ports[i]->lock); > } > } > > @@ -472,6 +487,7 @@ static int cros_typec_init_ports(struct cros_typec_data *typec) > goto unregister_ports; > } > > + mutex_init(&cros_port->lock); > cros_port->port_num = port_num; > cros_port->typec_data = typec; > typec->ports[port_num] = cros_port; > @@ -1232,6 +1248,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) > return -EINVAL; > } > > + mutex_lock(&typec->ports[port_num]->lock); > req.port = port_num; > req.role = USB_PD_CTRL_ROLE_NO_CHANGE; > req.mux = USB_PD_CTRL_MUX_NO_CHANGE; > @@ -1241,7 +1258,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) > EC_CMD_USB_PD_CONTROL, &req, sizeof(req), > &resp, sizeof(resp)); > if (ret < 0) > - return ret; > + goto unlock; > > /* Update the switches if they exist, according to requested state */ > ret = cros_typec_configure_mux(typec, port_num, &resp); > @@ -1263,7 +1280,10 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) > if (typec->typec_cmd_supported) > cros_typec_handle_status(typec, port_num); > > - return 0; > + ret = 0; > +unlock: > + mutex_unlock(&typec->ports[port_num]->lock); > + return ret; > } > > static int cros_typec_get_cmd_version(struct cros_typec_data *typec) > diff --git a/drivers/platform/chrome/cros_ec_typec.h b/drivers/platform/chrome/cros_ec_typec.h > index f9c31f04c102..d0a8a12ec91a 100644 > --- a/drivers/platform/chrome/cros_ec_typec.h > +++ b/drivers/platform/chrome/cros_ec_typec.h > @@ -82,6 +82,9 @@ struct cros_typec_port { > struct usb_power_delivery_capabilities *partner_sink_caps; > > struct cros_typec_data *typec_data; > + > + /* Mutex to protect port data against concurrent access */ > + struct mutex lock; > }; > > #endif /* __CROS_EC_TYPEC__ */ > -- > 2.50.0.727.gbf7dc18ff4-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port 2025-07-09 13:22 ` [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port Radu Vele 2025-07-09 18:54 ` Benson Leung @ 2025-07-10 4:06 ` Tzung-Bi Shih 1 sibling, 0 replies; 6+ messages in thread From: Tzung-Bi Shih @ 2025-07-10 4:06 UTC (permalink / raw) To: Radu Vele Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies, Andrei Kuchynski, chrome-platform, linux-kernel On Wed, Jul 09, 2025 at 01:22:32PM +0000, Radu Vele wrote: > @@ -8,6 +8,7 @@ > > #include <linux/acpi.h> > #include <linux/module.h> > +#include <linux/mutex.h> > #include <linux/of.h> > #include <linux/platform_data/cros_ec_commands.h> > #include <linux/platform_data/cros_usbpd_notify.h> Please consider to include linux/cleanup.h and use guard() to simplify the patch. > @@ -54,8 +55,11 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m > .mode_to_enter = CROS_EC_ALTMODE_USB4 > }; > > - return cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL, > + mutex_lock(&port->lock); > + int ret = cros_ec_cmd(port->typec_data->ec, 0, EC_CMD_TYPEC_CONTROL, > &req, sizeof(req), NULL, 0); > + mutex_unlock(&port->lock); > + return ret; E.g.: guard(mutex)(&port->lock); return cros_ec_cmd(...); > static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type) > @@ -70,6 +74,8 @@ static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_r > if (!data->pd_ctrl_ver) > return -EOPNOTSUPP; > > + mutex_lock(&port->lock); It can drop all the following diff hunks in the function if using: guard(mutex)(&port->lock); > @@ -1232,6 +1248,7 @@ static int cros_typec_port_update(struct cros_typec_data *typec, int port_num) > return -EINVAL; > } > > + mutex_lock(&typec->ports[port_num]->lock); It can drop all the following diff hunks in the function if using: guard(mutex)(&typec->ports[port_num]->lock); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops 2025-07-09 13:22 [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Radu Vele 2025-07-09 13:22 ` [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port Radu Vele @ 2025-07-09 18:45 ` Benson Leung 2025-07-10 4:06 ` Tzung-Bi Shih 2 siblings, 0 replies; 6+ messages in thread From: Benson Leung @ 2025-07-09 18:45 UTC (permalink / raw) To: Radu Vele Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies, Andrei Kuchynski, Tzung-Bi Shih, chrome-platform, linux-kernel Hi Radu and Abhishek, Thanks for this change! On Wed, Jul 09, 2025 at 01:22:31PM +0000, Radu Vele wrote: > From: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > > Add the pr_set and dr_set typec_operations to registered typec ports. > This enables sysfs to control power and data role when the port is > capable of doing so. > > Signed-off-by: Abhishek Pandit-Subedi <abhishekpandit@chromium.org> > Co-developed-by: Radu Vele <raduvele@google.com> > Signed-off-by: Radu Vele <raduvele@google.com> Reviewed-by: Benson Leung <bleung@chromium.org> > --- > drivers/platform/chrome/cros_ec_typec.c | 77 ++++++++++++++++++++++++- > 1 file changed, 76 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c > index 7678e3d05fd3..289429ef959f 100644 > --- a/drivers/platform/chrome/cros_ec_typec.c > +++ b/drivers/platform/chrome/cros_ec_typec.c > @@ -58,8 +58,83 @@ static int cros_typec_enter_usb_mode(struct typec_port *tc_port, enum usb_mode m > &req, sizeof(req), NULL, 0); > } > > +static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type) > +{ > + struct cros_typec_port *port = typec_get_drvdata(tc_port); > + struct cros_typec_data *data = port->typec_data; > + struct ec_response_usb_pd_control_v2 resp; > + struct ec_params_usb_pd_control req; > + int role, ret; > + > + /* Must be at least v1 to support role swap. */ > + if (!data->pd_ctrl_ver) > + return -EOPNOTSUPP; > + > + /* First query the state */ > + req.port = port->port_num; > + req.role = USB_PD_CTRL_ROLE_NO_CHANGE; > + req.mux = USB_PD_CTRL_MUX_NO_CHANGE; > + req.swap = USB_PD_CTRL_SWAP_NONE; > + > + ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, > + &req, sizeof(req), &resp, sizeof(resp)); > + if (ret < 0) > + return ret; > + > + switch (swap_type) { > + case USB_PD_CTRL_SWAP_DATA: > + role = (resp.role & PD_CTRL_RESP_ROLE_DATA) ? TYPEC_HOST : > + TYPEC_DEVICE; > + break; > + case USB_PD_CTRL_SWAP_POWER: > + role = (resp.role & PD_CTRL_RESP_ROLE_POWER) ? TYPEC_SOURCE : > + TYPEC_SINK; > + break; > + default: > + dev_warn(data->dev, "Unsupported role swap type %d", swap_type); > + return -EOPNOTSUPP; > + } > + > + if (role == target_role) > + return 0; > + > + req.swap = swap_type; > + ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, > + &req, sizeof(req), &resp, sizeof(resp)); > + > + if (ret < 0) > + return ret; > + > + switch (swap_type) { > + case USB_PD_CTRL_SWAP_DATA: > + typec_set_data_role(tc_port, resp.role & PD_CTRL_RESP_ROLE_DATA ? > + TYPEC_HOST : > + TYPEC_DEVICE); > + break; > + case USB_PD_CTRL_SWAP_POWER: > + typec_set_pwr_role(tc_port, resp.role & PD_CTRL_RESP_ROLE_POWER ? > + TYPEC_SOURCE : > + TYPEC_SINK); > + break; > + } > + > + return 0; > +} > + > +static int cros_typec_dr_swap(struct typec_port *port, enum typec_data_role role) > +{ > + return cros_typec_perform_role_swap(port, role, USB_PD_CTRL_SWAP_DATA); > +} > + > +static int cros_typec_pr_swap(struct typec_port *port, enum typec_role role) > +{ > + return cros_typec_perform_role_swap(port, role, USB_PD_CTRL_SWAP_POWER); > +} > + > static const struct typec_operations cros_typec_usb_mode_ops = { > - .enter_usb_mode = cros_typec_enter_usb_mode > + .enter_usb_mode = cros_typec_enter_usb_mode, > + .dr_set = cros_typec_dr_swap, > + .pr_set = cros_typec_pr_swap > }; > > static int cros_typec_parse_port_props(struct typec_capability *cap, > -- > 2.50.0.727.gbf7dc18ff4-goog > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops 2025-07-09 13:22 [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Radu Vele 2025-07-09 13:22 ` [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port Radu Vele 2025-07-09 18:45 ` [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Benson Leung @ 2025-07-10 4:06 ` Tzung-Bi Shih 2 siblings, 0 replies; 6+ messages in thread From: Tzung-Bi Shih @ 2025-07-10 4:06 UTC (permalink / raw) To: Radu Vele Cc: Benson Leung, Abhishek Pandit-Subedi, Jameson Thies, Andrei Kuchynski, chrome-platform, linux-kernel On Wed, Jul 09, 2025 at 01:22:31PM +0000, Radu Vele wrote: > +static int cros_typec_perform_role_swap(struct typec_port *tc_port, int target_role, u8 swap_type) > +{ > [...] > + switch (swap_type) { > + case USB_PD_CTRL_SWAP_DATA: > + role = (resp.role & PD_CTRL_RESP_ROLE_DATA) ? TYPEC_HOST : > + TYPEC_DEVICE; > + break; > + case USB_PD_CTRL_SWAP_POWER: > + role = (resp.role & PD_CTRL_RESP_ROLE_POWER) ? TYPEC_SOURCE : > + TYPEC_SINK; > + break; > + default: > + dev_warn(data->dev, "Unsupported role swap type %d", swap_type); Append a newline at the end of message? > + return -EOPNOTSUPP; > + } > + > + if (role == target_role) > + return 0; > + > + req.swap = swap_type; > + ret = cros_ec_cmd(data->ec, data->pd_ctrl_ver, EC_CMD_USB_PD_CONTROL, > + &req, sizeof(req), &resp, sizeof(resp)); > + > + if (ret < 0) > + return ret; > + > + switch (swap_type) { > + case USB_PD_CTRL_SWAP_DATA: > + typec_set_data_role(tc_port, resp.role & PD_CTRL_RESP_ROLE_DATA ? > + TYPEC_HOST : > + TYPEC_DEVICE); To improve the readability a bit, how about re-use `role` variable here: role = resp.role & ... ? ... : ...; Also I'm wondering doesn't it need to assert (resp.role & ... ? ... : ...) == target_role here? Otherwise, could it just use `target_role` as the parameter for typec_set_data_role()? > + break; > + case USB_PD_CTRL_SWAP_POWER: > + typec_set_pwr_role(tc_port, resp.role & PD_CTRL_RESP_ROLE_POWER ? > + TYPEC_SOURCE : > + TYPEC_SINK); Similar here. > + break; > + } It already checks in previous code block. Not sure if some checkers would find the swtich-case block doesn't have a default clause. How about still add a default case, and have a comment to remind that should never execute? > static const struct typec_operations cros_typec_usb_mode_ops = { > - .enter_usb_mode = cros_typec_enter_usb_mode > + .enter_usb_mode = cros_typec_enter_usb_mode, > + .dr_set = cros_typec_dr_swap, > + .pr_set = cros_typec_pr_swap Leave a comma "," at the end of line? ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-07-10 4:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-09 13:22 [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Radu Vele 2025-07-09 13:22 ` [PATCH v1 2/2] platform/chrome: cros_ec_typec: Add lock per-port Radu Vele 2025-07-09 18:54 ` Benson Leung 2025-07-10 4:06 ` Tzung-Bi Shih 2025-07-09 18:45 ` [PATCH v1 1/2] platform/chrome: cros_ec_typec: Add role swap ops Benson Leung 2025-07-10 4:06 ` Tzung-Bi Shih
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox