Re: [PATCH RESEND 1/2] usb: typec: tipd: fix event checking for tps25750

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Thu, Mar 28, 2024 at 05:55:51PM +0100, Javier Carrasco wrote:
> In its current form, the interrupt service routine of the tps25750
> checks the event flags in the lowest 64 bits of the interrupt event
> register (event[0]), but also in the upper part (event[1]).
> 
> Given that all flags are defined as BIT() or BIT_ULL(), they are
> restricted to the first 64 bits of the INT_EVENT1 register. Including
> the upper part of the register can lead to false positives e.g. if the
> event 64 bits above the one being checked is set, but the one being
> checked is not.
> 
> Restrict the flag checking to the first 64 bits of the INT_EVENT1
> register.
> 
> Fixes: 7e7a3c815d22 ("USB: typec: tps6598x: Add TPS25750 support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/tipd/core.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 0717cfcd9f8c..7c2f01344860 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -604,11 +604,11 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
>  	if (!tps6598x_read_status(tps, &status))
>  		goto err_clear_ints;
>  
> -	if ((event[0] | event[1]) & TPS_REG_INT_POWER_STATUS_UPDATE)
> +	if (event[0] & TPS_REG_INT_POWER_STATUS_UPDATE)
>  		if (!tps6598x_read_power_status(tps))
>  			goto err_clear_ints;
>  
> -	if ((event[0] | event[1]) & TPS_REG_INT_DATA_STATUS_UPDATE)
> +	if (event[0] & TPS_REG_INT_DATA_STATUS_UPDATE)
>  		if (!tps6598x_read_data_status(tps))
>  			goto err_clear_ints;
>  
> @@ -617,7 +617,7 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
>  	 * a plug event. Therefore, we need to check
>  	 * for pr/dr status change to set TypeC dr/pr accordingly.
>  	 */
> -	if ((event[0] | event[1]) & TPS_REG_INT_PLUG_EVENT ||
> +	if (event[0] & TPS_REG_INT_PLUG_EVENT ||
>  	    tps6598x_has_role_changed(tps, status))
>  		tps6598x_handle_plug_event(tps, status);
>  
> 
> -- 
> 2.40.1

-- 
heikki