The patch

ASoC: pcm1792a: Rename internal data and function to pcm179x

has been applied to the asoc tree at

git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git

All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying to this mail.

Thanks, Mark

From 823733b91619aef5a2be21d0918ef6dd996de72a Mon Sep 17 00:00:00 2001

From: Michael Trimarchi michael@amarulasolutions.com Date: Sun, 10 Jan 2016 00:38:03 +0100 Subject: [PATCH] ASoC: pcm1792a: Rename internal data and function to pcm179x

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/codecs/pcm1792a.c | 158 ++++++++++++++++++++++---------------------- sound/soc/codecs/pcm1792a.h | 6 +- 2 files changed, 82 insertions(+), 82 deletions(-)

diff --git a/sound/soc/codecs/pcm1792a.c b/sound/soc/codecs/pcm1792a.c index 08bb4863e96f..a56c7b767d90 100644 --- a/sound/soc/codecs/pcm1792a.c +++ b/sound/soc/codecs/pcm1792a.c @@ -1,5 +1,5 @@ /* - * PCM1792A ASoC codec driver + * PCM179X ASoC codec driver * * Copyright (c) Amarula Solutions B.V. 2013 * @@ -31,21 +31,21 @@ #include <linux/of.h> #include <linux/of_device.h>

-#include "pcm1792a.h" +#include "pcm179x.h"

-#define PCM1792A_DAC_VOL_LEFT 0x10 -#define PCM1792A_DAC_VOL_RIGHT 0x11 -#define PCM1792A_FMT_CONTROL 0x12 -#define PCM1792A_MODE_CONTROL 0x13 -#define PCM1792A_SOFT_MUTE PCM1792A_FMT_CONTROL +#define PCM179X_DAC_VOL_LEFT 0x10 +#define PCM179X_DAC_VOL_RIGHT 0x11 +#define PCM179X_FMT_CONTROL 0x12 +#define PCM179X_MODE_CONTROL 0x13 +#define PCM179X_SOFT_MUTE PCM179X_FMT_CONTROL

-#define PCM1792A_FMT_MASK 0x70 -#define PCM1792A_FMT_SHIFT 4 -#define PCM1792A_MUTE_MASK 0x01 -#define PCM1792A_MUTE_SHIFT 0 -#define PCM1792A_ATLD_ENABLE (1 << 7) +#define PCM179X_FMT_MASK 0x70 +#define PCM179X_FMT_SHIFT 4 +#define PCM179X_MUTE_MASK 0x01 +#define PCM179X_MUTE_SHIFT 0 +#define PCM179X_ATLD_ENABLE (1 << 7)

-static const struct reg_default pcm1792a_reg_defaults[] = { +static const struct reg_default pcm179x_reg_defaults[] = { { 0x10, 0xff }, { 0x11, 0xff }, { 0x12, 0x50 }, @@ -56,57 +56,57 @@ static const struct reg_default pcm1792a_reg_defaults[] = { { 0x17, 0x00 }, };

-static bool pcm1792a_accessible_reg(struct device *dev, unsigned int reg) +static bool pcm179x_accessible_reg(struct device *dev, unsigned int reg) { return reg >= 0x10 && reg <= 0x17; }

-static bool pcm1792a_writeable_reg(struct device *dev, unsigned register reg) +static bool pcm179x_writeable_reg(struct device *dev, unsigned register reg) { bool accessible;

- accessible = pcm1792a_accessible_reg(dev, reg); + accessible = pcm179x_accessible_reg(dev, reg);

return accessible && reg != 0x16 && reg != 0x17; }

-struct pcm1792a_private { +struct pcm179x_private { struct regmap *regmap; unsigned int format; unsigned int rate; };

-static int pcm1792a_set_dai_fmt(struct snd_soc_dai *codec_dai, +static int pcm179x_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int format) { struct snd_soc_codec *codec = codec_dai->codec; - struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec); + struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec);

priv->format = format;

return 0; }

-static int pcm1792a_digital_mute(struct snd_soc_dai *dai, int mute) +static int pcm179x_digital_mute(struct snd_soc_dai *dai, int mute) { struct snd_soc_codec *codec = dai->codec; - struct pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec); + struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec); int ret;

- ret = regmap_update_bits(priv->regmap, PCM1792A_SOFT_MUTE, - PCM1792A_MUTE_MASK, !!mute); + ret = regmap_update_bits(priv->regmap, PCM179X_SOFT_MUTE, + PCM179X_MUTE_MASK, !!mute); if (ret < 0) return ret;

return 0; }

-static int pcm1792a_hw_params(struct snd_pcm_substream *substream, +static int pcm179x_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 pcm1792a_private *priv = snd_soc_codec_get_drvdata(codec); + struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec); int val = 0, ret;

priv->rate = params_rate(params); @@ -143,129 +143,129 @@ static int pcm1792a_hw_params(struct snd_pcm_substream *substream, return -EINVAL; }

- val = val << PCM1792A_FMT_SHIFT | PCM1792A_ATLD_ENABLE; + val = val << PCM179X_FMT_SHIFT | PCM179X_ATLD_ENABLE;

- ret = regmap_update_bits(priv->regmap, PCM1792A_FMT_CONTROL, - PCM1792A_FMT_MASK | PCM1792A_ATLD_ENABLE, val); + ret = regmap_update_bits(priv->regmap, PCM179X_FMT_CONTROL, + PCM179X_FMT_MASK | PCM179X_ATLD_ENABLE, val); if (ret < 0) return ret;

return 0; }

-static const struct snd_soc_dai_ops pcm1792a_dai_ops = { - .set_fmt = pcm1792a_set_dai_fmt, - .hw_params = pcm1792a_hw_params, - .digital_mute = pcm1792a_digital_mute, +static const struct snd_soc_dai_ops pcm179x_dai_ops = { + .set_fmt = pcm179x_set_dai_fmt, + .hw_params = pcm179x_hw_params, + .digital_mute = pcm179x_digital_mute, };

-static const DECLARE_TLV_DB_SCALE(pcm1792a_dac_tlv, -12000, 50, 1); +static const DECLARE_TLV_DB_SCALE(pcm179x_dac_tlv, -12000, 50, 1);

-static const struct snd_kcontrol_new pcm1792a_controls[] = { - SOC_DOUBLE_R_RANGE_TLV("DAC Playback Volume", PCM1792A_DAC_VOL_LEFT, - PCM1792A_DAC_VOL_RIGHT, 0, 0xf, 0xff, 0, - pcm1792a_dac_tlv), - SOC_SINGLE("DAC Invert Output Switch", PCM1792A_MODE_CONTROL, 7, 1, 0), - SOC_SINGLE("DAC Rolloff Filter Switch", PCM1792A_MODE_CONTROL, 1, 1, 0), +static const struct snd_kcontrol_new pcm179x_controls[] = { + SOC_DOUBLE_R_RANGE_TLV("DAC Playback Volume", PCM179X_DAC_VOL_LEFT, + PCM179X_DAC_VOL_RIGHT, 0, 0xf, 0xff, 0, + pcm179x_dac_tlv), + SOC_SINGLE("DAC Invert Output Switch", PCM179X_MODE_CONTROL, 7, 1, 0), + SOC_SINGLE("DAC Rolloff Filter Switch", PCM179X_MODE_CONTROL, 1, 1, 0), };

-static const struct snd_soc_dapm_widget pcm1792a_dapm_widgets[] = { +static const struct snd_soc_dapm_widget pcm179x_dapm_widgets[] = { SND_SOC_DAPM_OUTPUT("IOUTL+"), SND_SOC_DAPM_OUTPUT("IOUTL-"), SND_SOC_DAPM_OUTPUT("IOUTR+"), SND_SOC_DAPM_OUTPUT("IOUTR-"), };

-static const struct snd_soc_dapm_route pcm1792a_dapm_routes[] = { +static const struct snd_soc_dapm_route pcm179x_dapm_routes[] = { { "IOUTL+", NULL, "Playback" }, { "IOUTL-", NULL, "Playback" }, { "IOUTR+", NULL, "Playback" }, { "IOUTR-", NULL, "Playback" }, };

-static struct snd_soc_dai_driver pcm1792a_dai = { - .name = "pcm1792a-hifi", +static struct snd_soc_dai_driver pcm179x_dai = { + .name = "pcm179x-hifi", .playback = { .stream_name = "Playback", .channels_min = 2, .channels_max = 2, .rates = PCM1792A_RATES, .formats = PCM1792A_FORMATS, }, - .ops = &pcm1792a_dai_ops, + .ops = &pcm179x_dai_ops, };

-static const struct of_device_id pcm1792a_of_match[] = { +static const struct of_device_id pcm179x_of_match[] = { { .compatible = "ti,pcm1792a", }, { } }; -MODULE_DEVICE_TABLE(of, pcm1792a_of_match); +MODULE_DEVICE_TABLE(of, pcm179x_of_match);

-static const struct regmap_config pcm1792a_regmap = { +static const struct regmap_config pcm179x_regmap = { .reg_bits = 8, .val_bits = 8, .max_register = 23, - .reg_defaults = pcm1792a_reg_defaults, - .num_reg_defaults = ARRAY_SIZE(pcm1792a_reg_defaults), - .writeable_reg = pcm1792a_writeable_reg, - .readable_reg = pcm1792a_accessible_reg, + .reg_defaults = pcm179x_reg_defaults, + .num_reg_defaults = ARRAY_SIZE(pcm179x_reg_defaults), + .writeable_reg = pcm179x_writeable_reg, + .readable_reg = pcm179x_accessible_reg, };

-static struct snd_soc_codec_driver soc_codec_dev_pcm1792a = { - .controls = pcm1792a_controls, - .num_controls = ARRAY_SIZE(pcm1792a_controls), - .dapm_widgets = pcm1792a_dapm_widgets, - .num_dapm_widgets = ARRAY_SIZE(pcm1792a_dapm_widgets), - .dapm_routes = pcm1792a_dapm_routes, - .num_dapm_routes = ARRAY_SIZE(pcm1792a_dapm_routes), +static struct snd_soc_codec_driver soc_codec_dev_pcm179x = { + .controls = pcm179x_controls, + .num_controls = ARRAY_SIZE(pcm179x_controls), + .dapm_widgets = pcm179x_dapm_widgets, + .num_dapm_widgets = ARRAY_SIZE(pcm179x_dapm_widgets), + .dapm_routes = pcm179x_dapm_routes, + .num_dapm_routes = ARRAY_SIZE(pcm179x_dapm_routes), };

-static int pcm1792a_spi_probe(struct spi_device *spi) +static int pcm179x_spi_probe(struct spi_device *spi) { - struct pcm1792a_private *pcm1792a; + struct pcm179x_private *pcm179x; int ret;

- pcm1792a = devm_kzalloc(&spi->dev, sizeof(struct pcm1792a_private), + pcm179x = devm_kzalloc(&spi->dev, sizeof(struct pcm179x_private), GFP_KERNEL); - if (!pcm1792a) + if (!pcm179x) return -ENOMEM;

- spi_set_drvdata(spi, pcm1792a); + spi_set_drvdata(spi, pcm179x);

- pcm1792a->regmap = devm_regmap_init_spi(spi, &pcm1792a_regmap); - if (IS_ERR(pcm1792a->regmap)) { - ret = PTR_ERR(pcm1792a->regmap); + pcm179x->regmap = devm_regmap_init_spi(spi, &pcm179x_regmap); + if (IS_ERR(pcm179x->regmap)) { + ret = PTR_ERR(pcm179x->regmap); dev_err(&spi->dev, "Failed to register regmap: %d\n", ret); return ret; }

return snd_soc_register_codec(&spi->dev, - &soc_codec_dev_pcm1792a, &pcm1792a_dai, 1); + &soc_codec_dev_pcm179x, &pcm179x_dai, 1); }

-static int pcm1792a_spi_remove(struct spi_device *spi) +static int pcm179x_spi_remove(struct spi_device *spi) { snd_soc_unregister_codec(&spi->dev); return 0; }

-static const struct spi_device_id pcm1792a_spi_ids[] = { - { "pcm1792a", 0 }, +static const struct spi_device_id pcm179x_spi_ids[] = { + { "pcm179x", 0 }, { }, }; -MODULE_DEVICE_TABLE(spi, pcm1792a_spi_ids); +MODULE_DEVICE_TABLE(spi, pcm179x_spi_ids);

-static struct spi_driver pcm1792a_codec_driver = { +static struct spi_driver pcm179x_codec_driver = { .driver = { - .name = "pcm1792a", - .of_match_table = of_match_ptr(pcm1792a_of_match), + .name = "pcm179x", + .of_match_table = of_match_ptr(pcm179x_of_match), }, - .id_table = pcm1792a_spi_ids, - .probe = pcm1792a_spi_probe, - .remove = pcm1792a_spi_remove, + .id_table = pcm179x_spi_ids, + .probe = pcm179x_spi_probe, + .remove = pcm179x_spi_remove, };

-module_spi_driver(pcm1792a_codec_driver); +module_spi_driver(pcm179x_codec_driver);

-MODULE_DESCRIPTION("ASoC PCM1792A driver"); +MODULE_DESCRIPTION("ASoC PCM179X driver"); MODULE_AUTHOR("Michael Trimarchi michael@amarulasolutions.com"); MODULE_LICENSE("GPL"); diff --git a/sound/soc/codecs/pcm1792a.h b/sound/soc/codecs/pcm1792a.h index 51d5470fee16..c6fdc062a497 100644 --- a/sound/soc/codecs/pcm1792a.h +++ b/sound/soc/codecs/pcm1792a.h @@ -1,5 +1,5 @@ /* - * definitions for PCM1792A + * definitions for PCM179X * * Copyright 2013 Amarula Solutions * @@ -14,8 +14,8 @@ * GNU General Public License for more details. */

