usb: typec: ucsi: split read operation

The read operation is only used to read fixed data at fixed offsets
(UCSI_VERSION, UCSI_CCI, UCSI_MESSAGE_IN). In some cases drivers apply
offset-specific overrides. Split the read() operation into three
operations, read_version(), read_cci(), read_message_in().

Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Link: https://lore.kernel.org/r/20240627-ucsi-rework-interface-v4-3-289ddc6874c7@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index e8172b7711c87a74c664fb1880cb9ea5166c9f49..17d12c1872f67e1820cb4e184ed953aaa93113e6 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -46,7 +46,7 @@ static int ucsi_read_message_in(struct ucsi *ucsi, void *buf,
        if (ucsi->version <= UCSI_VERSION_1_2)
                buf_size = clamp(buf_size, 0, 16);
 
-       return ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, buf, buf_size);
+       return ucsi->ops->read_message_in(ucsi, buf, buf_size);
 }
 
 static int ucsi_acknowledge(struct ucsi *ucsi, bool conn_ack)
@@ -159,7 +159,7 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)
        if (ret)
                return ret;
 
-       ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+       ret = ucsi->ops->read_cci(ucsi, &cci);
        if (ret)
                return ret;
 
@@ -1338,7 +1338,7 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
 
        mutex_lock(&ucsi->ppm_lock);
 
-       ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+       ret = ucsi->ops->read_cci(ucsi, &cci);
        if (ret < 0)
                goto out;
 
