All of lore.kernel.org
 help / color / mirror / Atom feed
* Audio codec with multiple I2C devices
       [not found] <mailman.300.1396595329.806.alsa-devel@alsa-project.org>
@ 2014-04-04 22:02 ` Gilles
  2014-04-06  9:15   ` Lars-Peter Clausen
  0 siblings, 1 reply; 5+ messages in thread
From: Gilles @ 2014-04-04 22:02 UTC (permalink / raw)
  To: alsa-devel

Folks,

I'm writing an ALSA SoC codec which uses a 3 distinct I2C devices (a DAC, a CAG for volume and a input selector) all 3 at different I2C addresses. My existing code relies on DT to declare the primary I2C address (in this case the DAC) and it works. But I now need to add support for the extended functionality.

Can someone point me to the best way to declare the second I2C address in the DT and make use of it in the driver?

I know I could always hard code it and add a new i2c_new_device() in the probe() into the private data but I'm trying to make it a bit more DT friendly. Not having much luck on finding examples of this.

I'm attaching the existing code code below if anyone wants to reference to snipets of it. As you can see, the set/get volume are declared as _EXT since the volume is controlled by a secondary GC I2C device. Similarily, the input selector talks to a third I2C device.

Any help would be appreciated.  Thanks!




/*
//  ALSA SoC codec driver for Birdland Audio SDM-50 Digital Amplifier
//  Copyright (C) 2012 Birdland Audio - http://birdland.com
//  Author: Gilles Gameiro <gilles@whospot.com>
//
*/
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/init.h>
#include <linux/delay.h>
#include <linux/pm.h>
#include <linux/i2c.h>
#include <linux/gpio.h>
#include <linux/of_gpio.h>
#include <linux/slab.h>
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
#include <sound/soc.h>
#include <sound/initval.h>
#include <sound/tlv.h>


#define  DRV_NAME "sdm2x-audio-codec"
#define  BIRDLAND_MODEL_250     0

#define  SDM2X_REG0_VOLUME  0
#define  SDM2X_REG1_MUXCLK  1
#define   SDM2X_INPUT_EXTERNAL   0x80
#define   SDM2X_PLAYRATE_48000   0x08


#define SDM2X_RATES     (  SNDRV_PCM_RATE_32000 \
                         | SNDRV_PCM_RATE_44100 \
                         | SNDRV_PCM_RATE_48000 \
                         | SNDRV_PCM_RATE_64000 \
                         | SNDRV_PCM_RATE_88200 \
                         | SNDRV_PCM_RATE_96000 \
                         | SNDRV_PCM_RATE_176400 \
                         | SNDRV_PCM_RATE_192000 )

#define SDM2X_FORMATS  (   SNDRV_PCM_FMTBIT_S16_LE \
                         | SNDRV_PCM_FMTBIT_S20_3LE \
                         | SNDRV_PCM_FMTBIT_S24_3LE \
                         | SNDRV_PCM_FMTBIT_S32_LE )



/* codec private data */
struct sdm2x_priv {
  struct snd_soc_codec *codec;
  enum snd_soc_control_type control_type;
  u16 model;
  unsigned int sysclk;
  u8 input_cache;     // Caches the back panel input selected     (Controlled by AK4113)
  u8 volume_cache;    // Caches the current value of the volume   (Controlled by DAC121C085)
};


// FPGA I2C Slave IP is write only. Use register cache
#define SDM2X_RCACHE_SIZE  4
static const u8 sdm2x_reg_cache[SDM2X_RCACHE_SIZE] = {
  0x00, (SDM2X_INPUT_EXTERNAL | SDM2X_PLAYRATE_48000), 0x00, 0x00,
};




static int sdm250_get_volume(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
{
  struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
  struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);

  ucontrol->value.integer.value[0] = sdm2x->volume_cache;
  return 0;
}
static int sdm250_set_volume(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
{
  struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
  struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);
  int volume = ucontrol->value.integer.value[0] & 0x7f;

  sdm2x->volume_cache = volume;
  // TODO: Write it to GA

  return 0;
}



static int sdm250_get_input(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
{
  struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
  struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);

  ucontrol->value.integer.value[0] = sdm2x->input_cache;
  return 0;
}
static int sdm250_set_input(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
{
  struct snd_soc_codec *codec = snd_kcontrol_chip(kcontrol);
  struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);

  int input = ucontrol->value.integer.value[0] & 0x7f;
  sdm2x->input_cache = volume;
  // TODO: Write it to AK4113

  return 0;
}