-#ifndef __PCM1792A_H__ -#define __PCM1792A_H__ +#ifndef __PCM179X_H__ +#define __PCM179X_H__

#define PCM1792A_RATES (SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_8000_48000 | \ SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com --- Not get time to retest

Changes v3 -> v2: - rebase after i2c support

Changes v1 -> v2: - Use switch for support new variants - sort the compatible list

sound/soc/codecs/pcm179x-i2c.c | 6 ---- sound/soc/codecs/pcm179x-spi.c | 6 ---- sound/soc/codecs/pcm179x.c | 62 ++++++++++++++++++++++++++++++++++++++++-- sound/soc/codecs/pcm179x.h | 9 ++++-- 4 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c index 4118106..609f07f 100644 --- a/sound/soc/codecs/pcm179x-i2c.c +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -44,12 +44,6 @@ static int pcm179x_i2c_remove(struct i2c_client *client) return pcm179x_common_exit(&client->dev); }

-static const struct of_device_id pcm179x_of_match[] = { - { .compatible = "ti,pcm1792a", }, - { } -}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match); - static const struct i2c_device_id pcm179x_i2c_ids[] = { { "pcm179x", 0 }, { } diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c index da924d4..6ae0e4d 100644 --- a/sound/soc/codecs/pcm179x-spi.c +++ b/sound/soc/codecs/pcm179x-spi.c @@ -43,12 +43,6 @@ static int pcm179x_spi_remove(struct spi_device *spi) return pcm179x_common_exit(&spi->dev); }

