Re: [PATCH 3/3] soundwire: bus: clear parity interrupt before the mask is enabled

[Vinod Koul <vkoul@kernel.org>]

On 26-01-21, 16:37, Bard Liao wrote:
> From: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> 
> We recently added the ability to discard bus clash interrupts reported
> on startup. These bus clash interrupts can happen randomly on some
> platforms and don't seem to be valid. A master-level quirk helped
> squelch those spurious errors.
> 
> Additional tests on a new platform with the Maxim 98373 amplifier
> showed a rare case where the parity interrupt is also thrown on
> startup, at the same time as bus clashes. This issue only seems to
> happen infrequently and was only observed during suspend-resume stress
> tests while audio is streaming. We could make the problem go away by
> adding a Slave-level quirk, but there is no evidence that the issue is
> actually a Slave problem: the parity is provided by the Master, which
> could also set an invalid parity in corner cases.
> 
> This patch suggests an additional bus-level quirk for parity, which is
> only applied when the codec device is not known to have an issue with
> parity. The initial parity error will be ignored, but a trace will be
> logged to help identify potential root causes (likely a combination of
> issues on both master and slave sides influenced by board-specific
> electrical parameters).
> 
> BugLink: https://github.com/thesofproject/linux/issues/2533
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> Reviewed-by: Rander Wang <rander.wang@intel.com>
> Signed-off-by: Bard Liao <yung-chuan.liao@linux.intel.com>
> ---
>  drivers/soundwire/bus.c       | 9 +++++++++
>  drivers/soundwire/intel.c     | 3 ++-
>  include/linux/soundwire/sdw.h | 1 +
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soundwire/bus.c b/drivers/soundwire/bus.c
> index d394905936e4..57581fdb2ea9 100644
> --- a/drivers/soundwire/bus.c
> +++ b/drivers/soundwire/bus.c
> @@ -1256,6 +1256,15 @@ static int sdw_initialize_slave(struct sdw_slave *slave)
>  			sdw_write_no_pm(slave, SDW_SCP_INT1, SDW_SCP_INT1_BUS_CLASH);
>  		}
>  	}
> +	if ((slave->bus->prop.quirks & SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY) &&
> +	    !(slave->prop.quirks & SDW_SLAVE_QUIRKS_INVALID_INITIAL_PARITY)) {
> +		/* Clear parity interrupt before enabling interrupt mask */
> +		status = sdw_read_no_pm(slave, SDW_SCP_INT1);
> +		if (status & SDW_SCP_INT1_PARITY) {
> +			dev_warn(&slave->dev, "PARITY error detected before INT mask is enabled\n");
> +			sdw_write_no_pm(slave, SDW_SCP_INT1, SDW_SCP_INT1_PARITY);
> +		}
> +	}
>  
>  	/*
>  	 * Set SCP_INT1_MASK register, typically bus clash and
> diff --git a/drivers/soundwire/intel.c b/drivers/soundwire/intel.c
> index f7ba1a77a1df..c1fdc85d0a74 100644
> --- a/drivers/soundwire/intel.c
> +++ b/drivers/soundwire/intel.c
> @@ -1286,7 +1286,8 @@ static int sdw_master_read_intel_prop(struct sdw_bus *bus)
>  	if (quirk_mask & SDW_INTEL_QUIRK_MASK_BUS_DISABLE)
>  		prop->hw_disabled = true;
>  
> -	prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH;
> +	prop->quirks = SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH |
> +		SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY;

move this to intel patch please..

>  
>  	return 0;
>  }
> diff --git a/include/linux/soundwire/sdw.h b/include/linux/soundwire/sdw.h
> index a2766c3b603d..30415354d419 100644
> --- a/include/linux/soundwire/sdw.h
> +++ b/include/linux/soundwire/sdw.h
> @@ -426,6 +426,7 @@ struct sdw_master_prop {
>  };
>  
>  #define SDW_MASTER_QUIRKS_CLEAR_INITIAL_CLASH	BIT(0)
> +#define SDW_MASTER_QUIRKS_CLEAR_INITIAL_PARITY	BIT(1)

Why not add this quirk in patch 1..?

Also add comments about each quirk, hopefully it wont be a big table

-- 
~Vinod