From: Sing-Han Chen <singhanc@xxxxxxxxxx> For the CCGx, when the OPM field in the INTR_REG is cleared, then the CCI data in the PPM is reset. To align with the CCGx UCSI interface guide, this patch updates the driver to copy CCI and MESSAGE_IN before clearing UCSI interrupt. When a new command is sent, the driver will clear the old CCI and MESSAGE_IN copy. Finally, clear UCSI_READ_INT before calling complete() to ensure that the ucsi_ccg_sync_write() would wait for the interrupt handling to complete. It prevents the driver from resetting CCI prematurely. Signed-off-by: Sing-Han Chen <singhanc@xxxxxxxxxx> Signed-off-by: Haotien Hsu <haotienh@xxxxxxxxxx> --- V1->V2 - Fix uninitialized symbol 'cci' v2->v3 - Remove misusing Reported-by tags v3->v4 - Add comments for op_lock --- drivers/usb/typec/ucsi/ucsi_ccg.c | 90 ++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 7 deletions(-) diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c index eab3012e1b01..532813a32cc1 100644 --- a/drivers/usb/typec/ucsi/ucsi_ccg.c +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c @@ -192,6 +192,12 @@ struct ucsi_ccg_altmode { bool checked; } __packed; +#define CCGX_MESSAGE_IN_MAX 4 +struct op_region { + u32 cci; + u32 message_in[CCGX_MESSAGE_IN_MAX]; +}; + struct ucsi_ccg { struct device *dev; struct ucsi *ucsi; @@ -222,6 +228,13 @@ struct ucsi_ccg { bool has_multiple_dp; struct ucsi_ccg_altmode orig[UCSI_MAX_ALTMODES]; struct ucsi_ccg_altmode updated[UCSI_MAX_ALTMODES]; + + /* + * This spinlock protects op_data which includes CCI and MESSAGE_IN that + * will be updated in ISR + */ + spinlock_t op_lock; + struct op_region op_data; }; static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) @@ -305,12 +318,57 @@ static int ccg_write(struct ucsi_ccg *uc, u16 rab, const u8 *data, u32 len) return 0; } +static void ccg_op_region_read(struct ucsi_ccg *uc, unsigned int offset, + void *val, size_t val_len) +{ + struct op_region *data = &uc->op_data; + + spin_lock(&uc->op_lock); + if (offset == UCSI_CCI) + memcpy(val, &data->cci, val_len); + else if (offset == UCSI_MESSAGE_IN) + memcpy(val, &data->message_in, val_len); + spin_unlock(&uc->op_lock); +} + +static void ccg_op_region_update(struct ucsi_ccg *uc, u32 cci) +{ + u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(UCSI_MESSAGE_IN); + struct op_region *data = &uc->op_data; + u32 message_in[CCGX_MESSAGE_IN_MAX]; + + if (UCSI_CCI_LENGTH(cci)) + if (ccg_read(uc, reg, (void *)&message_in, + sizeof(message_in))) { + dev_err(uc->dev, "failed to read MESSAGE_IN\n"); + return; + } + + spin_lock(&uc->op_lock); + memcpy(&data->cci, &cci, sizeof(cci)); + if (UCSI_CCI_LENGTH(cci)) + memcpy(&data->message_in, &message_in, sizeof(message_in)); + spin_unlock(&uc->op_lock); +} + +static void ccg_op_region_clean(struct ucsi_ccg *uc) +{ + struct op_region *data = &uc->op_data; + + spin_lock(&uc->op_lock); + memset(&data->cci, 0, sizeof(data->cci)); + memset(&data->message_in, 0, sizeof(data->message_in)); + spin_unlock(&uc->op_lock); +} + static int ucsi_ccg_init(struct ucsi_ccg *uc) { unsigned int count = 10; u8 data; int status; + spin_lock_init(&uc->op_lock); + data = CCGX_RAB_UCSI_CONTROL_STOP; status = ccg_write(uc, CCGX_RAB_UCSI_CONTROL, &data, sizeof(data)); if (status < 0) @@ -520,9 +578,13 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset, u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(offset); struct ucsi_capability *cap; struct ucsi_altmode *alt; - int ret; + int ret = 0; + + if ((offset == UCSI_CCI) || (offset == UCSI_MESSAGE_IN)) + ccg_op_region_read(uc, offset, val, val_len); + else + ret = ccg_read(uc, reg, val, val_len); - ret = ccg_read(uc, reg, val, val_len); if (ret) return ret; @@ -559,9 +621,13 @@ static int ucsi_ccg_read(struct ucsi *ucsi, unsigned int offset, static int ucsi_ccg_async_write(struct ucsi *ucsi, unsigned int offset, const void *val, size_t val_len) { + struct ucsi_ccg *uc = ucsi_get_drvdata(ucsi); u16 reg = CCGX_RAB_UCSI_DATA_BLOCK(offset); - return ccg_write(ucsi_get_drvdata(ucsi), reg, val, val_len); + if (offset == UCSI_CONTROL) + ccg_op_region_clean(uc); + + return ccg_write(uc, reg, val, val_len); } static int ucsi_ccg_sync_write(struct ucsi *ucsi, unsigned int offset, @@ -616,12 +682,17 @@ static irqreturn_t ccg_irq_handler(int irq, void *data) struct ucsi_ccg *uc = data; u8 intr_reg; u32 cci; - int ret; + int ret = 0; ret = ccg_read(uc, CCGX_RAB_INTR_REG, &intr_reg, sizeof(intr_reg)); if (ret) return ret; + if (!intr_reg) + return IRQ_HANDLED; + else if (!(intr_reg & UCSI_READ_INT)) + goto err_clear_irq; + ret = ccg_read(uc, reg, (void *)&cci, sizeof(cci)); if (ret) goto err_clear_irq; @@ -629,13 +700,18 @@ static irqreturn_t ccg_irq_handler(int irq, void *data) if (UCSI_CCI_CONNECTOR(cci)) ucsi_connector_change(uc->ucsi, UCSI_CCI_CONNECTOR(cci)); - if (test_bit(DEV_CMD_PENDING, &uc->flags) && - cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE)) - complete(&uc->complete); + /* As per CCGx UCSI interface guide, copy CCI and MESSAGE_IN + * to the OpRegion before clear the UCSI interrupt + */ + ccg_op_region_update(uc, cci); err_clear_irq: ccg_write(uc, CCGX_RAB_INTR_REG, &intr_reg, sizeof(intr_reg)); + if (!ret && test_bit(DEV_CMD_PENDING, &uc->flags) && + cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE)) + complete(&uc->complete); + return IRQ_HANDLED; } -- 2.25.1