-static const struct of_device_id pcm179x_of_match[] = { - { .compatible = "ti,pcm1792a", }, - { } -}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match); - static const struct spi_device_id pcm179x_spi_ids[] = { { "pcm179x", 0 }, { }, diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index 06a6657..73c80d3 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -20,6 +20,8 @@ #include <linux/slab.h> #include <linux/kernel.h> #include <linux/device.h> +#include <linux/of.h> +#include <linux/of_device.h>

#include <sound/core.h> #include <sound/pcm.h> @@ -72,8 +74,31 @@ struct pcm179x_private { struct regmap *regmap; unsigned int format; unsigned int rate; +#define PCM1795 1 + unsigned int codec_model; };

+static int pcm179x_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_codec *codec = dai->codec; + struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec); + u64 formats = PCM1792A_FORMATS; + + switch (priv->codec_model) { + case PCM1795: + formats = PCM1795_FORMATS; + break; + default: + break; + } + + snd_pcm_hw_constraint_mask64(substream->runtime, + SNDRV_PCM_HW_PARAM_FORMAT, formats); + + return 0; +} + static int pcm179x_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int format) { @@ -112,8 +137,10 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, switch (priv->format & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_RIGHT_J: switch (params_width(params)) { - case 24: case 32: + val = 1; + break; + case 24: val = 2; break; case 16: @@ -125,8 +152,10 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_I2S: switch (params_width(params)) { - case 24: case 32: + val = 4; + break; + case 24: val = 5; break; case 16: @@ -152,6 +181,7 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, }

static const struct snd_soc_dai_ops pcm179x_dai_ops = { + .startup = pcm179x_startup, .set_fmt = pcm179x_set_dai_fmt, .hw_params = pcm179x_hw_params, .digital_mute = pcm179x_digital_mute, @@ -190,7 +220,7 @@ static struct snd_soc_dai_driver pcm179x_dai = { .rates = SNDRV_PCM_RATE_CONTINUOUS, .rate_min = 10000, .rate_max = 200000, - .formats = PCM1792A_FORMATS, }, + .formats = PCM179X_FORMATS, }, .ops = &pcm179x_dai_ops, };

@@ -214,15 +244,41 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm179x = { .num_dapm_routes = ARRAY_SIZE(pcm179x_dapm_routes), };

+static const unsigned int codec_model = PCM1795; + +const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { .compatible = "ti,pcm1795", + .data = &codec_model, + }, + { .compatible = "ti,pcm1796", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); +EXPORT_SYMBOL_GPL(pcm179x_of_match); + int pcm179x_common_init(struct device *dev, struct regmap *regmap) { struct pcm179x_private *pcm179x; + struct device_node *np = dev->of_node; + const unsigned int *codec_model = NULL;

pcm179x = devm_kzalloc(dev, sizeof(struct pcm179x_private), GFP_KERNEL); if (!pcm179x) return -ENOMEM;

+ if (np) { + const struct of_device_id *of_id; + + of_id = of_match_device(pcm179x_of_match, dev); + if (of_id) + codec_model = of_id->data; + } + + if (codec_model) + pcm179x->codec_model = *codec_model; + pcm179x->regmap = regmap; dev_set_drvdata(dev, pcm179x);

diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index 11e3312..4c00047 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,15 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__

-#define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ - SNDRV_PCM_FMTBIT_S16_LE) +#define PCM179X_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ + SNDRV_PCM_FMTBIT_S16_LE) + +#define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE) + +#define PCM1795_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE)

extern const struct regmap_config pcm179x_regmap_config; +extern const struct of_device_id pcm179x_of_match[];

int pcm179x_common_init(struct device *dev, struct regmap *regmap); int pcm179x_common_exit(struct device *dev);

On Sat, Feb 20, 2016 at 10:27 AM, Michael Trimarchi michael@amarulasolutions.com wrote:

Hi Jacob

On Fri, Feb 5, 2016 at 6:24 PM, Michael Trimarchi michael@amarulasolutions.com wrote:

...

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com

Not get time to retest

Can you confirm that it works for you?

Hi Michael!

Sorry for not getting back to your earlier. I tried some time ago, and it made me realize that our application has been using an incorrect format setting (S32_LE) all along. Changing to the (more) correct S24_LE doesn't work (due to some reason I yet have not confirmed - but it doesn't look to be caused by this driver).

However, from the datasheet, the removal of S32_LE from PCM1792A_FORMATS seems correct. For the logical changes, the driver works for me if I manually re-add S32_LE again.

I was hoping to figure out the cause of my S24_LE issue and then be able to test your patch without modifications, but I realize that it's taking too much time.

A few comments below.

Michael

...

Changes v3 -> v2: - rebase after i2c support

Changes v1 -> v2: - Use switch for support new variants - sort the compatible list

sound/soc/codecs/pcm179x-i2c.c | 6 ---- sound/soc/codecs/pcm179x-spi.c | 6 ---- sound/soc/codecs/pcm179x.c | 62 ++++++++++++++++++++++++++++++++++++++++-- sound/soc/codecs/pcm179x.h | 9 ++++-- 4 files changed, 66 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c index 4118106..609f07f 100644 --- a/sound/soc/codecs/pcm179x-i2c.c +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -44,12 +44,6 @@ static int pcm179x_i2c_remove(struct i2c_client *client) return pcm179x_common_exit(&client->dev); }

-static const struct of_device_id pcm179x_of_match[] = {

•   { .compatible = "ti,pcm1792a", },
  

•   { }
  

-}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match);

