Re: [PATCH RFC v2 3/6] ASoC: soc: qcom: sc8280xp: add support for I2S clocks

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

On 10/8/25 8:56 PM, Neil Armstrong wrote:
> Add support for getting the I2S clocks used for the MI2S
> interfaces, and enable/disable the clocks on the PCM
> startup and shutdown card callbacks.
> 
> The rate can be easily calculated since the card forces 48Hz,
> 2 channels at 16bit slot size.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---

[...]

> +static int sc8280xp_snd_i2s_index(struct snd_soc_dai *dai)
> +{
> +	switch (dai->id) {
> +	case PRIMARY_MI2S_RX...PRIMARY_MI2S_TX:

I have mixed feelings about the range syntax here.. it's only 2 entries
per and it's quite error-prone (no errors in this case, but it
encourages the thinking that things are always contiguous)..

[...]

>  	switch (cpu_dai->id) {
>  	case PRIMARY_MI2S_RX...QUATERNARY_MI2S_TX:
>  	case QUINARY_MI2S_RX...QUINARY_MI2S_TX:

whereas e.g. here we see that it's not really the case, but it's
tempting for someone trying to 'clean up' the code to change it to:

case PRIMARY_MI2S_RX...QUINARY_MI2S_TX 

and the reviewers may not catch it

Konrad