/* General Volume -127dB to 0dB on 1dB steps */
static DECLARE_TLV_DB_SCALE(general_volume_tlv, -12700, 100, 0);

/* Enum declarations */
static const char *sdm2x_srcin[] = {"Internal", "External"};
static const struct soc_enum sdm2x_srcin_enum = SOC_ENUM_SINGLE(SDM2X_REG1_MUXCLK, 7, 2, sdm2x_srcin);

static const char *sdm2x_i2sin[] = {"Optical 1", "Optical 2", "Optical 3", "XLR" };
static const struct soc_enum sdm2x_i2sin_enum = SOC_ENUM_SINGLE(SDM2X_REG1_MUXCLK, 0, 3, sdm2x_i2sin);

static const struct snd_kcontrol_new sdm2x_snd_controls[] = {
  /* General Volume: This is set outside of the FPGA so use getter/setter functions */
  SOC_SINGLE_EXT_TLV("Amplifier Volume", SDM2X_REG0_VOLUME, 0, 127, 0, sdm250_get_volume, sdm250_set_volume, general_volume_tlv),
  /* FPGA Manged Controls (primary I2C of this driver) */
  SOC_SINGLE("Digital Mute", SDM2X_REG0_VOLUME, 7, 1, 0),   // TODO: Make this a DAI op?
  SOC_ENUM("SRC Input Source", sdm2x_srcin_enum),
  /* Back Pannel Controls (AK4113) */
  SOC_ENUM_EXT("Back Panel Input", sdm2x_i2sin_enum, sdm250_get_input, sdm250_set_input)
};




static int sdm2x_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
{
  struct snd_soc_codec *codec = dai->codec;
  //struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);
  int fsref;
  unsigned char divreg;

  // Set Data word length not needed in I2S)
  fsref = params_format(params);

  // Set Sampling Rate: I haven't figured out how to make the mcasp I2S master for TX and slave for RX so let the FPGA generate the clocks Sampling Rate
  switch (fsref) {
    case SNDRV_PCM_RATE_32000:
      divreg = 0x0C;
      break;
    case SNDRV_PCM_RATE_44100:
    case SNDRV_PCM_RATE_48000:
      divreg = 0x08;
      break;
    case SNDRV_PCM_RATE_64000:
      divreg = 0x06;
      break;
    case SNDRV_PCM_RATE_88200:
    case SNDRV_PCM_RATE_96000:
      divreg = 0x04;
      break;
    case SNDRV_PCM_RATE_176400:
    case SNDRV_PCM_RATE_192000:
      divreg = 0x02;
      break;
    default:
      printk(KERN_ERR "sdm2x-codec.%s(): Requested rate 0x%08x not supported\n", __func__, fsref);
      return -EINVAL;
  }
  if (fsref % 11025 == 0) divreg |= 0x10;

  snd_soc_update_bits(codec, SDM2X_REG1_MUXCLK, 0x1F, divreg);

  return 0;
}



// DAI Ops

static int sdm2x_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int fmt)
{
  //struct snd_soc_codec *codec = codec_dai->codec;
  //struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);

  // SDM-50 only supports vanilla I2S format.
  switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
    case (SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF):
      break;
    default:
      printk(KERN_ERR "sdm2x-codec.%s(): Requested Format 0x%08x not supported\n", __func__, (fmt & SND_SOC_DAIFMT_FORMAT_MASK));
      return -EINVAL;
  }

  // SDM-50 Uses normal I2S Clock polarities
  switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
    case SND_SOC_DAIFMT_NB_NF:
      break;
    default:
      printk(KERN_ERR "sdm2x-codec.%s(): Requested Clock Polarity 0x%08x not supported\n", __func__, (fmt & SND_SOC_DAIFMT_INV_MASK));
      return -EINVAL;
  }

  // And SDM-50 is the I2S Clock Master for both Bit and Frame
  switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
    case (SND_SOC_DAIFMT_CBM_CFM):
      break;
    default:
      printk(KERN_ERR "sdm2x-codec.%s(): Requested Master/Slave Clock 0x%08x not supported\n", __func__, (fmt & SND_SOC_DAIFMT_MASTER_MASK));
      return -EINVAL;
  }

  return 0;
}