static const struct i2c_device_id pcm179x_i2c_ids[] = { { "pcm179x", 0 }, { } diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c index da924d4..6ae0e4d 100644 --- a/sound/soc/codecs/pcm179x-spi.c +++ b/sound/soc/codecs/pcm179x-spi.c @@ -43,12 +43,6 @@ static int pcm179x_spi_remove(struct spi_device *spi) return pcm179x_common_exit(&spi->dev); }

-static const struct of_device_id pcm179x_of_match[] = {

•   { .compatible = "ti,pcm1792a", },
  

•   { }
  

-}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match);

static const struct spi_device_id pcm179x_spi_ids[] = { { "pcm179x", 0 }, { }, diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index 06a6657..73c80d3 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -20,6 +20,8 @@ #include <linux/slab.h> #include <linux/kernel.h> #include <linux/device.h> +#include <linux/of.h> +#include <linux/of_device.h>

#include <sound/core.h> #include <sound/pcm.h> @@ -72,8 +74,31 @@ struct pcm179x_private { struct regmap *regmap; unsigned int format; unsigned int rate; +#define PCM1795 1

•   unsigned int codec_model;
  

};

+static int pcm179x_startup(struct snd_pcm_substream *substream,

•                       struct snd_soc_dai *dai)
  

+{

•   struct snd_soc_codec *codec = dai->codec;
  

•   struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec);
  

