* [PATCH 1/3] usb: typec: ucsi: return CCI and message from sync_control callback
2025-01-17 10:49 [PATCH 0/3] usb: typec: ucsi: continue rework of command interface Dmitry Baryshkov
@ 2025-01-17 10:49 ` Dmitry Baryshkov
2025-01-18 7:28 ` kernel test robot
2025-01-17 10:49 ` [PATCH 2/3] usb: typec: ucsi: ccg: move command quirks to ucsi_ccg_sync_control() Dmitry Baryshkov
` (2 subsequent siblings)
3 siblings, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2025-01-17 10:49 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel
Some of the drivers emulate or handle some of the commands in the
platform-specific way. The code ends up being split between several
callbacks, which complicates emulation.
In preparation to reworking such drivers, move read_cci() and
read_message_in() calls into ucsi_sync_control_common().
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/usb/typec/ucsi/ucsi.c | 19 +++++++++++--------
drivers/usb/typec/ucsi/ucsi.h | 6 ++++--
drivers/usb/typec/ucsi/ucsi_acpi.c | 5 +++--
drivers/usb/typec/ucsi/ucsi_ccg.c | 5 +++--
4 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index fcf499cc9458c0d12015a7e36e5f1ac448c3a431..559390a07a4e427c9b520dffaac905277d071638 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -55,7 +55,8 @@ void ucsi_notify_common(struct ucsi *ucsi, u32 cci)
}
EXPORT_SYMBOL_GPL(ucsi_notify_common);
-int ucsi_sync_control_common(struct ucsi *ucsi, u64 command)
+int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size)
{
bool ack = UCSI_COMMAND(command) == UCSI_ACK_CC_CI;
int ret;
@@ -80,6 +81,13 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command)
else
clear_bit(COMMAND_PENDING, &ucsi->flags);
+ if (!ret && cci)
+ ret = ucsi->ops->read_cci(ucsi, cci);
+
+ if (!ret && data &&
+ (*cci & UCSI_CCI_COMMAND_COMPLETE))
+ ret = ucsi->ops->read_message_in(ucsi, data, size);
+
return ret;
}
EXPORT_SYMBOL_GPL(ucsi_sync_control_common);
@@ -95,7 +103,7 @@ static int ucsi_acknowledge(struct ucsi *ucsi, bool conn_ack)
ctrl |= UCSI_ACK_CONNECTOR_CHANGE;
}
- return ucsi->ops->sync_control(ucsi, ctrl);
+ return ucsi->ops->sync_control(ucsi, ctrl, NULL, NULL, 0);
}
static int ucsi_run_command(struct ucsi *ucsi, u64 command, u32 *cci,
@@ -108,9 +116,7 @@ static int ucsi_run_command(struct ucsi *ucsi, u64 command, u32 *cci,
if (size > UCSI_MAX_DATA_LENGTH(ucsi))
return -EINVAL;
- ret = ucsi->ops->sync_control(ucsi, command);
- if (ucsi->ops->read_cci(ucsi, cci))
- return -EIO;
+ ret = ucsi->ops->sync_control(ucsi, command, cci, data, size);
if (*cci & UCSI_CCI_BUSY)
return ucsi_run_command(ucsi, UCSI_CANCEL, cci, NULL, 0, false) ?: -EBUSY;
@@ -127,9 +133,6 @@ static int ucsi_run_command(struct ucsi *ucsi, u64 command, u32 *cci,
else
err = 0;
- if (!err && data && UCSI_CCI_LENGTH(*cci))
- err = ucsi->ops->read_message_in(ucsi, data, size);
-
/*
* Don't ACK connection change if there was an error.
*/
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 5ff369c24a2fc445e8559201d563c31d83b2c876..9ffc8debd7d77d118db042f2749cf429a848f8df 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -76,7 +76,8 @@ struct ucsi_operations {
int (*read_version)(struct ucsi *ucsi, u16 *version);
int (*read_cci)(struct ucsi *ucsi, u32 *cci);
int (*read_message_in)(struct ucsi *ucsi, void *val, size_t val_len);
- int (*sync_control)(struct ucsi *ucsi, u64 command);
+ int (*sync_control)(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size);
int (*async_control)(struct ucsi *ucsi, u64 command);
bool (*update_altmodes)(struct ucsi *ucsi, struct ucsi_altmode *orig,
struct ucsi_altmode *updated);
@@ -528,7 +529,8 @@ void ucsi_altmode_update_active(struct ucsi_connector *con);
int ucsi_resume(struct ucsi *ucsi);
void ucsi_notify_common(struct ucsi *ucsi, u32 cci);
-int ucsi_sync_control_common(struct ucsi *ucsi, u64 command);
+int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size);
#if IS_ENABLED(CONFIG_POWER_SUPPLY)
int ucsi_register_port_psy(struct ucsi_connector *con);
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 5c55155519634d95b6e544632f869c2867093617..8b02082201ec5b85031472563b8b8d1eea6134de 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -122,12 +122,13 @@ static int ucsi_gram_read_message_in(struct ucsi *ucsi, void *val, size_t val_le
return ret;
}
-static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command)
+static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size)
{
struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
int ret;
- ret = ucsi_sync_control_common(ucsi, command);
+ ret = ucsi_sync_control_common(ucsi, command, cci, data, size);
if (ret < 0)
return ret;
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 740171f24ef9fae9abdb52d7995abe692e0a7623..02ac04a52239327475a3590734b77f3ac74bb589 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -628,7 +628,8 @@ static int ucsi_ccg_async_control(struct ucsi *ucsi, u64 command)
return ccg_write(uc, reg, (u8 *)&command, sizeof(command));
}
-static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command)
+static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *data, size_t size)
{
struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi);
struct ucsi_connector *con;
@@ -652,7 +653,7 @@ static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command)
ucsi_ccg_update_set_new_cam_cmd(uc, con, &command);
}
- ret = ucsi_sync_control_common(ucsi, command);
+ ret = ucsi_sync_control_common(ucsi, command, cci, data, size);
err_put:
pm_runtime_put_sync(uc->dev);
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/3] usb: typec: ucsi: ccg: move command quirks to ucsi_ccg_sync_control()
2025-01-17 10:49 [PATCH 0/3] usb: typec: ucsi: continue rework of command interface Dmitry Baryshkov
2025-01-17 10:49 ` [PATCH 1/3] usb: typec: ucsi: return CCI and message from sync_control callback Dmitry Baryshkov
@ 2025-01-17 10:49 ` Dmitry Baryshkov
2025-01-17 10:49 ` [PATCH 3/3] usb: typec: ucsi: acpi: move LG Gram quirk to ucsi_gram_sync_control() Dmitry Baryshkov
2025-01-28 8:26 ` [PATCH 0/3] usb: typec: ucsi: continue rework of command interface Heikki Krogerus
3 siblings, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2025-01-17 10:49 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel
It is easier to keep all command-specific quirks in a single place. Move
them to ucsi_ccg_sync_control() as the code now allows us to return
modified messages data.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/usb/typec/ucsi/ucsi_ccg.c | 62 ++++++++++++++++++---------------------
1 file changed, 29 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 02ac04a52239327475a3590734b77f3ac74bb589..47498ee6cca89a9cc3fee872703b8d27487ea7ae 100644
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -222,7 +222,6 @@ struct ucsi_ccg {
u16 fw_build;
struct work_struct pm_work;
- u64 last_cmd_sent;
bool has_multiple_dp;
struct ucsi_ccg_altmode orig[UCSI_MAX_ALTMODES];
struct ucsi_ccg_altmode updated[UCSI_MAX_ALTMODES];
@@ -538,9 +537,10 @@ static void ucsi_ccg_update_set_new_cam_cmd(struct ucsi_ccg *uc,
* first and then vdo=0x3
*/
static void ucsi_ccg_nvidia_altmode(struct ucsi_ccg *uc,
- struct ucsi_altmode *alt)
+ struct ucsi_altmode *alt,
+ u64 command)
{
- switch (UCSI_ALTMODE_OFFSET(uc->last_cmd_sent)) {
+ switch (UCSI_ALTMODE_OFFSET(command)) {
case NVIDIA_FTB_DP_OFFSET:
if (alt[0].mid == USB_TYPEC_NVIDIA_VLINK_DBG_VDO)
alt[0].mid = USB_TYPEC_NVIDIA_VLINK_DP_VDO |
@@ -578,37 +578,11 @@ static int ucsi_ccg_read_cci(struct ucsi *ucsi, u32 *cci)
static int ucsi_ccg_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
{
struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi);
- struct ucsi_capability *cap;
- struct ucsi_altmode *alt;
spin_lock(&uc->op_lock);
memcpy(val, uc->op_data.message_in, val_len);
spin_unlock(&uc->op_lock);
- switch (UCSI_COMMAND(uc->last_cmd_sent)) {
- case UCSI_GET_CURRENT_CAM:
- if (uc->has_multiple_dp)
- ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)val);
- break;
- case UCSI_GET_ALTERNATE_MODES:
- if (UCSI_ALTMODE_RECIPIENT(uc->last_cmd_sent) ==
- UCSI_RECIPIENT_SOP) {
- alt = val;
- if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID)
- ucsi_ccg_nvidia_altmode(uc, alt);
- }
- break;
- case UCSI_GET_CAPABILITY:
- if (uc->fw_build == CCG_FW_BUILD_NVIDIA_TEGRA) {
- cap = val;
- cap->features &= ~UCSI_CAP_ALT_MODE_DETAILS;
- }
- break;
- default:
- break;
- }
- uc->last_cmd_sent = 0;
-
return 0;
}
@@ -639,11 +613,9 @@ static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
mutex_lock(&uc->lock);
pm_runtime_get_sync(uc->dev);
- uc->last_cmd_sent = command;
-
- if (UCSI_COMMAND(uc->last_cmd_sent) == UCSI_SET_NEW_CAM &&
+ if (UCSI_COMMAND(command) == UCSI_SET_NEW_CAM &&
uc->has_multiple_dp) {
- con_index = (uc->last_cmd_sent >> 16) &
+ con_index = (command >> 16) &
UCSI_CMD_CONNECTOR_MASK;
if (con_index == 0) {
ret = -EINVAL;
@@ -655,6 +627,30 @@ static int ucsi_ccg_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
ret = ucsi_sync_control_common(ucsi, command, cci, data, size);
+ switch (UCSI_COMMAND(command)) {
+ case UCSI_GET_CURRENT_CAM:
+ if (uc->has_multiple_dp)
+ ucsi_ccg_update_get_current_cam_cmd(uc, (u8 *)data);
+ break;
+ case UCSI_GET_ALTERNATE_MODES:
+ if (UCSI_ALTMODE_RECIPIENT(command) == UCSI_RECIPIENT_SOP) {
+ struct ucsi_altmode *alt = data;
+
+ if (alt[0].svid == USB_TYPEC_NVIDIA_VLINK_SID)
+ ucsi_ccg_nvidia_altmode(uc, alt, command);
+ }
+ break;
+ case UCSI_GET_CAPABILITY:
+ if (uc->fw_build == CCG_FW_BUILD_NVIDIA_TEGRA) {
+ struct ucsi_capability *cap = data;
+
+ cap->features &= ~UCSI_CAP_ALT_MODE_DETAILS;
+ }
+ break;
+ default:
+ break;
+ }
+
err_put:
pm_runtime_put_sync(uc->dev);
mutex_unlock(&uc->lock);
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/3] usb: typec: ucsi: acpi: move LG Gram quirk to ucsi_gram_sync_control()
2025-01-17 10:49 [PATCH 0/3] usb: typec: ucsi: continue rework of command interface Dmitry Baryshkov
2025-01-17 10:49 ` [PATCH 1/3] usb: typec: ucsi: return CCI and message from sync_control callback Dmitry Baryshkov
2025-01-17 10:49 ` [PATCH 2/3] usb: typec: ucsi: ccg: move command quirks to ucsi_ccg_sync_control() Dmitry Baryshkov
@ 2025-01-17 10:49 ` Dmitry Baryshkov
2025-01-28 8:26 ` [PATCH 0/3] usb: typec: ucsi: continue rework of command interface Heikki Krogerus
3 siblings, 0 replies; 6+ messages in thread
From: Dmitry Baryshkov @ 2025-01-17 10:49 UTC (permalink / raw)
To: Heikki Krogerus, Greg Kroah-Hartman; +Cc: linux-usb, linux-kernel
It is easier to keep all command-specific quirks in a single place. Move
them to ucsi_gram_sync_control() as the code now allows us to return
modified messages data.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
drivers/usb/typec/ucsi/ucsi_acpi.c | 30 +++++++++---------------------
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 8b02082201ec5b85031472563b8b8d1eea6134de..ada5d0d21ee6fb1f406b6a8b8466bc71ffdb5b46 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -99,17 +99,23 @@ static const struct ucsi_operations ucsi_acpi_ops = {
.async_control = ucsi_acpi_async_control
};
-static int ucsi_gram_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
+static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
+ void *val, size_t len)
{
u16 bogus_change = UCSI_CONSTAT_POWER_LEVEL_CHANGE |
UCSI_CONSTAT_PDOS_CHANGE;
struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
int ret;
- ret = ucsi_acpi_read_message_in(ucsi, val, val_len);
+ ret = ucsi_sync_control_common(ucsi, command, cci, val, len);
if (ret < 0)
return ret;
+ if (UCSI_COMMAND(ua->cmd) == UCSI_GET_PDOS &&
+ ua->cmd & UCSI_GET_PDOS_PARTNER_PDO(1) &&
+ ua->cmd & UCSI_GET_PDOS_SRC_PDOS)
+ ua->check_bogus_event = true;
+
if (UCSI_COMMAND(ua->cmd) == UCSI_GET_CONNECTOR_STATUS &&
ua->check_bogus_event) {
/* Clear the bogus change */
@@ -122,28 +128,10 @@ static int ucsi_gram_read_message_in(struct ucsi *ucsi, void *val, size_t val_le
return ret;
}
-static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command, u32 *cci,
- void *data, size_t size)
-{
- struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
- int ret;
-
- ret = ucsi_sync_control_common(ucsi, command, cci, data, size);
- if (ret < 0)
- return ret;
-
- if (UCSI_COMMAND(ua->cmd) == UCSI_GET_PDOS &&
- ua->cmd & UCSI_GET_PDOS_PARTNER_PDO(1) &&
- ua->cmd & UCSI_GET_PDOS_SRC_PDOS)
- ua->check_bogus_event = true;
-
- return ret;
-}
-
static const struct ucsi_operations ucsi_gram_ops = {
.read_version = ucsi_acpi_read_version,
.read_cci = ucsi_acpi_read_cci,
- .read_message_in = ucsi_gram_read_message_in,
+ .read_message_in = ucsi_acpi_read_message_in,
.sync_control = ucsi_gram_sync_control,
.async_control = ucsi_acpi_async_control
};
--
2.39.5
^ permalink raw reply related [flat|nested] 6+ messages in thread