All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: codrin.ciubotariu@microchip.com
Cc: alsa-devel@alsa-project.org
Subject: [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format
Date: Wed, 19 Jun 2019 17:34:14 +0300	[thread overview]
Message-ID: <20190619143414.GA26033@mwanda> (raw)

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

             reply	other threads:[~2019-06-19 14:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-19 14:34 Dan Carpenter [this message]
2019-06-19 16:39 ` [bug report] ASoC: codecs: ad193x: Fix frame polarity for DSP_A format Codrin.Ciubotariu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190619143414.GA26033@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=codrin.ciubotariu@microchip.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.