static int sdm2x_set_dai_sysclk(struct snd_soc_dai *codec_dai, int clk_id, unsigned int freq, int dir)
{
  struct snd_soc_codec *codec = codec_dai->codec;
  struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);
  sdm2x->sysclk = freq;
  return 0;
}




static const struct snd_soc_dai_ops sdm2x_dai_ops = {
  .hw_params    = sdm2x_hw_params,
  .set_fmt      = sdm2x_set_dai_fmt,
  .set_sysclk   = sdm2x_set_dai_sysclk,
  //.digital_mute = sdm2x_mute,
};


static struct snd_soc_dai_driver sdm2x_dai = {
  .name = "sdm2x-hifi",
  .playback = {
    .stream_name = "Playback",
    .channels_min = 1,
    .channels_max = 2,
    .rates = SDM2X_RATES,
    .formats = SDM2X_FORMATS,
  },
  .capture = {
    .stream_name = "Capture",
    .channels_min = 1,
    .channels_max = 2,
    .rates = SDM2X_RATES,
    .formats = SDM2X_FORMATS,
  },
  .ops = &sdm2x_dai_ops,
  .symmetric_rates = 1,
};



static int sdm2x_suspend(struct snd_soc_codec *codec)
{
  return 0;
}

static int sdm2x_resume(struct snd_soc_codec *codec)
{
  return 0;
}




/* PROBE: Setup private data, initialize codec and attach it to ASoC */
static int sdm2x_probe(struct snd_soc_codec *codec)
{
  int ret;
  struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);

  sdm2x->codec = codec;

  // This belongs in the core but not sure if 3.8.13 supports it already..
  ret = snd_soc_codec_set_cache_io(codec, 8, 8, sdm2x->control_type);
  if (ret != 0) {
    printk(KERN_ERR "sdm2x-codec.%s(): Failed to set cache I/O: %d\n", __func__, ret);
    return ret;
  }

  //codec->cache_only = 1;

  // Init...
  snd_soc_write(codec, SDM2X_REG0_VOLUME, 0);
  snd_soc_write(codec, SDM2X_REG1_MUXCLK, (SDM2X_INPUT_EXTERNAL | SDM2X_PLAYRATE_48000));
  //snd_soc_update_bits(codec, SDM250_REG2_STATUS, SOFT_RESET, SOFT_RESET);  // Auto Clears after reset complete (might not work with cached registers)

  // Add AMIXER controls (Volume, etc..)
  snd_soc_add_codec_controls(codec, sdm2x_snd_controls, ARRAY_SIZE(sdm2x_snd_controls));

  return 0;
}


static int sdm2x_remove(struct snd_soc_codec *codec)
{
  // struct sdm2x_priv *sdm2x = snd_soc_codec_get_drvdata(codec);
  // TODO: Where is private data freed up?
  return 0;
}


static struct snd_soc_codec_driver soc_codec_dev_sdm2x = {
  .reg_cache_size = ARRAY_SIZE(sdm2x_reg_cache),
  .reg_word_size = sizeof(u8),
  .reg_cache_default = sdm2x_reg_cache,
  .probe = sdm2x_probe,
  .remove = sdm2x_remove,
  .suspend = sdm2x_suspend,
  .resume = sdm2x_resume,
};



static const struct i2c_device_id sdm2x_i2c_id[] = {
    { "sdm2x", BIRDLAND_MODEL_250 },
    { }
};
MODULE_DEVICE_TABLE(i2c, sdm2x_i2c_id);


/* If the i2c layer weren't so broken, we could pass this kind of data around */
static int sdm2x_i2c_probe(struct i2c_client *i2c, const struct i2c_device_id *id)
{
  struct sdm2x_priv *sdm2x;
  struct device_node *np = i2c->dev.of_node;
  int ret;

  sdm2x = devm_kzalloc(&i2c->dev, sizeof(struct sdm2x_priv), GFP_KERNEL);
  if (sdm2x == NULL) {
    printk(KERN_ERR "sdm2x-codec.%s(): Failed to set cache I/O\n", __func__);
    return -ENOMEM;
  }

  // This driver doesn't support the old style platform data
  if (!np) {
    printk(KERN_ERR "sdm2x-codec.%s(): missing DT data info\n", __func__);
    return -EINVAL;
  }

  sdm2x->control_type = SND_SOC_I2C;

  i2c_set_clientdata(i2c, sdm2x);

  sdm2x->model = id->driver_data;

  ret = snd_soc_register_codec(&i2c->dev, &soc_codec_dev_sdm2x, &sdm2x_dai, 1);
  return ret;
}

