[alsa-devel] [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format

[Dan Carpenter]

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