On Thu, Mar 28, 2024 at 05:55:52PM +0100, Javier Carrasco wrote:
> The current interrupt service routine of the tps6598x only reads the
> first 64 bits of the INT_EVENT1 and INT_EVENT2 registers, which means
> that any event above that range will be ignored, leaving interrupts
> unattended. Moreover, those events will not be cleared, and the device
> will keep the interrupt enabled.
>
> This issue has been observed while attempting to load patches, and the
> 'ReadyForPatch' field (bit 81) of INT_EVENT1 was set.
>
> Read the complete INT_EVENT registers to handle all interrupts generated
> by the device in a similar fashion to what is already done for the
> tps25750.
>
> Fixes: 0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Javier Carrasco <javier.carrasco@xxxxxxxxxxxxxx>
> ---
> drivers/usb/typec/tipd/core.c | 31 ++++++++++++++++++-------------
> 1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 7c2f01344860..308748d6cae6 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -637,48 +637,53 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
> static irqreturn_t tps6598x_interrupt(int irq, void *data)
> {
> struct tps6598x *tps = data;
> - u64 event1 = 0;
> - u64 event2 = 0;
> + u64 event1[2] = { };
> + u64 event2[2] = { };
> u32 status;
> int ret;
>
> mutex_lock(&tps->lock);
>
> - ret = tps6598x_read64(tps, TPS_REG_INT_EVENT1, &event1);
> - ret |= tps6598x_read64(tps, TPS_REG_INT_EVENT2, &event2);
> + ret = tps6598x_block_read(tps, TPS_REG_INT_EVENT1, event1, 11);
This is not going to work with the older TI PD controllers.
The lenght of these registers is 8 bytes on the older TI PD
controllers (TPS65981, TPS65982, etc.). I think we need to split this
function.
> if (ret) {
> - dev_err(tps->dev, "%s: failed to read events\n", __func__);
> + dev_err(tps->dev, "%s: failed to read event1\n", __func__);
> goto err_unlock;
> }
> - trace_tps6598x_irq(event1, event2);
> + ret = tps6598x_block_read(tps, TPS_REG_INT_EVENT2, event2, 11);
> + if (ret) {
> + dev_err(tps->dev, "%s: failed to read event2\n", __func__);
> + goto err_unlock;
> + }
> + trace_tps6598x_irq(event1[0], event2[0]);
>
> - if (!(event1 | event2))
> + if (!(event1[0] | event1[1] | event2[0] | event2[1]))
> goto err_unlock;
>
> if (!tps6598x_read_status(tps, &status))
> goto err_clear_ints;
>
> - if ((event1 | event2) & TPS_REG_INT_POWER_STATUS_UPDATE)
> + if ((event1[0] | event2[0]) & TPS_REG_INT_POWER_STATUS_UPDATE)
> if (!tps6598x_read_power_status(tps))
> goto err_clear_ints;
>
> - if ((event1 | event2) & TPS_REG_INT_DATA_STATUS_UPDATE)
> + if ((event1[0] | event2[0]) & TPS_REG_INT_DATA_STATUS_UPDATE)
> if (!tps6598x_read_data_status(tps))
> goto err_clear_ints;
>
> /* Handle plug insert or removal */
> - if ((event1 | event2) & TPS_REG_INT_PLUG_EVENT)
> + if ((event1[0] | event2[0]) & TPS_REG_INT_PLUG_EVENT)
> tps6598x_handle_plug_event(tps, status);
>
> err_clear_ints:
> - tps6598x_write64(tps, TPS_REG_INT_CLEAR1, event1);
> - tps6598x_write64(tps, TPS_REG_INT_CLEAR2, event2);
> + tps6598x_block_write(tps, TPS_REG_INT_CLEAR1, event1, 11);
> + tps6598x_block_write(tps, TPS_REG_INT_CLEAR2, event2, 11);
>
> err_unlock:
> mutex_unlock(&tps->lock);
>
> - if (event1 | event2)
> + if (event1[0] | event1[1] | event2[0] | event2[1])
> return IRQ_HANDLED;
> +
> return IRQ_NONE;
> }
>
>
> --
> 2.40.1
--
heikki
