* [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format
@ 2019-06-19 14:34 Dan Carpenter
2019-06-19 16:39 ` Codrin.Ciubotariu
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-06-19 14:34 UTC (permalink / raw)
To: codrin.ciubotariu; +Cc: alsa-devel
Hi Codrin,
Linus recently corrected me on one of my patches and I said I would
look through the kernel and fix similar bugs as well. The problem I
realized is that I'm really stupid and I have forgot how to do math...
The patch 90f6e6803139: "ASoC: codecs: ad193x: Fix frame polarity for
DSP_A format" from Feb 18, 2019, leads to the following static
checker warning:
sound/soc/codecs/ad193x.c:244 ad193x_set_dai_fmt()
warn: passing casted pointer '&dac_fmt' to 'change_bit()' 32 vs 64.
sound/soc/codecs/ad193x.c
194 static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai,
195 unsigned int fmt)
196 {
197 struct ad193x_priv *ad193x = snd_soc_component_get_drvdata(codec_dai->component);
198 unsigned int adc_serfmt = 0;
199 unsigned int dac_serfmt = 0;
200 unsigned int adc_fmt = 0;
201 unsigned int dac_fmt = 0;
^^^^^^^^^^^^^^^^^^^^^^^^
This is a u32.
202
203 /* At present, the driver only support AUX ADC mode(SND_SOC_DAIFMT_I2S
204 * with TDM), ADC&DAC TDM mode(SND_SOC_DAIFMT_DSP_A) and DAC I2S mode
205 * (SND_SOC_DAIFMT_I2S)
206 */
207 switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
208 case SND_SOC_DAIFMT_I2S:
209 adc_serfmt |= AD193X_ADC_SERFMT_TDM;
210 dac_serfmt |= AD193X_DAC_SERFMT_STEREO;
211 break;
212 case SND_SOC_DAIFMT_DSP_A:
213 adc_serfmt |= AD193X_ADC_SERFMT_AUX;
214 dac_serfmt |= AD193X_DAC_SERFMT_TDM;
215 break;
216 default:
217 if (ad193x_has_adc(ad193x))
218 return -EINVAL;
219 }
220
221 switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
222 case SND_SOC_DAIFMT_NB_NF: /* normal bit clock + frame */
223 break;
224 case SND_SOC_DAIFMT_NB_IF: /* normal bclk + invert frm */
225 adc_fmt |= AD193X_ADC_LEFT_HIGH;
226 dac_fmt |= AD193X_DAC_LEFT_HIGH;
227 break;
228 case SND_SOC_DAIFMT_IB_NF: /* invert bclk + normal frm */
229 adc_fmt |= AD193X_ADC_BCLK_INV;
230 dac_fmt |= AD193X_DAC_BCLK_INV;
231 break;
232 case SND_SOC_DAIFMT_IB_IF: /* invert bclk + frm */
233 adc_fmt |= AD193X_ADC_LEFT_HIGH;
234 adc_fmt |= AD193X_ADC_BCLK_INV;
235 dac_fmt |= AD193X_DAC_LEFT_HIGH;
236 dac_fmt |= AD193X_DAC_BCLK_INV;
237 break;
238 default:
239 return -EINVAL;
240 }
241
242 /* For DSP_*, LRCLK's polarity must be inverted */
243 if (fmt & SND_SOC_DAIFMT_DSP_A) {
244 change_bit(ffs(AD193X_DAC_LEFT_HIGH) - 1,
245 (unsigned long *)&dac_fmt);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This change_bit() will work on little endian systems but on 64 bit big
endian systems it will corrupt memory. I *think* the correct code looks
like this, but again, I'm feeling dumb so I might be wrong:
dac_fmt ^= 1 << (ffs(AD193X_DAC_LEFT_HIGH) - 1);
246 }
247
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format
2019-06-19 14:34 [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format Dan Carpenter
@ 2019-06-19 16:39 ` Codrin.Ciubotariu
0 siblings, 0 replies; 2+ messages in thread
From: Codrin.Ciubotariu @ 2019-06-19 16:39 UTC (permalink / raw)
To: dan.carpenter; +Cc: alsa-devel
On 19.06.2019 17:34, Dan Carpenter wrote:
> External E-Mail
>
>
> Hi Codrin,
>
> Linus recently corrected me on one of my patches and I said I would
> look through the kernel and fix similar bugs as well. The problem I
> realized is that I'm really stupid and I have forgot how to do math...
>
> The patch 90f6e6803139: "ASoC: codecs: ad193x: Fix frame polarity for
> DSP_A format" from Feb 18, 2019, leads to the following static
> checker warning:
>
> sound/soc/codecs/ad193x.c:244 ad193x_set_dai_fmt()
> warn: passing casted pointer '&dac_fmt' to 'change_bit()' 32 vs 64.
>
> sound/soc/codecs/ad193x.c
> 194 static int ad193x_set_dai_fmt(struct snd_soc_dai *codec_dai,
> 195 unsigned int fmt)
> 196 {
> 197 struct ad193x_priv *ad193x = snd_soc_component_get_drvdata(codec_dai->component);
> 198 unsigned int adc_serfmt = 0;
> 199 unsigned int dac_serfmt = 0;
> 200 unsigned int adc_fmt = 0;
> 201 unsigned int dac_fmt = 0;
> ^^^^^^^^^^^^^^^^^^^^^^^^
> This is a u32.
>
> 202
> 203 /* At present, the driver only support AUX ADC mode(SND_SOC_DAIFMT_I2S
> 204 * with TDM), ADC&DAC TDM mode(SND_SOC_DAIFMT_DSP_A) and DAC I2S mode
> 205 * (SND_SOC_DAIFMT_I2S)
> 206 */
> 207 switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
> 208 case SND_SOC_DAIFMT_I2S:
> 209 adc_serfmt |= AD193X_ADC_SERFMT_TDM;
> 210 dac_serfmt |= AD193X_DAC_SERFMT_STEREO;
> 211 break;
> 212 case SND_SOC_DAIFMT_DSP_A:
> 213 adc_serfmt |= AD193X_ADC_SERFMT_AUX;
> 214 dac_serfmt |= AD193X_DAC_SERFMT_TDM;
> 215 break;
> 216 default:
> 217 if (ad193x_has_adc(ad193x))
> 218 return -EINVAL;
> 219 }
> 220
> 221 switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
> 222 case SND_SOC_DAIFMT_NB_NF: /* normal bit clock + frame */
> 223 break;
> 224 case SND_SOC_DAIFMT_NB_IF: /* normal bclk + invert frm */
> 225 adc_fmt |= AD193X_ADC_LEFT_HIGH;
> 226 dac_fmt |= AD193X_DAC_LEFT_HIGH;
> 227 break;
> 228 case SND_SOC_DAIFMT_IB_NF: /* invert bclk + normal frm */
> 229 adc_fmt |= AD193X_ADC_BCLK_INV;
> 230 dac_fmt |= AD193X_DAC_BCLK_INV;
> 231 break;
> 232 case SND_SOC_DAIFMT_IB_IF: /* invert bclk + frm */
> 233 adc_fmt |= AD193X_ADC_LEFT_HIGH;
> 234 adc_fmt |= AD193X_ADC_BCLK_INV;
> 235 dac_fmt |= AD193X_DAC_LEFT_HIGH;
> 236 dac_fmt |= AD193X_DAC_BCLK_INV;
> 237 break;
> 238 default:
> 239 return -EINVAL;
> 240 }
> 241
> 242 /* For DSP_*, LRCLK's polarity must be inverted */
> 243 if (fmt & SND_SOC_DAIFMT_DSP_A) {
> 244 change_bit(ffs(AD193X_DAC_LEFT_HIGH) - 1,
> 245 (unsigned long *)&dac_fmt);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> This change_bit() will work on little endian systems but on 64 bit big
> endian systems it will corrupt memory. I *think* the correct code looks
> like this, but again, I'm feeling dumb so I might be wrong:
>
> dac_fmt ^= 1 << (ffs(AD193X_DAC_LEFT_HIGH) - 1);
or just:
dac_fmt ^= AD193X_DAC_LEFT_HIGH;
Thank you for reporting this. I will send a patch to fix it.
Best regards,
Codrin
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2019-06-19 16:40 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-06-19 14:34 [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format Dan Carpenter
2019-06-19 16:39 ` Codrin.Ciubotariu
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.