•   u64 formats = PCM1792A_FORMATS;
  

How does this work? Are formats always overwritten here (and hence PCM179X_FORMATS never really used)?

...

•   switch (priv->codec_model) {
  

•   case PCM1795:
  

•           formats = PCM1795_FORMATS;
  

•           break;
  

•   default:
  

•           break;
  

•   }
  

•   snd_pcm_hw_constraint_mask64(substream->runtime,
  

•                                SNDRV_PCM_HW_PARAM_FORMAT, formats);
  

•   return 0;
  

+}

static int pcm179x_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int format) { @@ -112,8 +137,10 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, switch (priv->format & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_RIGHT_J: switch (params_width(params)) {

•           case 24:
            case 32:
  

•                   val = 1;
  

•                   break;
  

•           case 24:
                    val = 2;
                    break;
            case 16:
  

@@ -125,8 +152,10 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_I2S: switch (params_width(params)) {

•           case 24:
            case 32:
  

•                   val = 4;
  

•                   break;
  

•           case 24:
                    val = 5;
                    break;
            case 16:
  

@@ -152,6 +181,7 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, }

static const struct snd_soc_dai_ops pcm179x_dai_ops = {

•   .startup        = pcm179x_startup,
    .set_fmt        = pcm179x_set_dai_fmt,
    .hw_params      = pcm179x_hw_params,
    .digital_mute   = pcm179x_digital_mute,
  

@@ -190,7 +220,7 @@ static struct snd_soc_dai_driver pcm179x_dai = { .rates = SNDRV_PCM_RATE_CONTINUOUS, .rate_min = 10000, .rate_max = 200000,

•           .formats = PCM1792A_FORMATS, },
  

•           .formats = PCM179X_FORMATS, },
    .ops = &pcm179x_dai_ops,
  

};

@@ -214,15 +244,41 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm179x = { .num_dapm_routes = ARRAY_SIZE(pcm179x_dapm_routes), };

+static const unsigned int codec_model = PCM1795;

+const struct of_device_id pcm179x_of_match[] = {

•   { .compatible = "ti,pcm1792a", },
  

•   { .compatible = "ti,pcm1795",
  

•     .data = &codec_model,
  

•   },
  

•   { .compatible = "ti,pcm1796", },
  

•   { }
  

+}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); +EXPORT_SYMBOL_GPL(pcm179x_of_match);

int pcm179x_common_init(struct device *dev, struct regmap *regmap) { struct pcm179x_private *pcm179x;

•   struct device_node *np = dev->of_node;
  

•   const unsigned int *codec_model = NULL;
  
    pcm179x = devm_kzalloc(dev, sizeof(struct pcm179x_private),
                            GFP_KERNEL);
    if (!pcm179x)
            return -ENOMEM;
  
  

•   if (np) {
  

•           const struct of_device_id *of_id;
  

•           of_id = of_match_device(pcm179x_of_match, dev);
  

•           if (of_id)
  

•                   codec_model = of_id->data;
  

•   }
  

•   if (codec_model)
  

•           pcm179x->codec_model =  *codec_model;
  

Double space.

...

•   pcm179x->regmap = regmap;
    dev_set_drvdata(dev, pcm179x);
  
  

diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index 11e3312..4c00047 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,15 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__

-#define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \

•                     SNDRV_PCM_FMTBIT_S16_LE)
  

+#define PCM179X_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \

•                    SNDRV_PCM_FMTBIT_S16_LE)
  