@@ -1356,8 +1356,7 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
 
                tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS);
                do {
-                       ret = ucsi->ops->read(ucsi, UCSI_CCI,
-                                             &cci, sizeof(cci));
+                       ret = ucsi->ops->read_cci(ucsi, &cci);
                        if (ret < 0)
                                goto out;
                        if (cci & UCSI_CCI_COMMAND_COMPLETE)
@@ -1386,7 +1385,7 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
                /* Give the PPM time to process a reset before reading CCI */
                msleep(20);
 
-               ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+               ret = ucsi->ops->read_cci(ucsi, &cci);
                if (ret)
                        goto out;
 
@@ -1806,7 +1805,7 @@ static int ucsi_init(struct ucsi *ucsi)
        ucsi->ntfy = ntfy;
 
        mutex_lock(&ucsi->ppm_lock);
-       ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
+       ret = ucsi->ops->read_cci(ucsi, &cci);
        mutex_unlock(&ucsi->ppm_lock);
        if (ret)
                return ret;
@@ -1920,7 +1919,9 @@ struct ucsi *ucsi_create(struct device *dev, const struct ucsi_operations *ops)
 {
        struct ucsi *ucsi;
 
-       if (!ops || !ops->read || !ops->sync_control || !ops->async_control)
+       if (!ops ||
+           !ops->read_version || !ops->read_cci || !ops->read_message_in ||
+           !ops->sync_control || !ops->async_control)
                return ERR_PTR(-EINVAL);
 
        ucsi = kzalloc(sizeof(*ucsi), GFP_KERNEL);
@@ -1956,8 +1957,7 @@ int ucsi_register(struct ucsi *ucsi)
 {
        int ret;
 
-       ret = ucsi->ops->read(ucsi, UCSI_VERSION, &ucsi->version,
-                             sizeof(ucsi->version));
+       ret = ucsi->ops->read_version(ucsi, &ucsi->version);
        if (ret)
                return ret;
 

diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index a8c161a39f118e859999e99124a77d8a86d56652..2560e144e158a34521bb5d1434625ad11bf3cd40 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -56,7 +56,9 @@ struct dentry;
 
 /**
  * struct ucsi_operations - UCSI I/O operations
- * @read: Read operation
+ * @read_version: Read implemented UCSI version
+ * @read_cci: Read CCI register
+ * @read_message_in: Read message data from UCSI
  * @sync_control: Blocking control operation
  * @async_control: Non-blocking control operation
  * @update_altmodes: Squashes duplicate DP altmodes
@@ -68,8 +70,9 @@ struct dentry;
  * return immediately after sending the data to the PPM.
  */
 struct ucsi_operations {
-       int (*read)(struct ucsi *ucsi, unsigned int offset,
-                   void *val, size_t val_len);
+       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 (*async_control)(struct ucsi *ucsi, u64 command);
        bool (*update_altmodes)(struct ucsi *ucsi, struct ucsi_altmode *orig,

diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index f54e4722d8f6ffe088893188a89e128857f3371e..3660dc3e6d32b4e2e9f61045b06b04a7c780ed06 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -46,8 +46,7 @@ static int ucsi_acpi_dsm(struct ucsi_acpi *ua, int func)
        return 0;
 }
 
-static int ucsi_acpi_read(struct ucsi *ucsi, unsigned int offset,
-                         void *val, size_t val_len)
+static int ucsi_acpi_read_version(struct ucsi *ucsi, u16 *version)
 {
        struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
        int ret;
@@ -56,7 +55,35 @@ static int ucsi_acpi_read(struct ucsi *ucsi, unsigned int offset,
        if (ret)
                return ret;
 
-       memcpy(val, ua->base + offset, val_len);
+       memcpy(version, ua->base + UCSI_VERSION, sizeof(*version));
+
+       return 0;
+}
+
+static int ucsi_acpi_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+       struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+       int ret;
+
+       ret = ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_READ);
+       if (ret)
+               return ret;
+
+       memcpy(cci, ua->base + UCSI_CCI, sizeof(*cci));
+
+       return 0;
+}
+
+static int ucsi_acpi_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
+{
+       struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+       int ret;
+
+       ret = ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_READ);
+       if (ret)
+               return ret;
+
+       memcpy(val, ua->base + UCSI_MESSAGE_IN, val_len);
 
        return 0;
 }
@@ -99,36 +126,50 @@ out_clear_bit:
 }
 
 static const struct ucsi_operations ucsi_acpi_ops = {
-       .read = ucsi_acpi_read,
+       .read_version = ucsi_acpi_read_version,
+       .read_cci = ucsi_acpi_read_cci,
+       .read_message_in = ucsi_acpi_read_message_in,
        .sync_control = ucsi_acpi_sync_control,
        .async_control = ucsi_acpi_async_control
 };
 
 static int
-ucsi_zenbook_read(struct ucsi *ucsi, unsigned int offset, void *val, size_t val_len)
+ucsi_zenbook_read_cci(struct ucsi *ucsi, u32 *cci)
 {
        struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
        int ret;
 
-       if (offset == UCSI_VERSION || UCSI_COMMAND(ua->cmd) == UCSI_PPM_RESET) {
+       if (UCSI_COMMAND(ua->cmd) == UCSI_PPM_RESET) {
                ret = ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_READ);
                if (ret)
                        return ret;
        }
 
-       memcpy(val, ua->base + offset, val_len);
+       memcpy(cci, ua->base + UCSI_CCI, sizeof(*cci));
+
+       return 0;
+}
+
+static int
+ucsi_zenbook_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
+{
+       struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
+
+       /* UCSI_MESSAGE_IN is never read for PPM_RESET, return stored data */
+       memcpy(val, ua->base + UCSI_MESSAGE_IN, val_len);
 
        return 0;
 }
 
 static const struct ucsi_operations ucsi_zenbook_ops = {
-       .read = ucsi_zenbook_read,
+       .read_version = ucsi_acpi_read_version,
+       .read_cci = ucsi_zenbook_read_cci,
+       .read_message_in = ucsi_zenbook_read_message_in,
        .sync_control = ucsi_acpi_sync_control,
        .async_control = ucsi_acpi_async_control
 };
 
-static int ucsi_gram_read(struct ucsi *ucsi, unsigned int offset,
-                         void *val, size_t val_len)
+static int ucsi_gram_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
 {
        u16 bogus_change = UCSI_CONSTAT_POWER_LEVEL_CHANGE |
                           UCSI_CONSTAT_PDOS_CHANGE;
@@ -136,13 +177,12 @@ static int ucsi_gram_read(struct ucsi *ucsi, unsigned int offset,
        struct ucsi_connector_status *status;
        int ret;
 
-       ret = ucsi_acpi_read(ucsi, offset, val, val_len);
+       ret = ucsi_acpi_read_message_in(ucsi, val, val_len);
        if (ret < 0)
                return ret;
 
        if (UCSI_COMMAND(ua->cmd) == UCSI_GET_CONNECTOR_STATUS &&
-           test_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags) &&
-           offset == UCSI_MESSAGE_IN) {
+           test_bit(UCSI_ACPI_CHECK_BOGUS_EVENT, &ua->flags)) {
                status = (struct ucsi_connector_status *)val;
 
                /* Clear the bogus change */
@@ -173,7 +213,9 @@ static int ucsi_gram_sync_control(struct ucsi *ucsi, u64 command)
 }
 
 static const struct ucsi_operations ucsi_gram_ops = {
-       .read = ucsi_gram_read,
+       .read_version = ucsi_acpi_read_version,
+       .read_cci = ucsi_acpi_read_cci,
+       .read_message_in = ucsi_gram_read_message_in,
        .sync_control = ucsi_gram_sync_control,
        .async_control = ucsi_acpi_async_control
 };
@@ -203,7 +245,7 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
        u32 cci;
        int ret;
 
-       ret = ua->ucsi->ops->read(ua->ucsi, UCSI_CCI, &cci, sizeof(cci));
+       ret = ua->ucsi->ops->read_cci(ua->ucsi, &cci);
        if (ret)
                return;
 

diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
index 76b39bb9762d1a0c590ed86e65b6c61cd3e8e890..6ccc394f268ed15dadd23a7ab68080315965216f 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi_ccg.c
+++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
@@ -556,32 +556,34 @@ static void ucsi_ccg_nvidia_altmode(struct ucsi_ccg *uc,
        }
 }
 
-static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset,
-                        void *val, size_t val_len)
+static int ucsi_ccg_read_version(struct ucsi *ucsi, u16 *version)
 {
        struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi);