static int sdm2x_i2c_remove(struct i2c_client *client)
{
  // TODO: Where is private data freed up?
  snd_soc_unregister_codec(&client->dev);
  return 0;
}

#if defined(CONFIG_OF)
static const struct of_device_id sdm2x_of_match[] = {
  { .compatible = "birdland,sdm2x", },
  {},
};
MODULE_DEVICE_TABLE(of, sdm2x_of_match);
#endif

/* machine i2c codec control layer */
static struct i2c_driver sdm2x_i2c_driver = {
  .driver = {
    .name = "sdm2x-codec",
    .owner = THIS_MODULE,
    .of_match_table = of_match_ptr(sdm2x_of_match),
  },
  .probe = sdm2x_i2c_probe,
  .remove = sdm2x_i2c_remove,
  .id_table = sdm2x_i2c_id,
};

module_i2c_driver(sdm2x_i2c_driver);

MODULE_DESCRIPTION("ASoC SDM2x codec driver");
MODULE_AUTHOR("Gilles Gameiro");
MODULE_LICENSE("GPL");

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Audio codec with multiple I2C devices
  2014-04-04 22:02 ` Audio codec with multiple I2C devices Gilles
@ 2014-04-06  9:15   ` Lars-Peter Clausen
  2014-04-07 11:37     ` Mark Brown
  0 siblings, 1 reply; 5+ messages in thread
From: Lars-Peter Clausen @ 2014-04-06  9:15 UTC (permalink / raw)
  To: Gilles; +Cc: alsa-devel

On 04/05/2014 12:02 AM, Gilles wrote:
> Folks,
>
> I'm writing an ALSA SoC codec which uses a 3 distinct I2C devices (a DAC, a CAG for volume and a input selector) all 3 at different I2C addresses.

If those are 3 distinct devices you'd typically implement them as 3 
different drivers and model the interconnections between them at the 
board/machine driver level.

- Lars

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Audio codec with multiple I2C devices
  2014-04-06  9:15   ` Lars-Peter Clausen
@ 2014-04-07 11:37     ` Mark Brown
  2014-04-07 18:32       ` Gilles
  0 siblings, 1 reply; 5+ messages in thread
From: Mark Brown @ 2014-04-07 11:37 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: alsa-devel, Gilles


[-- Attachment #1.1: Type: text/plain, Size: 1034 bytes --]

On Sun, Apr 06, 2014 at 11:15:11AM +0200, Lars-Peter Clausen wrote:
> On 04/05/2014 12:02 AM, Gilles wrote:

Always CC kernel related mails to relevant maintainers, it's very hit
and miss if things on the list only will get seen.

> >I'm writing an ALSA SoC codec which uses a 3 distinct I2C devices (a
> >DAC, a CAG for volume and a input selector) all 3 at different I2C
> >addresses.

> If those are 3 distinct devices you'd typically implement them as 3
> different drivers and model the interconnections between them at the
> board/machine driver level.

If they're hard wired together then i2c_new_dummy() allows a single
driver to control multiple slaves - have the driver instantiate from one
of the addresses and then register dummies for the other addresses.  The
framework is probably still going to want to see this registered as a
series of CODECs though unless that's extended.  In any case the
internal mappings for the devices ought to at least have helpers
available so machine drivers don't need to reinvent things.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Audio codec with multiple I2C devices
  2014-04-07 11:37     ` Mark Brown
@ 2014-04-07 18:32       ` Gilles
       [not found]         ` <5342F68B.9060801@metafoo.de>
  0 siblings, 1 reply; 5+ messages in thread
From: Gilles @ 2014-04-07 18:32 UTC (permalink / raw)
  To: Mark Brown; +Cc: alsa-devel, Lars-Peter Clausen


On Apr 7, 2014, at 04:37 , Mark Brown <broonie@kernel.org> wrote:

> On Sun, Apr 06, 2014 at 11:15:11AM +0200, Lars-Peter Clausen wrote:
>> On 04/05/2014 12:02 AM, Gilles wrote:
> 
> Always CC kernel related mails to relevant maintainers, it's very hit
> and miss if things on the list only will get seen.
> 
>>> I'm writing an ALSA SoC codec which uses a 3 distinct I2C devices (a
>>> DAC, a CAG for volume and a input selector) all 3 at different I2C
>>> addresses.
> 
>> If those are 3 distinct devices you'd typically implement them as 3
>> different drivers and model the interconnections between them at the
>> board/machine driver level.
> 
> If they're hard wired together then i2c_new_dummy() allows a single
> driver to control multiple slaves - have the driver instantiate from one
> of the addresses and then register dummies for the other addresses.  The
> framework is probably still going to want to see this registered as a
> series of CODECs though unless that's extended.  In any case the
> internal mappings for the devices ought to at least have helpers
> available so machine drivers don't need to reinvent things.

Yes, it would make much more sense for it to be all in the same driver for two reasons. (1) The only reason why there are multiple I2C are because the adapter has a novelty way of providing certain functions (like Volume) and there are no Codec chips on the market with that built-in. And (2) the functionality of - say the secondary I2C device - is only a simple control (in this case, a general volume which happens to be at a different I2C address.

Doesn't it make perfect sense to have the Volume be a control of the Codec driver even if it's provided by a separate chip on the same sound card?

I'm going to look into the suggested i2c_new_dummy(), I don't really want to write a separate volume codec. I was just wondering what would be the best way to make it DT friendly but I can see that's pretty much up to me to define my own DT property for the I2C address of the secondary chip.

Thanks for the pointer.
Gilles
.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Audio codec with multiple I2C devices
       [not found]           ` <81C8B318-CBB6-4862-A779-E0EBFACA48C6@whospot.com>
