[alsa-devel] [PATCH 0/2] ASoC: bcm2835: add support for hifiberry-dac
From: Martin Sperl kernel@martin.sperl.org
Add support for the hifiberry-dac hat that can get attached to a raspberry pi.
This hat also the ti-pcm5102a DAC, so support for this DAC has been added to codecs.
This is a backport of the drivers in the rpi-downstream kernel.
Changelog: * Drivers written by: Florian Meier florian.meier@koalo.de * ref-count patch by Matthias Reichl hias@horus.com * rebased, checkpath fixed and dt-binding documentation added by Martin Sperl kernel@martin.sperl.org
Florian Meier (2): ASoC: pcm5102a: Add support for PCM5102A codec ASoC: Add support for HifiBerry DAC
.../bindings/sound/hifiberry,hifiberry-dac.txt | 12 ++ .../devicetree/bindings/sound/pcm5102a.txt | 13 +++ sound/soc/bcm/Kconfig | 7 ++ sound/soc/bcm/Makefile | 3 +- sound/soc/bcm/hifiberry_dac.c | 126 +++++++++++++++++++++ sound/soc/codecs/Kconfig | 4 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm5102a.c | 69 +++++++++++ 8 files changed, 235 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/sound/hifiberry,hifiberry-dac.txt create mode 100644 Documentation/devicetree/bindings/sound/pcm5102a.txt create mode 100644 sound/soc/bcm/hifiberry_dac.c create mode 100644 sound/soc/codecs/pcm5102a.c
-- 2.1.4
From: Florian Meier florian.meier@koalo.de
Some definitions to support the PCM5102A codec by Texas Instruments.
Signed-off-by: Florian Meier florian.meier@koalo.de
Changes to original patch by Florian Meier: * rebased (Makefile and Kconfig * fixed checkpath errors (spaces, newlines) * added dt-binding documentation
Signed-off-by: Martin Sperl kernel@martin.sperl.org --- .../devicetree/bindings/sound/pcm5102a.txt | 13 ++++ sound/soc/codecs/Kconfig | 4 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm5102a.c | 69 ++++++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/pcm5102a.txt create mode 100644 sound/soc/codecs/pcm5102a.c
diff --git a/Documentation/devicetree/bindings/sound/pcm5102a.txt b/Documentation/devicetree/bindings/sound/pcm5102a.txt new file mode 100644 index 0000000..c63ab0b6 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/pcm5102a.txt @@ -0,0 +1,13 @@ +PCM5102a audio CODECs + +These devices does not use I2C or SPI. + +Required properties: + + - compatible : set as "ti,pcm5102a" + +Examples: + + pcm5102a: pcm5102a { + compatible = "ti,pcm5102a"; + }; diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 7ef3a0c..26ae0b5 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -94,6 +94,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_PCM3008 select SND_SOC_PCM3168A_I2C if I2C select SND_SOC_PCM3168A_SPI if SPI_MASTER + select SND_SOC_PCM5102A select SND_SOC_PCM512x_I2C if I2C select SND_SOC_PCM512x_SPI if SPI_MASTER select SND_SOC_RT286 if I2C @@ -575,6 +576,9 @@ config SND_SOC_PCM3168A_SPI select SND_SOC_PCM3168A select REGMAP_SPI
+config SND_SOC_PCM5102A + tristate + config SND_SOC_PCM512x tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 185a712..4532a74 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -89,6 +89,7 @@ snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm3168a-objs := pcm3168a.o snd-soc-pcm3168a-i2c-objs := pcm3168a-i2c.o snd-soc-pcm3168a-spi-objs := pcm3168a-spi.o +snd-soc-pcm5102a-objs := pcm5102a.o snd-soc-pcm512x-objs := pcm512x.o snd-soc-pcm512x-i2c-objs := pcm512x-i2c.o snd-soc-pcm512x-spi-objs := pcm512x-spi.o @@ -298,6 +299,7 @@ obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM3168A) += snd-soc-pcm3168a.o obj-$(CONFIG_SND_SOC_PCM3168A_I2C) += snd-soc-pcm3168a-i2c.o obj-$(CONFIG_SND_SOC_PCM3168A_SPI) += snd-soc-pcm3168a-spi.o +obj-$(CONFIG_SND_SOC_PCM5102A) += snd-soc-pcm5102a.o obj-$(CONFIG_SND_SOC_PCM512x) += snd-soc-pcm512x.o obj-$(CONFIG_SND_SOC_PCM512x_I2C) += snd-soc-pcm512x-i2c.o obj-$(CONFIG_SND_SOC_PCM512x_SPI) += snd-soc-pcm512x-spi.o diff --git a/sound/soc/codecs/pcm5102a.c b/sound/soc/codecs/pcm5102a.c new file mode 100644 index 0000000..ed51567 --- /dev/null +++ b/sound/soc/codecs/pcm5102a.c @@ -0,0 +1,69 @@ +/* + * Driver for the PCM5102A codec + * + * Author: Florian Meier florian.meier@koalo.de + * Copyright 2013 + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include <linux/init.h> +#include <linux/module.h> +#include <linux/platform_device.h> + +#include <sound/soc.h> + +static struct snd_soc_dai_driver pcm5102a_dai = { + .name = "pcm5102a-hifi", + .playback = { + .channels_min = 2, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_192000, + .formats = SNDRV_PCM_FMTBIT_S16_LE | + SNDRV_PCM_FMTBIT_S24_LE | + SNDRV_PCM_FMTBIT_S32_LE + }, +}; + +static struct snd_soc_codec_driver soc_codec_dev_pcm5102a; + +static int pcm5102a_probe(struct platform_device *pdev) +{ + return snd_soc_register_codec(&pdev->dev, &soc_codec_dev_pcm5102a, + &pcm5102a_dai, 1); +} + +static int pcm5102a_remove(struct platform_device *pdev) +{ + snd_soc_unregister_codec(&pdev->dev); + return 0; +} + +static const struct of_device_id pcm5102a_of_match[] = { + { .compatible = "ti,pcm5102a", }, + { } +}; +MODULE_DEVICE_TABLE(of, pcm5102a_of_match); + +static struct platform_driver pcm5102a_codec_driver = { + .probe = pcm5102a_probe, + .remove = pcm5102a_remove, + .driver = { + .name = "pcm5102a-codec", + .owner = THIS_MODULE, + .of_match_table = pcm5102a_of_match, + }, +}; + +module_platform_driver(pcm5102a_codec_driver); + +MODULE_DESCRIPTION("ASoC PCM5102A codec driver"); +MODULE_AUTHOR("Florian Meier florian.meier@koalo.de"); +MODULE_LICENSE("GPL v2");
On Fri, May 13, 2016 at 09:14:12AM +0000, kernel@martin.sperl.org wrote:
From: Florian Meier florian.meier@koalo.de
Some definitions to support the PCM5102A codec by Texas Instruments.
Signed-off-by: Florian Meier florian.meier@koalo.de
Changes to original patch by Florian Meier:
- rebased (Makefile and Kconfig
- fixed checkpath errors (spaces, newlines)
- added dt-binding documentation
Signed-off-by: Martin Sperl kernel@martin.sperl.org
.../devicetree/bindings/sound/pcm5102a.txt | 13 ++++ sound/soc/codecs/Kconfig | 4 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm5102a.c | 69 ++++++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/pcm5102a.txt create mode 100644 sound/soc/codecs/pcm5102a.c
diff --git a/Documentation/devicetree/bindings/sound/pcm5102a.txt b/Documentation/devicetree/bindings/sound/pcm5102a.txt new file mode 100644 index 0000000..c63ab0b6 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/pcm5102a.txt @@ -0,0 +1,13 @@ +PCM5102a audio CODECs
+These devices does not use I2C or SPI.
s/does/do/ or singular device.
What do they use? Describe the interface (or lack of), not 2 possible interfaces that it doesn't have.
+Required properties:
- compatible : set as "ti,pcm5102a"
+Examples:
- pcm5102a: pcm5102a {
compatible = "ti,pcm5102a";- };
Le 13/05/2016 à 11:14, kernel@martin.sperl.org a écrit :
From: Florian Meier florian.meier@koalo.de
Some definitions to support the PCM5102A codec by Texas Instruments.
Signed-off-by: Florian Meier florian.meier@koalo.de
Changes to original patch by Florian Meier:
- rebased (Makefile and Kconfig
- fixed checkpath errors (spaces, newlines)
- added dt-binding documentation
Signed-off-by: Martin Sperl kernel@martin.sperl.org
.../devicetree/bindings/sound/pcm5102a.txt | 13 ++++ sound/soc/codecs/Kconfig | 4 ++ sound/soc/codecs/Makefile | 2 + sound/soc/codecs/pcm5102a.c | 69 ++++++++++++++++++++++ 4 files changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/pcm5102a.txt create mode 100644 sound/soc/codecs/pcm5102a.c
diff --git a/Documentation/devicetree/bindings/sound/pcm5102a.txt b/Documentation/devicetree/bindings/sound/pcm5102a.txt new file mode 100644 index 0000000..c63ab0b6 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/pcm5102a.txt @@ -0,0 +1,13 @@ +PCM5102a audio CODECs
+These devices does not use I2C or SPI.
+Required properties:
- compatible : set as "ti,pcm5102a"
+Examples:
- pcm5102a: pcm5102a {
compatible = "ti,pcm5102a";- };
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index 7ef3a0c..26ae0b5 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -94,6 +94,7 @@ config SND_SOC_ALL_CODECS select SND_SOC_PCM3008 select SND_SOC_PCM3168A_I2C if I2C select SND_SOC_PCM3168A_SPI if SPI_MASTER
- select SND_SOC_PCM5102A select SND_SOC_PCM512x_I2C if I2C select SND_SOC_PCM512x_SPI if SPI_MASTER select SND_SOC_RT286 if I2C
@@ -575,6 +576,9 @@ config SND_SOC_PCM3168A_SPI select SND_SOC_PCM3168A select REGMAP_SPI
+config SND_SOC_PCM5102A
- tristate
config SND_SOC_PCM512x tristate
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile index 185a712..4532a74 100644 --- a/sound/soc/codecs/Makefile +++ b/sound/soc/codecs/Makefile @@ -89,6 +89,7 @@ snd-soc-pcm3008-objs := pcm3008.o snd-soc-pcm3168a-objs := pcm3168a.o snd-soc-pcm3168a-i2c-objs := pcm3168a-i2c.o snd-soc-pcm3168a-spi-objs := pcm3168a-spi.o +snd-soc-pcm5102a-objs := pcm5102a.o snd-soc-pcm512x-objs := pcm512x.o snd-soc-pcm512x-i2c-objs := pcm512x-i2c.o snd-soc-pcm512x-spi-objs := pcm512x-spi.o @@ -298,6 +299,7 @@ obj-$(CONFIG_SND_SOC_PCM3008) += snd-soc-pcm3008.o obj-$(CONFIG_SND_SOC_PCM3168A) += snd-soc-pcm3168a.o obj-$(CONFIG_SND_SOC_PCM3168A_I2C) += snd-soc-pcm3168a-i2c.o obj-$(CONFIG_SND_SOC_PCM3168A_SPI) += snd-soc-pcm3168a-spi.o +obj-$(CONFIG_SND_SOC_PCM5102A) += snd-soc-pcm5102a.o obj-$(CONFIG_SND_SOC_PCM512x) += snd-soc-pcm512x.o obj-$(CONFIG_SND_SOC_PCM512x_I2C) += snd-soc-pcm512x-i2c.o obj-$(CONFIG_SND_SOC_PCM512x_SPI) += snd-soc-pcm512x-spi.o diff --git a/sound/soc/codecs/pcm5102a.c b/sound/soc/codecs/pcm5102a.c new file mode 100644 index 0000000..ed51567 --- /dev/null +++ b/sound/soc/codecs/pcm5102a.c @@ -0,0 +1,69 @@ +/*
- Driver for the PCM5102A codec
- Author: Florian Meier florian.meier@koalo.de
Copyright 2013
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License
- version 2 as published by the Free Software Foundation.
- This program is distributed in the hope that it will be useful, but
- WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- General Public License for more details.
- */
+#include <linux/init.h> +#include <linux/module.h> +#include <linux/platform_device.h>
+#include <sound/soc.h>
+static struct snd_soc_dai_driver pcm5102a_dai = {
- .name = "pcm5102a-hifi",
- .playback = {
.channels_min = 2,.channels_max = 2,.rates = SNDRV_PCM_RATE_8000_192000,.formats = SNDRV_PCM_FMTBIT_S16_LE |SNDRV_PCM_FMTBIT_S24_LE |SNDRV_PCM_FMTBIT_S32_LE- },
+};
+static struct snd_soc_codec_driver soc_codec_dev_pcm5102a;
+static int pcm5102a_probe(struct platform_device *pdev) +{
- return snd_soc_register_codec(&pdev->dev, &soc_codec_dev_pcm5102a,
&pcm5102a_dai, 1);+}
+static int pcm5102a_remove(struct platform_device *pdev) +{
- snd_soc_unregister_codec(&pdev->dev);
- return 0;
+}
+static const struct of_device_id pcm5102a_of_match[] = {
- { .compatible = "ti,pcm5102a", },
- { }
+}; +MODULE_DEVICE_TABLE(of, pcm5102a_of_match);
+static struct platform_driver pcm5102a_codec_driver = {
- .probe = pcm5102a_probe,
- .remove = pcm5102a_remove,
- .driver = {
.name = "pcm5102a-codec",.owner = THIS_MODULE,.of_match_table = pcm5102a_of_match,- },
+};
+module_platform_driver(pcm5102a_codec_driver);
+MODULE_DESCRIPTION("ASoC PCM5102A codec driver"); +MODULE_AUTHOR("Florian Meier florian.meier@koalo.de"); +MODULE_LICENSE("GPL v2");
Hello,
There is nothing PCM5102A specific here, and it is pretty generic. Wouldn't it be better to write instead a simple-i2s-codec for all the classics I2S "hifi" DACs which will get the I2S/DAI parameters from DT ? PCM510x, PCM5122 in HW mode, ES9023, a bunch of ES90xx implementations etc... will use exactly the same code with only format and rate variation. And for the rate, it is implementation dependent, even in the case of pcm5102a.
Emmanuel.
On Sun, May 22, 2016 at 11:29:55PM +0200, Emmanuel Fusté wrote:
There is nothing PCM5102A specific here, and it is pretty generic. Wouldn't it be better to write instead a simple-i2s-codec for all the classics I2S "hifi" DACs which will get the I2S/DAI parameters from DT ? PCM510x, PCM5122 in HW mode, ES9023, a bunch of ES90xx implementations etc... will use exactly the same code with only format and rate variation. And for the rate, it is implementation dependent, even in the case of pcm5102a.
If we do that then we have no idea what the hardware actually is and we're creating more effort on the DT side, the DT has to specify all the parameters for the device rather than just the name. Given how trivial the code is it's not clear that this is a win.
Le 23/05/2016 à 19:08, Mark Brown a écrit :
On Sun, May 22, 2016 at 11:29:55PM +0200, Emmanuel Fusté wrote:
There is nothing PCM5102A specific here, and it is pretty generic. Wouldn't it be better to write instead a simple-i2s-codec for all the classics I2S "hifi" DACs which will get the I2S/DAI parameters from DT ? PCM510x, PCM5122 in HW mode, ES9023, a bunch of ES90xx implementations etc... will use exactly the same code with only format and rate variation. And for the rate, it is implementation dependent, even in the case of pcm5102a.
If we do that then we have no idea what the hardware actually is and we're creating more effort on the DT side, the DT has to specify all the parameters for the device rather than just the name. Given how trivial the code is it's not clear that this is a win.
All the parameters are only rate and format. And even rate is implementation dependent. Knowing the name of the hardware give nothing. You still have to look at the schematic or look at the board to know valid rates. Actually is anything but a PCM5102A codec driver. It is a driver for an I2S device accepting PCM rate from 8khz to 192khz with bck rate of 48fs or 64fs (32fs is normally supported too by the hw mode used in hifiberry).
Yes this push more effort on the DT side, but I think this is better to describe in DT the actual "I2S only" hardware capabilities of the implementation of the device guided by wiring or design choice. Doing code cut&paste and two static assignment adjustment to create a "new" driver is completely overkill and nonflexible.
instead of the proposed dt, we could have :
/* pcm5102a in HW mode with external SCK (4wire mode) * at 128fs, 192fs or 256fs */ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = < SNDRV_PCM_FMTBIT_S16_LE |SNDRV_PCM_FMTBIT_S24_LE |SNDRV_PCM_FMTBIT_S32_LE>; }; or /* pcm5102a in HW mode with PLL on BCK (3wire mode) * 16to384khz but only with bck rate of 64fs */ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000 ^ SNDRV_PCM_RATE_8000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; }; Or /* ES9023 in asynchronous mode mclk > 37Mhz * I did not try bck rate <64fs as the sabre family did not generally * support it. */ es9023: es9023 { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; };
We could use I2S friendly property instead of "format" : bckrates = <48, 64>;
With two or tree more property (gpio for mute, de-emphasis and filter selection), you could cover 98% of I2S DAC with hardware only interfaces. It cover too startup mode of most hifi I2C or SPI controllable DAC. Which is a good starting point for chip for which a driver is not already written.
Emmanuel.
On Mon, May 23, 2016 at 10:37:18PM +0200, Emmanuel Fusté wrote:
All the parameters are only rate and format.
Off the top of my head clocking constraints are another thing we could know...
And even rate is implementation dependent. Knowing the name of the hardware give nothing. You still have to look at the schematic or look at the board to know valid rates.
Fortunately we have mechanisms for constraining things between multiple devices, it's not like this is a property unique to registerless CODECs.
Yes this push more effort on the DT side, but I think this is better to describe in DT the actual "I2S only" hardware capabilities of the implementation of the device guided by wiring or design choice. Doing code cut&paste and two static assignment adjustment to create a "new" driver is completely overkill and nonflexible.
If someone was so motivated they could also make one big driver with a table of per device constraints in it, it doesn't *have* to be separate files.
/* pcm5102a in HW mode with external SCK (4wire mode)
- at 128fs, 192fs or 256fs
*/ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = < SNDRV_PCM_FMTBIT_S16_LE |SNDRV_PCM_FMTBIT_S24_LE |SNDRV_PCM_FMTBIT_S32_LE>; };
Those defines look awfully Linux specific...
/* pcm5102a in HW mode with PLL on BCK (3wire mode)
- 16to384khz but only with bck rate of 64fs
*/ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000 ^ SNDRV_PCM_RATE_8000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; };
...and another way of doing the multiple options would be to have the binding be per device and use the clock binding to describe where the master clock comes from which then allows us to start and stop that clock on demand and possibly (as our clock API integration improves) constrain based on that. It may be that in future or perhaps even just for other users of the same board the set of constraints is different (perhaps they choose some other clock rate for something in the system due to other constraints which lets the master clock vary more for example).
It's also just more work for the second person to come across the device, at best they have to go and type in the same data over again.
A brief look at the datasheet suggests that there's also a few GPIO controls we might have wired up where we can expose them and of course regulators we may want to control in some systems.
On Mon, 23 May 2016, Emmanuel Fusté wrote:
/* pcm5102a in HW mode with external SCK (4wire mode)
- at 128fs, 192fs or 256fs
*/ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = < SNDRV_PCM_FMTBIT_S16_LE |SNDRV_PCM_FMTBIT_S24_LE |SNDRV_PCM_FMTBIT_S32_LE>; }; or /* pcm5102a in HW mode with PLL on BCK (3wire mode)
- 16to384khz but only with bck rate of 64fs
*/ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000 ^ SNDRV_PCM_RATE_8000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; }; Or /* ES9023 in asynchronous mode mclk > 37Mhz
- I did not try bck rate <64fs as the sabre family did not generally
- support it.
*/ es9023: es9023 { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; };
We could use I2S friendly property instead of "format" : bckrates = <48, 64>;
With two or tree more property (gpio for mute, de-emphasis and filter selection), you could cover 98% of I2S DAC with hardware only interfaces. It cover too startup mode of most hifi I2C or SPI controllable DAC. Which is a good starting point for chip for which a driver is not already written.
It seems to me that this only covers the cases where the is codec running in slave mode with MCLK derived from the BCLK, but not cases where the codec is master and needs to have its MCLK configured appropriately (in terms of device-specifc clock source and PLL settings), or am I missing something?
/Ricard
Le 24/05/2016 à 16:44, Ricard Wanderlof a écrit :
On Mon, 23 May 2016, Emmanuel Fusté wrote:
/* pcm5102a in HW mode with external SCK (4wire mode)
- at 128fs, 192fs or 256fs
*/ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = < SNDRV_PCM_FMTBIT_S16_LE |SNDRV_PCM_FMTBIT_S24_LE |SNDRV_PCM_FMTBIT_S32_LE>; }; or /* pcm5102a in HW mode with PLL on BCK (3wire mode)
- 16to384khz but only with bck rate of 64fs
*/ pcm5102a-4w: pcm5102a { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000 ^ SNDRV_PCM_RATE_8000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; }; Or /* ES9023 in asynchronous mode mclk > 37Mhz
- I did not try bck rate <64fs as the sabre family did not generally
- support it.
*/ es9023: es9023 { compatible = "simple-i2s-codec"; rate = <SNDRV_PCM_RATE_8000_192000>; format = <SNDRV_PCM_FMTBIT_S32_LE>; };
We could use I2S friendly property instead of "format" : bckrates = <48, 64>;
With two or tree more property (gpio for mute, de-emphasis and filter selection), you could cover 98% of I2S DAC with hardware only interfaces. It cover too startup mode of most hifi I2C or SPI controllable DAC. Which is a good starting point for chip for which a driver is not already written.
It seems to me that this only covers the cases where the is codec running in slave mode with MCLK derived from the BCLK, but not cases where the codec is master and needs to have its MCLK configured appropriately (in terms of device-specifc clock source and PLL settings), or am I missing something?
/Ricard
It not a codec but a simple I2S DAC. No I2S master mode. Like most DAC before the CODEC era and all the big mess around them. In the ES9023 case, the MCLK could either have no frequency an no phase relationship with the I2S clocks. In this case, the MCLK is surely derived from a clock of the Rpi SOC so you are right, and the whole HifiBerry driver should be rewritten with proper abstraction like Mark suggested : Clock binding etc ... But now as Mark say too, clock api integration is a goal, but not there. The proposed hw param rules is a start to not have to write specific machine driver because of clocking constraints. It is the second time I am scratching my head because of clocking constraints and because I want to do something better than writing another trivial machine driver. In my case (BBB or other Ti soc using mcasp) I will have to had mux/clk_set_parent switching action and/or clk_set_rate action. But I am not persuaded that is it the way to go.
That why, by the time we get proper clock api integration with proper constraint and constraint description mechanism, a simple DT configurable codec driver in which we push the resulting constraint (DAC+SOC+design choice) was appealing.
But you are all right. This is not the long term proper way. And the actual ultra simple PCM510xA driver could evolve to support all GPIO and later properly express all his clock constraints independently of the implementation choices.
Thank your for your patience.
Emmanuel.
From: Florian Meier florian.meier@koalo.de
This adds a machine driver for the HifiBerry DAC. It is a sound card that can be stacked onto the Raspberry Pi.
Signed-off-by: Florian Meier florian.meier@koalo.de
Changes to original patch by Florian Meier: * added .owner to struct snd_rpi_hifiberry_dac (ref-count)
Signed-off-by: Matthias Reichl hias@horus.com
Changes to original patch by Florian Meier: * change to use BCM2835 in config keys instead of bcm2708 * fixed checkpath errors (spaces, indentation) * added dt-binding documentation
Signed-off-by: Martin Sperl kernel@martin.sperl.org --- .../bindings/sound/hifiberry,hifiberry-dac.txt | 12 ++ sound/soc/bcm/Kconfig | 7 ++ sound/soc/bcm/Makefile | 3 +- sound/soc/bcm/hifiberry_dac.c | 126 +++++++++++++++++++++ 4 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/sound/hifiberry,hifiberry-dac.txt create mode 100644 sound/soc/bcm/hifiberry_dac.c
diff --git a/Documentation/devicetree/bindings/sound/hifiberry,hifiberry-dac.txt b/Documentation/devicetree/bindings/sound/hifiberry,hifiberry-dac.txt new file mode 100644 index 0000000..88ba248 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/hifiberry,hifiberry-dac.txt @@ -0,0 +1,12 @@ +* Hifiberry-dac soundcard-hat for raspberry-pi + +Required properties: +- compatible: "hifiberry,hifiberry-dac" +- i2s-controller: phandle of the bcm2835 i2s controller + +Example: + +sound { + compatible = "hifiberry,hifiberry-dac"; + i2s-controller = <&i2s>; +}; diff --git a/sound/soc/bcm/Kconfig b/sound/soc/bcm/Kconfig index 6a834e1..e912774 100644 --- a/sound/soc/bcm/Kconfig +++ b/sound/soc/bcm/Kconfig @@ -7,3 +7,10 @@ config SND_BCM2835_SOC_I2S Say Y or M if you want to add support for codecs attached to the BCM2835 I2S interface. You will also need to select the audio interfaces to support below. + +config SND_BCM2835_SOC_HIFIBERRY_DAC + tristate "Support for HifiBerry DAC" + depends on SND_BCM2835_SOC_I2S + select SND_SOC_PCM5102A + help + Say Y or M if you want to add support for HifiBerry DAC. diff --git a/sound/soc/bcm/Makefile b/sound/soc/bcm/Makefile index bc816b7..bc6e249 100644 --- a/sound/soc/bcm/Makefile +++ b/sound/soc/bcm/Makefile @@ -1,5 +1,6 @@ # BCM2835 Platform Support snd-soc-bcm2835-i2s-objs := bcm2835-i2s.o +snd-soc-hifiberry-dac-objs := hifiberry_dac.o
obj-$(CONFIG_SND_BCM2835_SOC_I2S) += snd-soc-bcm2835-i2s.o - +obj-$(CONFIG_SND_BCM2835_SOC_HIFIBERRY_DAC) += snd-soc-hifiberry-dac.o diff --git a/sound/soc/bcm/hifiberry_dac.c b/sound/soc/bcm/hifiberry_dac.c new file mode 100644 index 0000000..9e79bbf --- /dev/null +++ b/sound/soc/bcm/hifiberry_dac.c @@ -0,0 +1,126 @@ +/* + * ASoC Driver for HifiBerry DAC + * + * Author: Florian Meier florian.meier@koalo.de + * Copyright 2013 + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#include <linux/module.h> +#include <linux/platform_device.h> + +#include <sound/core.h> +#include <sound/pcm.h> +#include <sound/pcm_params.h> +#include <sound/soc.h> +#include <sound/jack.h> + +static int snd_rpi_hifiberry_dac_init(struct snd_soc_pcm_runtime *rtd) +{ + return 0; +} + +static int snd_rpi_hifiberry_dac_hw_params( + struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_soc_pcm_runtime *rtd = substream->private_data; + struct snd_soc_dai *cpu_dai = rtd->cpu_dai; + + unsigned int sample_bits = + snd_pcm_format_physical_width(params_format(params)); + + return snd_soc_dai_set_bclk_ratio(cpu_dai, sample_bits * 2); +} + +/* machine stream operations */ +static struct snd_soc_ops snd_rpi_hifiberry_dac_ops = { + .hw_params = snd_rpi_hifiberry_dac_hw_params, +}; + +static struct snd_soc_dai_link snd_rpi_hifiberry_dac_dai[] = { +{ + .name = "HifiBerry DAC", + .stream_name = "HifiBerry DAC HiFi", + .cpu_dai_name = "bcm2708-i2s.0", + .codec_dai_name = "pcm5102a-hifi", + .platform_name = "bcm2708-i2s.0", + .codec_name = "pcm5102a-codec", + .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF | + SND_SOC_DAIFMT_CBS_CFS, + .ops = &snd_rpi_hifiberry_dac_ops, + .init = snd_rpi_hifiberry_dac_init, +}, +}; + +/* audio machine driver */ +static struct snd_soc_card snd_rpi_hifiberry_dac = { + .name = "snd_rpi_hifiberry_dac", + .owner = THIS_MODULE, + .dai_link = snd_rpi_hifiberry_dac_dai, + .num_links = ARRAY_SIZE(snd_rpi_hifiberry_dac_dai), +}; + +static int snd_rpi_hifiberry_dac_probe(struct platform_device *pdev) +{ + struct device_node *i2s_node; + struct snd_soc_dai_link *dai; + int ret = 0; + + snd_rpi_hifiberry_dac.dev = &pdev->dev; + + if (pdev->dev.of_node) { + dai = &snd_rpi_hifiberry_dac_dai[0]; + i2s_node = of_parse_phandle(pdev->dev.of_node, + "i2s-controller", 0); + + if (i2s_node) { + dai->cpu_dai_name = NULL; + dai->cpu_of_node = i2s_node; + dai->platform_name = NULL; + dai->platform_of_node = i2s_node; + } + } + + ret = snd_soc_register_card(&snd_rpi_hifiberry_dac); + if (ret) + dev_err(&pdev->dev, + "snd_soc_register_card() failed: %d\n", ret); + + return ret; +} + +static int snd_rpi_hifiberry_dac_remove(struct platform_device *pdev) +{ + return snd_soc_unregister_card(&snd_rpi_hifiberry_dac); +} + +static const struct of_device_id snd_rpi_hifiberry_dac_of_match[] = { + { .compatible = "hifiberry,hifiberry-dac", }, + {}, +}; +MODULE_DEVICE_TABLE(of, snd_rpi_hifiberry_dac_of_match); + +static struct platform_driver snd_rpi_hifiberry_dac_driver = { + .driver = { + .name = "snd-hifiberry-dac", + .owner = THIS_MODULE, + .of_match_table = snd_rpi_hifiberry_dac_of_match, + }, + .probe = snd_rpi_hifiberry_dac_probe, + .remove = snd_rpi_hifiberry_dac_remove, +}; + +module_platform_driver(snd_rpi_hifiberry_dac_driver); + +MODULE_AUTHOR("Florian Meier florian.meier@koalo.de"); +MODULE_DESCRIPTION("ASoC Driver for HifiBerry DAC"); +MODULE_LICENSE("GPL v2"); -- 2.1.4
On Fri, May 13, 2016 at 09:14:13AM +0000, kernel@martin.sperl.org wrote:
+static int snd_rpi_hifiberry_dac_init(struct snd_soc_pcm_runtime *rtd) +{
- return 0;
+}
Remove empty functions. Either they are redundant or you really need to do something in them.
+static struct snd_soc_dai_link snd_rpi_hifiberry_dac_dai[] = { +{
- .name = "HifiBerry DAC",
- .stream_name = "HifiBerry DAC HiFi",
- .cpu_dai_name = "bcm2708-i2s.0",
- .codec_dai_name = "pcm5102a-hifi",
- .platform_name = "bcm2708-i2s.0",
- .codec_name = "pcm5102a-codec",
I would expect all this to be done with DT references in a DT driver rather than hard coding static names - look at how simple-card does this. You could almost use simple-card here but the BCLK ratio requirement is probably a bit much, I'm not immediately seeing a nice way to specify the ratio there since it depends on the sample width.
On 13.05.2016, at 12:54, Mark Brown broonie@kernel.org wrote:
On Fri, May 13, 2016 at 09:14:13AM +0000, kernel@martin.sperl.org wrote:
+static int snd_rpi_hifiberry_dac_init(struct snd_soc_pcm_runtime *rtd) +{
- return 0;
+}
Remove empty functions. Either they are redundant or you really need to do something in them.
Sure this can get removed.
As said - it is just pushing the patches upstream, so I did not even touch it.
+static struct snd_soc_dai_link snd_rpi_hifiberry_dac_dai[] = { +{
- .name = "HifiBerry DAC",
- .stream_name = "HifiBerry DAC HiFi",
- .cpu_dai_name = "bcm2708-i2s.0",
- .codec_dai_name = "pcm5102a-hifi",
- .platform_name = "bcm2708-i2s.0",
- .codec_name = "pcm5102a-codec",
I would expect all this to be done with DT references in a DT driver rather than hard coding static names - look at how simple-card does this. You could almost use simple-card here but the BCLK ratio requirement is probably a bit much, I'm not immediately seeing a nice way to specify the ratio there since it depends on the sample width.
Actually it is just: <sample-width> * <number of channels> where number of channels is fixed to 2 for bcm2835-i2s.
So maybe that already happens automatically in the framework?
I will need to test it...
I also know that some of the audio guys working on the rpi would like to be able to configure BCLK ratios based on both: sample frequency and sample_width.
Basically they want to control the clock divider/clock parent trying to avoid fractional dividers.
Something like this could get added to the core? Is there interest in getting something like this?
Could look like this: bclk-ratios = /* for 96kHz at 16bit/channel use bclk-ratio 40 */ <96000 16 40>, /* for 48kHz at 32bit/channel use bclk-ratio 80 */ <48000 16 80>, /* for any (other) sample frequency at 16 bit use bclk-ratio 32 */ <0 16 32>, /* for any (other) sample frequency at 32 bit use bclk-ratio 64 */ <0 32 64>;
but there could also be other approaches as well, how this could get described in the device tree.
On Fri, May 13, 2016 at 02:20:57PM +0200, Martin Sperl wrote:
On 13.05.2016, at 12:54, Mark Brown broonie@kernel.org wrote:
You could almost use simple-card here but the BCLK ratio requirement is probably a bit much, I'm not immediately seeing a nice way to specify the ratio there since it depends on the sample width.
Actually it is just: <sample-width> * <number of channels> where number of channels is fixed to 2 for bcm2835-i2s.
So maybe that already happens automatically in the framework?
No, it's not handled. Lots of devices don't care or have different requirements (eg, 256fs). The tricky bit with specifying it is that the ratio depends on the sample width here but it might depend on something else with another device, or the number of channels might vary. That should be doable with DT but it feels like more trouble than it's reasonable to ask you to take here.
Something like this could get added to the core? Is there interest in getting something like this?
Could look like this: bclk-ratios = /* for 96kHz at 16bit/channel use bclk-ratio 40 */ <96000 16 40>, /* for 48kHz at 32bit/channel use bclk-ratio 80 */ <48000 16 80>, /* for any (other) sample frequency at 16 bit use bclk-ratio 32 */ <0 16 32>, /* for any (other) sample frequency at 32 bit use bclk-ratio 64 */ <0 32 64>;
but there could also be other approaches as well, how this could get described in the device tree.
You may well end up with something like that, yeah.
On 13.05.2016, at 15:21, Mark Brown broonie@kernel.org wrote:
On Fri, May 13, 2016 at 02:20:57PM +0200, Martin Sperl wrote:
On 13.05.2016, at 12:54, Mark Brown broonie@kernel.org wrote:
You could almost use simple-card here but the BCLK ratio requirement is probably a bit much, I'm not immediately seeing a nice way to specify the ratio there since it depends on the sample width.
Actually it is just: <sample-width> * <number of channels> where number of channels is fixed to 2 for bcm2835-i2s.
So maybe that already happens automatically in the framework?
No, it's not handled. Lots of devices don't care or have different requirements (eg, 256fs). The tricky bit with specifying it is that the ratio depends on the sample width here but it might depend on something else with another device, or the number of channels might vary. That should be doable with DT but it feels like more trouble than it's reasonable to ask you to take here.
Something like this could get added to the core? Is there interest in getting something like this?
Could look like this: bclk-ratios = /* for 96kHz at 16bit/channel use bclk-ratio 40 */ <96000 16 40>, /* for 48kHz at 32bit/channel use bclk-ratio 80 */ <48000 16 80>, /* for any (other) sample frequency at 16 bit use bclk-ratio 32 */ <0 16 32>, /* for any (other) sample frequency at 32 bit use bclk-ratio 64 */ <0 32 64>;
but there could also be other approaches as well, how this could get described in the device tree.
You may well end up with something like that, yeah.
would something like this be a possibility:
sound { compatible = "simple-audio-card"; simple-audio-card,name = "HifiBerry DAC"; ... /* for 32bit@48k set bclk size to 40bit/channel = 80bits effectively */ hw-params-filter@0 { match-rate = <48000>; match-sample-bits = <32>; match-channels = <2>; action = "set-fixed-bclk-ratio"; action-value = <40>; };
/* for 16bit@96k set bclk size to 20bit/channel = 40bits effectively */ hw-params-filter@1 { match-rate = <96000>; match-sample-bits = <16>; match-channels = <2>; action = "set-fixed-bclk-ratio"; action-value = <40>; }; /* for all others take sample_bits * channel */ hw-params-filter@999 { action = "set-bclk-ratio-multiple-sample-bits-channels"; }; };
The title of nodes and properties can easily get changed. Additional "match-*” properties can also get added easily.
The biggest question is about "action" or maybe “actions” (in the sense of an array, so that there can be multiple actions)? Then if this should remain a string or should it be an id (that we would need to define in an include)…
More actions could get added easily - there could even be a framework that would allow drivers to specify more actions inside different drivers that are assigned here.
I would think that the “first” one matches - order would be enforced based on node name.
This would just get added to asoc_simple_card_hw_params.
This does not seem too complicated to create.
Thanks, Martin
On 13.05.2016, at 15:21, Mark Brown broonie@kernel.org wrote:
On Fri, May 13, 2016 at 02:20:57PM +0200, Martin Sperl wrote:
Could look like this: bclk-ratios = /* for 96kHz at 16bit/channel use bclk-ratio 40 */ <96000 16 40>, /* for 48kHz at 32bit/channel use bclk-ratio 80 */ <48000 16 80>, /* for any (other) sample frequency at 16 bit use bclk-ratio 32 */ <0 16 32>, /* for any (other) sample frequency at 32 bit use bclk-ratio 64 */ <0 32 64>;
but there could also be other approaches as well, how this could get described in the device tree.
You may well end up with something like that, yeah.
I am submitting a patch for this now...
participants (6)
-
Emmanuel Fusté -
kernel＠martin.sperl.org -
Mark Brown -
Martin Sperl
-
Ricard Wanderlof -
Rob Herring