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

[Charles Keepax <ckeepax@opensource.cirrus.com>]

On Sun, Aug 09, 2026 at 09:13:48PM +0200, Pierre-Louis Bossart wrote:
> 
> > 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?

Yeah this is doing what is intended. I guess it is technically
not spec compliant to not have a readable Function Status but I
don't see the harm in supporting it here.

The patch here looks pretty good for the tas driver, it definitely
wants those timeouts and the function status should definitely be
added to the regmap. The bit about the timeouts being the wrong
way round is pretty concerning, will need to double check that but
will send a patch later today if that is true. Probably better
to patch the core and update this patch to use the corrected
core. I will look into that and get a patch out.

Thanks,
Charles