@ 2014-04-08  6:40             ` Lars-Peter Clausen
  0 siblings, 0 replies; 5+ messages in thread
From: Lars-Peter Clausen @ 2014-04-08  6:40 UTC (permalink / raw)
  To: Gilles; +Cc: alsa-devel, Mark Brown

On 04/07/2014 09:29 PM, Gilles wrote:
> On Apr 7, 2014, at 12:03 , Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> On 04/07/2014 08:32 PM, Gilles wrote:
>> [...]
>>> Doesn't it make perfect sense to have the Volume be a control of the Codec driver even if it's provided by a separate chip on the same sound card?
>>
>> Not if they are standalone components that can also be used in different configurations on different sound cards. E.g. you code suggests that you are using a AK4113 and a DAC121C085. The CODEC driver is not the sound card driver, the sound card driver is the piece of code that glues all the different components that are found on the sound card together.
>>
>> i2c_new_dummy() is typically used for devices with multiple I2C addresses. Or where you have e.g. multiple devices with different addresses on the same die so they become a inseparable unit.
>>
>> - Lars
>
> Yes, but the functionality is very specific to the way the components are wired on the hardware. They are wired to work in conjunction with the ASIC Codec.
>
> The DAC121C085 is a very specific case as the DAC is actually wired to control the voltage of 4 quadrant multiplier which is hardwired to the Codec ASIC. This is a prototype. In the final product, the DAC chip may be built into the ASIC. For this one, I feel like the "Volume" should be a control of the same driver as the ASIC.
>
> This said I see your point about the AK4113 which is a generic multi channel receiver. In this specific application it is wired so only inputs 1,2 and 3 are hard wired (this actually isn't a sound card, but rather a finished product using Linux as the SoC OS as can be seen on the home page of http://birdland.com right now).
>
> There is already a working prototype, but as of now, the inputs are selected by the application talking directly to the I2C AKM. I'm just in the process of streamlining the code, and naturally, I envisioned the input selection be part of the sound card codec as I'd much rather have the sound card have a enum control with the 4 inputs shown by name show under amixer (this would enable my application to be more generic).
>
> Can you please point me to an example of a Sound board driver that uses multiple codecs? I suppose I can modify my sound board driver to use a codec for the audio and another for the AKM input selector).

Anything that uses aux devs. E.g. samsung/speyside.c
There is also this series https://lkml.org/lkml/2014/4/5/92 which adds DT 
support for aux devs.

- Lars

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-04-08  6:40 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <mailman.300.1396595329.806.alsa-devel@alsa-project.org>
2014-04-04 22:02 ` Audio codec with multiple I2C devices Gilles
2014-04-06  9:15   ` Lars-Peter Clausen
2014-04-07 11:37     ` Mark Brown
2014-04-07 18:32       ` Gilles
     [not found]         ` <5342F68B.9060801@metafoo.de>
     [not found]           ` <81C8B318-CBB6-4862-A779-E0EBFACA48C6@whospot.com>
2014-04-08  6:40             ` Lars-Peter Clausen

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.