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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d702a630-eee8-4889-98fc-62f4907791b0@linaro.org>
Date: Thu, 9 Oct 2025 11:45:28 +0200
From: Neil Armstrong <neil.armstrong@...aro.org>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
 Srinivas Kandagatla <srini@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
 Mark Brown <broonie@...nel.org>, Jaroslav Kysela <perex@...ex.cz>,
 Takashi Iwai <tiwai@...e.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Bjorn Andersson <andersson@...nel.org>,
 Konrad Dybcio <konradybcio@...nel.org>
Cc: linux-sound@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH RFC v2 3/6] ASoC: soc: qcom: sc8280xp: add support for I2S
 clocks

On 10/9/25 11:41, Konrad Dybcio wrote:
> 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@...aro.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

I understand your point, but I just took what was already in the file...

Is my change correct, yes, is the style of the code improvable, yes.

I'll do my best to fix the style issue later on.

Neil

> 
> Konrad