Message ID | 20191209183511.3576038-12-daniel@zonque.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | mfd: Add support for Analog Devices A2B transceiver | expand |
On Mon, Dec 09, 2019 at 07:35:11PM +0100, Daniel Mack wrote: > + /* > + * Setting clock inversion is only supported globally for both DAIs, > + * so we ignore the settings made for DAI1 here. > + */ > + if (index == 0) { > + switch (format & SND_SOC_DAIFMT_INV_MASK) { > + case SND_SOC_DAIFMT_NB_NF: I dunno if it's a blocker but it'd feel nicer to try to verify that the settings are the same and warn if not. > +static int ad242x_set_dai_fmt_dai0(struct snd_soc_dai *codec_dai, > + unsigned int format) > +{ > + return ad242x_set_dai_fmt(codec_dai, format, 0); > +} > + > +static int ad242x_set_dai_fmt_dai1(struct snd_soc_dai *codec_dai, > + unsigned int format) > +{ > + return ad242x_set_dai_fmt(codec_dai, format, 1); > +} You don't need separate ops, just look at dai->id. > +module_platform_driver(ad242x_platform_driver); > + > +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>"); > +MODULE_DESCRIPTION("AD242X ALSA SoC driver"); > +MODULE_LICENSE("GPL"); Should have a MODULE_ALIAS() as well for autoloading.
On 12/9/19 12:35 PM, Daniel Mack wrote: > This driver makes AD242x nodes available as DAIs in ASoC topologies. > > The hardware allows multiple TDM channel modes and bitdepths, but > as these modes have influence in the timing calculations at discovery > time, the mode in that the will be used in needs to be configured the mode in that the <what> will be used in? You should probably reword this for clarity. > statically in the devicetree. > + if (ad242x_node_is_master(priv->node) && > + ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBS_CFS)) { > + dev_err(component->dev, "master node must be clock slave\n"); > + return -EINVAL; > + } > + > + if (!ad242x_node_is_master(priv->node) && > + ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)) { > + dev_err(component->dev, "slave node must be clock master\n"); > + return -EINVAL; > + } It was my understanding that the master node provides the clock to the bus, so not sure how it could be a clock slave, and conversely how a slave node could provide a clock to the bus? > + switch (params_format(params)) { > + case SNDRV_PCM_FORMAT_S16_LE: > + if (priv->node->tdm_slot_size != 16) > + return -EINVAL; > + break; > + case SNDRV_PCM_FORMAT_S32_LE: > + if (priv->node->tdm_slot_size != 32) > + return -EINVAL; > + break; > + default: > + return -EINVAL; > + } how does this work for PDM data? is the PDM data packed into a regular TDM slot? > + > + if (priv->pdm[index]) { > + if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK) > + return -EINVAL; > + > + if (index == 0) { > + val = AD242X_PDMCTL_PDM0EN; > + mask = AD242X_PDMCTL_PDM0EN | AD242X_PDMCTL_PDM0SLOTS; > + } else { > + val = AD242X_PDMCTL_PDM1EN; > + mask = AD242X_PDMCTL_PDM1EN | AD242X_PDMCTL_PDM1SLOTS; > + } > + > + switch (params_channels(params)) { > + case 1: > + break; > + case 2: > + val = mask; > + break; A comment wouldn't hurt here...
Hi, On 12/17/19 8:28 PM, Pierre-Louis Bossart wrote: > On 12/9/19 12:35 PM, Daniel Mack wrote: >> + if (!ad242x_node_is_master(priv->node) && >> + ((format & SND_SOC_DAIFMT_MASTER_MASK) != >> SND_SOC_DAIFMT_CBM_CFM)) { >> + dev_err(component->dev, "slave node must be clock master\n"); >> + return -EINVAL; >> + } > > It was my understanding that the master node provides the clock to the > bus, so not sure how it could be a clock slave, and conversely how a > slave node could provide a clock to the bus? The slave nodes receive the A2B clock from the master node and then produce digital audio output that is sent to other components such as codecs. Hence, in ASoC terms, they are the clock master. Likewise, as the master node is receiving its clock from other components, it has to be a clock slave in the audio network. Does that make sense? >> + switch (params_format(params)) { >> + case SNDRV_PCM_FORMAT_S16_LE: >> + if (priv->node->tdm_slot_size != 16) >> + return -EINVAL; >> + break; >> + case SNDRV_PCM_FORMAT_S32_LE: >> + if (priv->node->tdm_slot_size != 32) >> + return -EINVAL; >> + break; >> + default: >> + return -EINVAL; >> + } > > how does this work for PDM data? > > is the PDM data packed into a regular TDM slot? Yes. But I admit this needs some more testing. We're still working on the hardware that uses this. I'll revisit this. And I'll also add a lot more comments all over the place, as also requested by Lee. Thanks, Daniel
On 12/18/19 3:49 AM, Daniel Mack wrote: > Hi, > > On 12/17/19 8:28 PM, Pierre-Louis Bossart wrote: >> On 12/9/19 12:35 PM, Daniel Mack wrote: > >>> + if (!ad242x_node_is_master(priv->node) && >>> + ((format & SND_SOC_DAIFMT_MASTER_MASK) != >>> SND_SOC_DAIFMT_CBM_CFM)) { >>> + dev_err(component->dev, "slave node must be clock master\n"); >>> + return -EINVAL; >>> + } >> >> It was my understanding that the master node provides the clock to the >> bus, so not sure how it could be a clock slave, and conversely how a >> slave node could provide a clock to the bus? > > The slave nodes receive the A2B clock from the master node and then > produce digital audio output that is sent to other components such as > codecs. Hence, in ASoC terms, they are the clock master. > > Likewise, as the master node is receiving its clock from other > components, it has to be a clock slave in the audio network. > > Does that make sense? Your slave node acts as a bridge then, but it seems you don't model the bus-facing interface, which has to follow the master clock. Or do you? Likewise the master has an 'SOC-facing' interface and a bus-facing interface. it *could* be master on both if ASRC was supported. The point is that the bus-facing interface is not clock slave.
Hi Pierre, [reducing the recipient list ALSA people] On 12/18/19 4:32 PM, Pierre-Louis Bossart wrote: > On 12/18/19 3:49 AM, Daniel Mack wrote: >> Hi, >> >> On 12/17/19 8:28 PM, Pierre-Louis Bossart wrote: >>> On 12/9/19 12:35 PM, Daniel Mack wrote: >> >>>> + if (!ad242x_node_is_master(priv->node) && >>>> + ((format & SND_SOC_DAIFMT_MASTER_MASK) != >>>> SND_SOC_DAIFMT_CBM_CFM)) { >>>> + dev_err(component->dev, "slave node must be clock master\n"); >>>> + return -EINVAL; >>>> + } >>> >>> It was my understanding that the master node provides the clock to the >>> bus, so not sure how it could be a clock slave, and conversely how a >>> slave node could provide a clock to the bus? >> >> The slave nodes receive the A2B clock from the master node and then >> produce digital audio output that is sent to other components such as >> codecs. Hence, in ASoC terms, they are the clock master. >> >> Likewise, as the master node is receiving its clock from other >> components, it has to be a clock slave in the audio network. >> >> Does that make sense? > > Your slave node acts as a bridge then, but it seems you don't model the > bus-facing interface, which has to follow the master clock. Or do you? Yes, the driver currently only models the SOC-facing side, and that follows the 'reverse' clocking scheme: * The master node always receives the clock on the SOC-facing side, and produces the clock on the bus-facing side. * The slave node always receives the clock on the bus-facing side, and produces the clock on the SOC-facing side. I currently don't see a reason for modelling the bus-facing side in the ASoC topology at all, but of course that could be added. But for the SOC-facing side on *slave* nodes, the currently implemented logic should be correct, no? Do you think it makes sense to add the bus-side as well? > Likewise the master has an 'SOC-facing' interface and a bus-facing > interface. it *could* be master on both if ASRC was supported. The point > is that the bus-facing interface is not clock slave. That's right, I need to look into the modes for the master node again. Maybe the check needs to be relaxed on that end. Thanks, Daniel
> Yes, the driver currently only models the SOC-facing side, and that > follows the 'reverse' clocking scheme: > > * The master node always receives the clock on the SOC-facing side, and > produces the clock on the bus-facing side. > * The slave node always receives the clock on the bus-facing side, and > produces the clock on the SOC-facing side. I thought the SOC would always be connected to a master node since all bus allocation/configurations required a bit of intelligence. Does your driver model the case when an SOC would run an ALSA/ASoC driver handling data produced/consumed by a A2B slave? Who would control the A2B master then? > I currently don't see a reason for modelling the bus-facing side in the > ASoC topology at all, but of course that could be added. > > But for the SOC-facing side on *slave* nodes, the currently implemented > logic should be correct, no? Do you think it makes sense to add the > bus-side as well? > >> Likewise the master has an 'SOC-facing' interface and a bus-facing >> interface. it *could* be master on both if ASRC was supported. The point >> is that the bus-facing interface is not clock slave. > > That's right, I need to look into the modes for the master node again. > Maybe the check needs to be relaxed on that end. Your questions are interesting, I am not sure I have answers. the ASoC clock definitions are usually 'codec-centric', but when a slave acts as a bridge and has soc- and audio-facing interfaces, and the latter one connects to say an amplifier, then what is the reference point? Or should all segments be considered independent?
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 4abf37b5083f..75365abc277f 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -22,6 +22,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_AD193X_SPI if SPI_MASTER select SND_SOC_AD193X_I2C if I2C select SND_SOC_AD1980 if SND_SOC_AC97_BUS + select SND_SOC_AD242X if MFD_AD242X select SND_SOC_AD73311 select SND_SOC_ADAU1373 if I2C select SND_SOC_ADAU1761_I2C if I2C @@ -333,6 +334,10 @@ config SND_SOC_AD1980 select REGMAP_AC97 tristate +config SND_SOC_AD242X + tristate "Analog Devices AD242x CODEC" + depends on MFD_AD242X + config SND_SOC_AD73311 tristate diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index ddfd07071925..ec76448fc1da 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -7,6 +7,7 @@ snd-soc-ad193x-objs := ad193x.o snd-soc-ad193x-spi-objs := ad193x-spi.o snd-soc-ad193x-i2c-objs := ad193x-i2c.o snd-soc-ad1980-objs := ad1980.o +snd-soc-ad242x-objs := ad242x.o snd-soc-ad73311-objs := ad73311.o snd-soc-adau-utils-objs := adau-utils.o snd-soc-adau1373-objs := adau1373.o @@ -294,6 +295,7 @@ obj-$(CONFIG_SND_SOC_AD193X) += snd-soc-ad193x.o obj-$(CONFIG_SND_SOC_AD193X_SPI) += snd-soc-ad193x-spi.o obj-$(CONFIG_SND_SOC_AD193X_I2C) += snd-soc-ad193x-i2c.o obj-$(CONFIG_SND_SOC_AD1980) += snd-soc-ad1980.o +obj-$(CONFIG_SND_SOC_AD242X) += snd-soc-ad242x.o obj-$(CONFIG_SND_SOC_AD73311) += snd-soc-ad73311.o obj-$(CONFIG_SND_SOC_ADAU_UTILS) += snd-soc-adau-utils.o obj-$(CONFIG_SND_SOC_ADAU1373) += snd-soc-adau1373.o diff --git a/sound/soc/codecs/ad242x.c b/sound/soc/codecs/ad242x.c new file mode 100644 index 000000000000..76189a7c3c92 --- /dev/null +++ b/sound/soc/codecs/ad242x.c @@ -0,0 +1,338 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +#include <linux/device.h> +#include <linux/module.h> +#include <linux/mfd/ad242x.h> +#include <linux/of_device.h> +#include <linux/slab.h> +#include <sound/asoundef.h> +#include <sound/core.h> +#include <sound/initval.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> + +struct ad242x_private { + struct ad242x_node *node; + bool pdm[2]; + bool pdm_highpass; +}; + +static const struct snd_soc_dapm_widget ad242x_dapm_widgets[] = { + SND_SOC_DAPM_AIF_IN("RX0", NULL, 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_AIF_IN("RX1", NULL, 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_AIF_OUT("TX0", NULL, 0, SND_SOC_NOPM, 0, 0), + SND_SOC_DAPM_AIF_OUT("TX1", NULL, 0, SND_SOC_NOPM, 0, 0), +}; + +static const struct snd_soc_dapm_route ad242x_dapm_routes[] = { + { "DAI0 Playback", NULL, "RX0" }, + { "TX0", NULL, "DAI0 Capture" }, + { "DAI1 Playback", NULL, "RX1" }, + { "TX1", NULL, "DAI1 Capture" }, +}; + +static int ad242x_set_dai_fmt(struct snd_soc_dai *codec_dai, + unsigned int format, + unsigned int index) +{ + struct snd_soc_component *component = codec_dai->component; + struct ad242x_private *priv = snd_soc_component_get_drvdata(component); + int ret, val = 0; + + /* set DAI format */ + switch (format & SND_SOC_DAIFMT_FORMAT_MASK) { + case SND_SOC_DAIFMT_I2S: + priv->pdm[index] = false; + break; + case SND_SOC_DAIFMT_PDM: + priv->pdm[index] = true; + break; + default: + dev_err(component->dev, "unsupported dai format\n"); + return -EINVAL; + } + + /* + * Setting clock inversion is only supported globally for both DAIs, + * so we ignore the settings made for DAI1 here. + */ + if (index == 0) { + switch (format & SND_SOC_DAIFMT_INV_MASK) { + case SND_SOC_DAIFMT_NB_NF: + val = 0; + break; + case SND_SOC_DAIFMT_IB_NF: + val = AD242X_I2SCTL_RXBCLKINV; + break; + case SND_SOC_DAIFMT_NB_IF: + case SND_SOC_DAIFMT_IB_IF: + dev_err(component->dev, "unsupported inversion mask\n"); + return -EINVAL; + } + + ret = regmap_update_bits(priv->node->regmap, AD242X_I2SCTL, + AD242X_I2SCTL_RXBCLKINV, val); + if (ret < 0) + return ret; + } + + if (ad242x_node_is_master(priv->node) && + ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBS_CFS)) { + dev_err(component->dev, "master node must be clock slave\n"); + return -EINVAL; + } + + if (!ad242x_node_is_master(priv->node) && + ((format & SND_SOC_DAIFMT_MASTER_MASK) != SND_SOC_DAIFMT_CBM_CFM)) { + dev_err(component->dev, "slave node must be clock master\n"); + return -EINVAL; + } + + return 0; +} + +static int ad242x_set_dai_fmt_dai0(struct snd_soc_dai *codec_dai, + unsigned int format) +{ + return ad242x_set_dai_fmt(codec_dai, format, 0); +} + +static int ad242x_set_dai_fmt_dai1(struct snd_soc_dai *codec_dai, + unsigned int format) +{ + return ad242x_set_dai_fmt(codec_dai, format, 1); +} + +static int ad242x_hw_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai, + int index) +{ + struct snd_soc_component *component = dai->component; + struct ad242x_private *priv = snd_soc_component_get_drvdata(component); + unsigned int sff_rate = ad242x_master_get_clk_rate(priv->node->master); + unsigned int rate = params_rate(params); + unsigned int val, mask; + int ret; + + switch (params_format(params)) { + case SNDRV_PCM_FORMAT_S16_LE: + if (priv->node->tdm_slot_size != 16) + return -EINVAL; + break; + case SNDRV_PCM_FORMAT_S32_LE: + if (priv->node->tdm_slot_size != 32) + return -EINVAL; + break; + default: + return -EINVAL; + } + + if (priv->pdm[index]) { + if (substream->stream != SNDRV_PCM_STREAM_PLAYBACK) + return -EINVAL; + + if (index == 0) { + val = AD242X_PDMCTL_PDM0EN; + mask = AD242X_PDMCTL_PDM0EN | AD242X_PDMCTL_PDM0SLOTS; + } else { + val = AD242X_PDMCTL_PDM1EN; + mask = AD242X_PDMCTL_PDM1EN | AD242X_PDMCTL_PDM1SLOTS; + } + + switch (params_channels(params)) { + case 1: + break; + case 2: + val = mask; + break; + default: + return -EINVAL; + } + + mask |= AD242X_PDMCTL_HPFEN; + if (priv->pdm_highpass) + val |= AD242X_PDMCTL_HPFEN; + + ret = regmap_update_bits(priv->node->regmap, AD242X_PDMCTL, + mask, val); + if (ret < 0) + return ret; + } else { + if (params_channels(params) != priv->node->tdm_mode) + return -EINVAL; + + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + if (index == 0) + mask = AD242X_I2SCTL_RX0EN; + else + mask = AD242X_I2SCTL_RX1EN; + } else { + if (index == 0) + mask = AD242X_I2SCTL_TX0EN; + else + mask = AD242X_I2SCTL_TX1EN; + } + + ret = regmap_update_bits(priv->node->regmap, AD242X_I2SCTL, + mask, mask); + if (ret < 0) + return ret; + } + + if (!ad242x_node_is_master(priv->node)) { + val = 0; + + if (rate == sff_rate / 2) + val = AD242X_I2SRATE_I2SRATE(1); + else if (rate == sff_rate / 4) + val = AD242X_I2SRATE_I2SRATE(2); + else if (rate == sff_rate * 2) + val = AD242X_I2SRATE_I2SRATE(5); + else if (rate == sff_rate * 4) + val = AD242X_I2SRATE_I2SRATE(6); + else if (rate != sff_rate) + return -EINVAL; + + ret = regmap_write(priv->node->regmap, AD242X_I2SRATE, val); + if (ret < 0) + return ret; + } + + return 0; +} + +static int ad242x_hw_params_dai0(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + return ad242x_hw_params(substream, params, dai, 0); +} + +static int ad242x_hw_params_dai1(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params, + struct snd_soc_dai *dai) +{ + return ad242x_hw_params(substream, params, dai, 1); +} + +static const struct snd_soc_dai_ops ad242x_dai0_ops = { + .hw_params = ad242x_hw_params_dai0, + .set_fmt = ad242x_set_dai_fmt_dai0, +}; + +static const struct snd_soc_dai_ops ad242x_dai1_ops = { + .hw_params = ad242x_hw_params_dai1, + .set_fmt = ad242x_set_dai_fmt_dai1, +}; + +#define AD242X_RATES ( \ + SNDRV_PCM_RATE_22050 | SNDRV_PCM_RATE_32000 | \ + SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 | \ + SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \ + SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000) +#define AD242X_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S32_LE) + +static struct snd_soc_dai_driver ad242x_dai[] = { + { + .name = "ad242x-dai0", + .playback = { + .stream_name = "DAI0 Playback", + .channels_min = 1, + .channels_max = 32, + .rates = AD242X_RATES, + .formats = AD242X_FORMATS, + }, + .capture = { + .stream_name = "DAI0 Capture", + .channels_min = 1, + .channels_max = 32, + .rates = AD242X_RATES, + .formats = AD242X_FORMATS, + }, + .ops = &ad242x_dai0_ops, + }, + { + .name = "ad242x-dai1", + .playback = { + .stream_name = "DAI1 Playback", + .channels_min = 1, + .channels_max = 32, + .rates = AD242X_RATES, + .formats = AD242X_FORMATS, + }, + .capture = { + .stream_name = "DAI1 Capture", + .channels_min = 1, + .channels_max = 32, + .rates = AD242X_RATES, + .formats = AD242X_FORMATS, + }, + .ops = &ad242x_dai1_ops, + }, +}; + +static int ad242x_soc_probe(struct snd_soc_component *component) +{ + struct ad242x_private *priv = snd_soc_component_get_drvdata(component); + + component->regmap = priv->node->regmap; + + return 0; +} + +static const struct snd_soc_component_driver soc_component_device_ad242x = { + .probe = ad242x_soc_probe, + .dapm_widgets = ad242x_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(ad242x_dapm_widgets), + .dapm_routes = ad242x_dapm_routes, + .num_dapm_routes = ARRAY_SIZE(ad242x_dapm_routes), + .idle_bias_on = 1, + .use_pmdown_time = 1, + .endianness = 1, + .non_legacy_dai_naming = 1, +}; + +static int ad242x_codec_platform_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct ad242x_private *priv; + + if (!dev->of_node) + return -ENODEV; + + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->node = dev_get_drvdata(dev->parent); + platform_set_drvdata(pdev, priv); + + priv->pdm_highpass = of_property_read_bool(dev->of_node, + "adi,pdm-highpass-filter"); + + return devm_snd_soc_register_component(dev, + &soc_component_device_ad242x, + ad242x_dai, + ARRAY_SIZE(ad242x_dai)); +} + +static const struct of_device_id ad242x_of_match[] = { + { .compatible = "adi,ad2428w-codec", }, + { } +}; +MODULE_DEVICE_TABLE(of, ad242x_of_match); + +static struct platform_driver ad242x_platform_driver = { + .driver = { + .name = "ad242x-codec", + .of_match_table = ad242x_of_match, + }, + .probe = ad242x_codec_platform_probe, +}; + +module_platform_driver(ad242x_platform_driver); + +MODULE_AUTHOR("Daniel Mack <daniel@zonque.org>"); +MODULE_DESCRIPTION("AD242X ALSA SoC driver"); +MODULE_LICENSE("GPL");
This driver makes AD242x nodes available as DAIs in ASoC topologies. The hardware allows multiple TDM channel modes and bitdepths, but as these modes have influence in the timing calculations at discovery time, the mode in that the will be used in needs to be configured statically in the devicetree. The configuration applied at runtime through hwparams() is then required to match the pre-configured settings. Signed-off-by: Daniel Mack <daniel@zonque.org> --- sound/soc/codecs/Kconfig | 5 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/ad242x.c | 338 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 345 insertions(+) create mode 100644 sound/soc/codecs/ad242x.c