+#define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE)

+#define PCM1795_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE)

extern const struct regmap_config pcm179x_regmap_config; +extern const struct of_device_id pcm179x_of_match[];

int pcm179x_common_init(struct device *dev, struct regmap *regmap); int pcm179x_common_exit(struct device *dev); -- 2.7.0

ASoC: pcm179x: Add support for pcm1795 and pcm1796

Signed-off-by: Michael Trimarchi michael@amarulasolutions.com Tested-by: Angelo Adamo adamo.a60@gmail.com --- Changes v4 -> v5 - remove dead code

Changes v3 -> v4: - clean up compatible list - use enum type instead of pointer of int

Changes v2 -> v3: - rebase after i2c support

Changes v1 -> v2: - Use switch for support new variants - sort the compatible list

sound/soc/codecs/pcm179x-i2c.c | 6 ---- sound/soc/codecs/pcm179x-spi.c | 6 ---- sound/soc/codecs/pcm179x.c | 63 ++++++++++++++++++++++++++++++++++++++++-- sound/soc/codecs/pcm179x.h | 9 ++++-- 4 files changed, 67 insertions(+), 17 deletions(-)

diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c index 4118106..609f07f 100644 --- a/sound/soc/codecs/pcm179x-i2c.c +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -44,12 +44,6 @@ static int pcm179x_i2c_remove(struct i2c_client *client) return pcm179x_common_exit(&client->dev); }

-static const struct of_device_id pcm179x_of_match[] = { - { .compatible = "ti,pcm1792a", }, - { } -}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match); - static const struct i2c_device_id pcm179x_i2c_ids[] = { { "pcm179x", 0 }, { } diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c index da924d4..6ae0e4d 100644 --- a/sound/soc/codecs/pcm179x-spi.c +++ b/sound/soc/codecs/pcm179x-spi.c @@ -43,12 +43,6 @@ static int pcm179x_spi_remove(struct spi_device *spi) return pcm179x_common_exit(&spi->dev); }

