Re: [PATCH v2 2/2] ASoC: cs35l45: Adds support for SYNC bus

Sashiko and Mark,

On 8/14/26 5:41 PM, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] cs35l45_sync_en_put() incorrectly returns 0 instead of 1 when the control value is successfully changed.
> - [Medium] The driver ignores hardware update failures in cs35l45_sync_en_put(), potentially desynchronizing software and hardware state.
> --
> 

<snip>

> 
> [Severity: Medium]
> Does this code need to check the return value of regmap_set_bits() and
> regmap_clear_bits() before updating the local software cache?
> 
> If the I2C/SPI bus transaction fails, cs35l45->sync_en will be updated even
> though the hardware state remains unchanged, potentially desynchronizing
> software and hardware state.

NACK on this in keeping with the style of the rest of ASoC unless Mark 
has any objections.

> 

<snip>

> 
> [Severity: Medium]
> Should this return 1 instead of 0 when the value is actually changed?
> 
> In the ALSA kcontrol API, put callbacks must return 1 when the hardware or
> cached state is successfully changed, so that ALSA mixer UIs and audio servers
> receive a SNDRV_CTL_EVENT_MASK_VALUE notification. Returning 0 here might lead
> to state desynchronization in userspace.

ACK, this will be fixed in v3.

> 
>> +}
> 

Ricardo