Re: [PATCH 1/3] ASoC: tas2783: let regmap-sdw-mbq poll for deferred transactions

Pierre-Louis Bossart <[email protected]>
Newsgroups org.kernel.vger.linux-sound,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
> Thanks for taking the time to review this. I am a bit of a kernel noob, 
> but I completely agree with you and the spec: FUNCTION_STATUS cannot be 
> deferred.
> 
> I think my commit message caused a misunderstanding. The patch doesn't 
> defer the status read:
> 
> The hardware is actually returning COMMAND_IGNORED for the UDMPU23 
> Cluster Index write, which is the only control tas2783_sdca_deferrable() 
> flags as true.
> 
> I only added FUNCTION_STATUS to mbq_size to expose it to -> 
> readable_reg(). Without that, regmap_sdw_mbq_poll_busy() considers the 
> status register unreadable, skips checking the busy bit for the deferred 
> UDMPU23 write, and instantly fails.

I think your patch exposes a problem in the initial implementation of
regmap_sdw_mbq_poll_busy(). FUNCTION_STATUS is a readable register by
default, not sure why we would ever need to test if it's readable.

Adding Charles for additional comments, this part looks questionable
with a possible register side-effect.

reg = SDW_SDCA_CTL(SDW_SDCA_CTL_FUNC(reg), 0,
		   SDCA_CTL_ENTITY_0_FUNCTION_STATUS, 0);

if (ctx->readable_reg(dev, reg)) { <<< are we testing the correct reg?
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.