Re: [PATCH v3 4/5] soundwire: cadence: Fix lost ATTACHED interrupts when enumerating

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

> diff --git a/drivers/soundwire/cadence_master.c b/drivers/soundwire/cadence_master.c
> index 245191d22ccd..2f131604d884 100644
> --- a/drivers/soundwire/cadence_master.c
> +++ b/drivers/soundwire/cadence_master.c
> @@ -954,9 +954,22 @@ static void cdns_update_slave_status_work(struct work_struct *work)
>  	u32 device0_status;
>  	int retry_count = 0;
>  
> +	/*
> +	 * Clear main interrupt first so we don't lose any assertions
> +	 * the happen during this function.

typo: that happen?

The flow looks good to me otherwise, so

Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>

> +	 */
> +	cdns_writel(cdns, CDNS_MCP_INTSTAT, CDNS_MCP_INT_SLAVE_MASK);
> +
>  	slave0 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT0);
>  	slave1 = cdns_readl(cdns, CDNS_MCP_SLAVE_INTSTAT1);
>  
> +	/*
> +	 * Clear the bits before handling so we don't lose any
> +	 * bits that re-assert.
> +	 */
> +	cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT0, slave0);
> +	cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave1);
> +
>  	/* combine the two status */
>  	slave_intstat = ((u64)slave1 << 32) | slave0;
>  
> @@ -964,8 +977,6 @@ static void cdns_update_slave_status_work(struct work_struct *work)
>  
>  update_status:
>  	cdns_update_slave_status(cdns, slave_intstat);
> -	cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT0, slave0);
> -	cdns_writel(cdns, CDNS_MCP_SLAVE_INTSTAT1, slave1);
>  
>  	/*
>  	 * When there is more than one peripheral per link, it's
> @@ -982,6 +993,11 @@ static void cdns_update_slave_status_work(struct work_struct *work)
>  	 * attention with PING commands. There is no need to check for
>  	 * ALERTS since they are not allowed until a non-zero
>  	 * device_number is assigned.
> +	 *
> +	 * Do not clear the INTSTAT0/1. While looping to enumerate devices on
> +	 * #0 there could be status changes on other devices - these must
> +	 * be kept in the INTSTAT so they can be handled when all #0 devices
> +	 * have been handled.
>  	 */
>  
>  	device0_status = cdns_readl(cdns, CDNS_MCP_SLAVE_STAT);
> @@ -1001,8 +1017,7 @@ static void cdns_update_slave_status_work(struct work_struct *work)
>  		}
>  	}
>  
> -	/* clear and unmask Slave interrupt now */
> -	cdns_writel(cdns, CDNS_MCP_INTSTAT, CDNS_MCP_INT_SLAVE_MASK);
> +	/* unmask Slave interrupt now */
>  	cdns_updatel(cdns, CDNS_MCP_INTMASK,
>  		     CDNS_MCP_INT_SLAVE_MASK, CDNS_MCP_INT_SLAVE_MASK);
>