-static const struct of_device_id pcm179x_of_match[] = { - { .compatible = "ti,pcm1792a", }, - { } -}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match); - static const struct spi_device_id pcm179x_spi_ids[] = { { "pcm179x", 0 }, { }, diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index 06a6657..52b2d4b 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -20,6 +20,8 @@ #include <linux/slab.h> #include <linux/kernel.h> #include <linux/device.h> +#include <linux/of.h> +#include <linux/of_device.h>

#include <sound/core.h> #include <sound/pcm.h> @@ -68,12 +70,40 @@ static bool pcm179x_writeable_reg(struct device *dev, unsigned register reg) return accessible && reg != 0x16 && reg != 0x17; }

+enum pcm179x_type { + PCM1792A, + PCM1795, + PCM1796, +}; + struct pcm179x_private { struct regmap *regmap; unsigned int format; unsigned int rate; + enum pcm179x_type codec_model; };

+static int pcm179x_startup(struct snd_pcm_substream *substream, + struct snd_soc_dai *dai) +{ + struct snd_soc_codec *codec = dai->codec; + struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec); + u64 formats = PCM1792A_FORMATS; + + switch (priv->codec_model) { + case PCM1795: + formats = PCM1795_FORMATS; + break; + default: + break; + } + + snd_pcm_hw_constraint_mask64(substream->runtime, + SNDRV_PCM_HW_PARAM_FORMAT, formats); + + return 0; +} + static int pcm179x_set_dai_fmt(struct snd_soc_dai *codec_dai, unsigned int format) { @@ -112,8 +142,10 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, switch (priv->format & SND_SOC_DAIFMT_FORMAT_MASK) { case SND_SOC_DAIFMT_RIGHT_J: switch (params_width(params)) { - case 24: case 32: + val = 1; + break; + case 24: val = 2; break; case 16: @@ -125,8 +157,10 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, break; case SND_SOC_DAIFMT_I2S: switch (params_width(params)) { - case 24: case 32: + val = 4; + break; + case 24: val = 5; break; case 16: @@ -152,6 +186,7 @@ static int pcm179x_hw_params(struct snd_pcm_substream *substream, }

static const struct snd_soc_dai_ops pcm179x_dai_ops = { + .startup = pcm179x_startup, .set_fmt = pcm179x_set_dai_fmt, .hw_params = pcm179x_hw_params, .digital_mute = pcm179x_digital_mute, @@ -190,7 +225,7 @@ static struct snd_soc_dai_driver pcm179x_dai = { .rates = SNDRV_PCM_RATE_CONTINUOUS, .rate_min = 10000, .rate_max = 200000, - .formats = PCM1792A_FORMATS, }, + .formats = PCM179X_FORMATS, }, .ops = &pcm179x_dai_ops, };

@@ -214,15 +249,37 @@ static struct snd_soc_codec_driver soc_codec_dev_pcm179x = { .num_dapm_routes = ARRAY_SIZE(pcm179x_dapm_routes), };

+const struct of_device_id pcm179x_of_match[] = { + { .compatible = "ti,pcm1792a", }, + { .compatible = "ti,pcm1795", .data = (void *)PCM1795, }, + { .compatible = "ti,pcm1796", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm179x_of_match); +EXPORT_SYMBOL_GPL(pcm179x_of_match); + int pcm179x_common_init(struct device *dev, struct regmap *regmap) { struct pcm179x_private *pcm179x; + struct device_node *np = dev->of_node; + enum pcm179x_type codec_model = PCM1792A;

pcm179x = devm_kzalloc(dev, sizeof(struct pcm179x_private), GFP_KERNEL); if (!pcm179x) return -ENOMEM;

+ if (np) { + const struct of_device_id *of_id; + + of_id = of_match_device(pcm179x_of_match, dev); + if (of_id) + codec_model = (enum pcm179x_type) of_id->data; + } + + if (codec_model) + pcm179x->codec_model = codec_model; + pcm179x->regmap = regmap; dev_set_drvdata(dev, pcm179x);

diff --git a/sound/soc/codecs/pcm179x.h b/sound/soc/codecs/pcm179x.h index 11e3312..4c00047 100644 --- a/sound/soc/codecs/pcm179x.h +++ b/sound/soc/codecs/pcm179x.h @@ -17,10 +17,15 @@ #ifndef __PCM179X_H__ #define __PCM179X_H__

-#define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ - SNDRV_PCM_FMTBIT_S16_LE) +#define PCM179X_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE | \ + SNDRV_PCM_FMTBIT_S16_LE) + +#define PCM1792A_FORMATS (SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S16_LE) + +#define PCM1795_FORMATS (SNDRV_PCM_FMTBIT_S32_LE | SNDRV_PCM_FMTBIT_S24_LE)

extern const struct regmap_config pcm179x_regmap_config; +extern const struct of_device_id pcm179x_of_match[];

int pcm179x_common_init(struct device *dev, struct regmap *regmap); int pcm179x_common_exit(struct device *dev);

On Sat, Mar 12, 2016 at 12:29:00AM +0100, Michael Trimarchi wrote:

diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c index 4118106..609f07f 100644 --- a/sound/soc/codecs/pcm179x-i2c.c +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -44,12 +44,6 @@ static int pcm179x_i2c_remove(struct i2c_client *client) return pcm179x_common_exit(&client->dev); }

-static const struct of_device_id pcm179x_of_match[] = {

• { .compatible = "ti,pcm1792a", },
• { }

-}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match);

One effect of moving this out of the I2C driver is to break module autoloading: currently modutils does not pay attention to the reference from the driver structure to the ID table but instead just looks at the MODULE_DEVICE_TABLE. This is a problem with some existing devices but we shouldn't introduce new instances.

I don't actually know if MODULE_DEVICE_TABLE can reference an object in a different file off the top of my head, though a brief test suggests it has issues.

+static int pcm179x_startup(struct snd_pcm_substream *substream,

• 	    struct snd_soc_dai *dai)
  

+{

• struct snd_soc_codec *codec = dai->codec;
• struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec);
• u64 formats = PCM1792A_FORMATS;
• switch (priv->codec_model) {
• case PCM1795:

• formats = PCM1795_FORMATS;
  

• break;
  

It would be more data driven to just register a different DAI structure for the 1795. Both approaches work, it's just a little nicer to avoid code where possible.

On Sun, Mar 13, 2016 at 09:53:46AM +0100, Michael Trimarchi wrote:

On Sat, Mar 12, 2016 at 12:53:17PM +0700, Mark Brown wrote:

...

On Sat, Mar 12, 2016 at 12:29:00AM +0100, Michael Trimarchi wrote:

...

diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c index 4118106..609f07f 100644 --- a/sound/soc/codecs/pcm179x-i2c.c +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -44,12 +44,6 @@ static int pcm179x_i2c_remove(struct i2c_client *client) return pcm179x_common_exit(&client->dev); }

-static const struct of_device_id pcm179x_of_match[] = {

• { .compatible = "ti,pcm1792a", },
• { }

-}; -MODULE_DEVICE_TABLE(of, pcm179x_of_match);

One effect of moving this out of the I2C driver is to break module autoloading: currently modutils does not pay attention to the reference from the driver structure to the ID table but instead just looks at the MODULE_DEVICE_TABLE. This is a problem with some existing devices but we shouldn't introduce new instances.

I don't actually know if MODULE_DEVICE_TABLE can reference an object in a different file off the top of my head, though a brief test suggests it has issues.

Does this approch can work?

I need to test, as you said I don't have feeling. I found only this commit

commit 5e4cb7b60833b0124a9f71dbc5118144ca79c3c4 Author: Axel Lin axel.lin@ingics.com Date: Thu Jun 25 21:44:13 2015 +0800

ASoC: cs42xx8: Setup of_match_table

Setup of_match_table and since cs42xx8_of_match is exported and used in cs42xx8-i2c.c, it cannot be static.

Signed-off-by: Axel Lin axel.lin@ingics.com Acked-by: Brian Austin brian.austin@cirrus.com Signed-off-by: Mark Brown broonie@kernel.org

Seems a similar approch

Michael

diff --git a/sound/soc/codecs/pcm179x-i2c.c b/sound/soc/codecs/pcm179x-i2c.c index 609f07f..6bed54f 100644 --- a/sound/soc/codecs/pcm179x-i2c.c +++ b/sound/soc/codecs/pcm179x-i2c.c @@ -49,6 +49,7 @@ static const struct i2c_device_id pcm179x_i2c_ids[] = { { } }; MODULE_DEVICE_TABLE(i2c, pcm179x_i2c_ids); +MODULE_DEVICE_TABLE(of, pcm179x_of_match);

static struct i2c_driver pcm179x_i2c_driver = { .driver = { diff --git a/sound/soc/codecs/pcm179x-spi.c b/sound/soc/codecs/pcm179x-spi.c index 6ae0e4d..556c875 100644 --- a/sound/soc/codecs/pcm179x-spi.c +++ b/sound/soc/codecs/pcm179x-spi.c @@ -48,6 +48,7 @@ static const struct spi_device_id pcm179x_spi_ids[] = { { }, }; MODULE_DEVICE_TABLE(spi, pcm179x_spi_ids); +MODULE_DEVICE_TABLE(of, pcm179x_of_match);

static struct spi_driver pcm179x_spi_driver = { .driver = { diff --git a/sound/soc/codecs/pcm179x.c b/sound/soc/codecs/pcm179x.c index fb01822..a34af5b 100644 --- a/sound/soc/codecs/pcm179x.c +++ b/sound/soc/codecs/pcm179x.c @@ -255,7 +255,6 @@ const struct of_device_id pcm179x_of_match[] = { { .compatible = "ti,pcm1796", }, { } }; -MODULE_DEVICE_TABLE(of, pcm179x_of_match); EXPORT_SYMBOL_GPL(pcm179x_of_match);

int pcm179x_common_init(struct device *dev, struct regmap *regmap)

...

...

+static int pcm179x_startup(struct snd_pcm_substream *substream,

• 	    struct snd_soc_dai *dai)
  

+{

• struct snd_soc_codec *codec = dai->codec;
• struct pcm179x_private *priv = snd_soc_codec_get_drvdata(codec);
• u64 formats = PCM1792A_FORMATS;
• switch (priv->codec_model) {
• case PCM1795:

• formats = PCM1795_FORMATS;
  

• break;
  

It would be more data driven to just register a different DAI structure for the 1795. Both approaches work, it's just a little nicer to avoid code where possible.

Yes I know but my idea was exactly to not create special DAI because technically they are the same code with diffferent constraints. This even give an other example of the usage of the API.

Michael

-- | Michael Nazzareno Trimarchi Amarula Solutions BV | | COO - Founder Cruquiuskade 47 | | +31(0)851119172 Amsterdam 1018 AM NL | | [`as] http://www.amarulasolutions.com |