-       u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(offset);
-       struct ucsi_capability *cap;
-       struct ucsi_altmode *alt;
-       int ret = 0;
+       u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(UCSI_VERSION);
 
-       if (offset == UCSI_CCI) {
-               spin_lock(&uc->op_lock);
-               memcpy(val, &(uc->op_data).cci, val_len);
-               spin_unlock(&uc->op_lock);
-       } else if (offset == UCSI_MESSAGE_IN) {
-               spin_lock(&uc->op_lock);
-               memcpy(val, &(uc->op_data).message_in, val_len);
-               spin_unlock(&uc->op_lock);
-       } else {
-               ret = ccg_read(uc, reg, val, val_len);
-       }
+       return ccg_read(uc, reg, (u8 *)version, sizeof(*version));
+}
 
-       if (ret)
-               return ret;
+static int ucsi_ccg_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+       struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi);
 
-       if (offset != UCSI_MESSAGE_IN)
-               return ret;
+       spin_lock(&uc->op_lock);
+       *cci = uc->op_data.cci;
+       spin_unlock(&uc->op_lock);
+
+       return 0;
+}
+
+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:
@@ -607,7 +609,7 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset,
        }
        uc->last_cmd_sent = 0;
 
-       return ret;
+       return 0;
 }
 
 static int ucsi_ccg_async_control(struct ucsi *ucsi, u64 command)
@@ -663,7 +665,9 @@ err_clear_bit:
 }
 
 static const struct ucsi_operations ucsi_ccg_ops = {
-       .read = ucsi_ccg_read,
+       .read_version = ucsi_ccg_read_version,
+       .read_cci = ucsi_ccg_read_cci,
+       .read_message_in = ucsi_ccg_read_message_in,
        .sync_control = ucsi_ccg_sync_control,
        .async_control = ucsi_ccg_async_control,
        .update_altmodes = ucsi_ccg_update_altmodes

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index ebd76257c4fcf8e889378b3af0e111bbac8a63dc..56bad054e78fc97410a3a0e84a8b944535497ebf 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -114,6 +114,21 @@ out_unlock:
        return ret;
 }
 
+static int pmic_glink_ucsi_read_version(struct ucsi *ucsi, u16 *version)
+{
+       return pmic_glink_ucsi_read(ucsi, UCSI_VERSION, version, sizeof(*version));
+}
+
+static int pmic_glink_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+       return pmic_glink_ucsi_read(ucsi, UCSI_CCI, cci, sizeof(*cci));
+}
+
+static int pmic_glink_ucsi_read_message_in(struct ucsi *ucsi, void *val, size_t val_len)
+{
+       return pmic_glink_ucsi_read(ucsi, UCSI_MESSAGE_IN, val, val_len);
+}
+
 static int pmic_glink_ucsi_locked_write(struct pmic_glink_ucsi *ucsi, unsigned int offset,
                                        const void *val, size_t val_len)
 {
@@ -214,7 +229,9 @@ static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
 }
 
 static const struct ucsi_operations pmic_glink_ucsi_ops = {
-       .read = pmic_glink_ucsi_read,
+       .read_version = pmic_glink_ucsi_read_version,
+       .read_cci = pmic_glink_ucsi_read_cci,
+       .read_message_in = pmic_glink_ucsi_read_message_in,
        .sync_control = pmic_glink_ucsi_sync_control,
        .async_control = pmic_glink_ucsi_async_control,
        .update_connector = pmic_glink_ucsi_update_connector,

diff --git a/drivers/usb/typec/ucsi/ucsi_stm32g0.c b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
index 396e2090e7c36d10012a8e3cc9e1b634c62f20e7..14737ca3724c4ef9a240dc62ae7dcad26d5df0c5 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi_stm32g0.c
+++ b/drivers/usb/typec/ucsi/ucsi_stm32g0.c
@@ -359,6 +359,21 @@ static int ucsi_stm32g0_read(struct ucsi *ucsi, unsigned int offset, void *val,
        return 0;
 }
 
+static int ucsi_stm32g0_read_version(struct ucsi *ucsi, u16 *version)
+{
+       return ucsi_stm32g0_read(ucsi, UCSI_VERSION, version, sizeof(*version));
+}
+
+static int ucsi_stm32g0_read_cci(struct ucsi *ucsi, u32 *cci)
+{
+       return ucsi_stm32g0_read(ucsi, UCSI_CCI, cci, sizeof(*cci));
+}
+
+static int ucsi_stm32g0_read_message_in(struct ucsi *ucsi, void *val, size_t len)
+{
+       return ucsi_stm32g0_read(ucsi, UCSI_MESSAGE_IN, val, len);
+}
+
 static int ucsi_stm32g0_async_control(struct ucsi *ucsi, u64 command)
 {
        struct ucsi_stm32g0 *g0 = ucsi_get_drvdata(ucsi);
@@ -446,7 +461,9 @@ static irqreturn_t ucsi_stm32g0_irq_handler(int irq, void *data)
 }
 
 static const struct ucsi_operations ucsi_stm32g0_ops = {
-       .read = ucsi_stm32g0_read,
+       .read_version = ucsi_stm32g0_read_version,
+       .read_cci = ucsi_stm32g0_read_cci,
+       .read_message_in = ucsi_stm32g0_read_message_in,
        .sync_control = ucsi_stm32g0_sync_control,
        .async_control = ucsi_stm32g0_async_control,
 };

diff --git a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
index e5e8ba0c0eaa7d5410cc29062b34428b497b4d96..95a333ad54966b36133d3ae5b06fe01987117a94 100644 (file)
--- a/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
+++ b/drivers/usb/typec/ucsi/ucsi_yoga_c630.c
@@ -27,8 +27,16 @@ struct yoga_c630_ucsi {
        u16 version;
 };
 
-static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset,
-                              void *val, size_t val_len)
+static int yoga_c630_ucsi_read_version(struct ucsi *ucsi, u16 *version)
+{
+       struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi);
+
+       *version = uec->version;
+
+       return 0;
+}
+
+static int yoga_c630_ucsi_read_cci(struct ucsi *ucsi, u32 *cci)
 {
        struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi);
        u8 buf[YOGA_C630_UCSI_READ_SIZE];
@@ -38,22 +46,26 @@ static int yoga_c630_ucsi_read(struct ucsi *ucsi, unsigned int offset,
        if (ret)
                return ret;
 
-       if (offset == UCSI_VERSION) {
-               memcpy(val, &uec->version, min(val_len, sizeof(uec->version)));
-               return 0;
-       }
+       memcpy(cci, buf, sizeof(*cci));
 
-       switch (offset) {
-       case UCSI_CCI:
-               memcpy(val, buf, min(val_len, YOGA_C630_UCSI_CCI_SIZE));
-               return 0;
-       case UCSI_MESSAGE_IN:
-               memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE,
-                      min(val_len, YOGA_C630_UCSI_DATA_SIZE));
-               return 0;
-       default:
-               return -EINVAL;
-       }
+       return 0;
+}
+
+static int yoga_c630_ucsi_read_message_in(struct ucsi *ucsi,
+                                         void *val, size_t val_len)
+{
+       struct yoga_c630_ucsi *uec = ucsi_get_drvdata(ucsi);
+       u8 buf[YOGA_C630_UCSI_READ_SIZE];
+       int ret;
+
+       ret = yoga_c630_ec_ucsi_read(uec->ec, buf);
+       if (ret)
+               return ret;
+
+       memcpy(val, buf + YOGA_C630_UCSI_CCI_SIZE,
+              min(val_len, YOGA_C630_UCSI_DATA_SIZE));
+
+       return 0;
 }
 
 static int yoga_c630_ucsi_async_control(struct ucsi *ucsi, u64 command)
@@ -93,7 +105,9 @@ out_clear_bit:
 }
 
 const struct ucsi_operations yoga_c630_ucsi_ops = {
-       .read = yoga_c630_ucsi_read,
+       .read_version = yoga_c630_ucsi_read_version,
+       .read_cci = yoga_c630_ucsi_read_cci,
+       .read_message_in = yoga_c630_ucsi_read_message_in,
        .sync_control = yoga_c630_ucsi_sync_control,
        .async_control = yoga_c630_ucsi_async_control,
 };
@@ -126,7 +140,7 @@ static int yoga_c630_ucsi_notify(struct notifier_block *nb,
                return NOTIFY_OK;
 
        case LENOVO_EC_EVENT_UCSI:
-               ret = uec->ucsi->ops->read(uec->ucsi, UCSI_CCI, &cci, sizeof(cci));
+               ret = uec->ucsi->ops->read_cci(uec->ucsi, &cci);
                if (ret)
                        return NOTIFY_DONE;