[alsa-devel] [PATCH 0/2] Cygnus Audio Driver
Please provide comments on the initial version of this driver.
This patchset contains the devicetree bindings and core audio driver for the Cygnus SoC.
There is an open question on how to fit this driver into the clock framework (if at all).
The audio PLL is embedded in the audio block and only used by the audio block. The audio PLL registers are also in the middle of the audio register map.
In addition, the audio PLL is adjustable to less than 1 Hz. The existing clock driver framework does not provide a mechanism to take advantage of the resolution of the hardware.
Can the audio PLL remain within the audio driver and/or any modifications required?
Lori Hikichi (2): ASoC: cygnus-audio: adding device tree bindings ASoC: add core audio driver for Broadcom Cygnus SOC.
.../bindings/sound/brcm,cygnus-audio.txt | 68 + sound/soc/bcm/Kconfig | 11 + sound/soc/bcm/Makefile | 5 +- sound/soc/bcm/cygnus-pcm.c | 918 +++++++++++ sound/soc/bcm/cygnus-pcm.h | 45 + sound/soc/bcm/cygnus-ssp.c | 1613 ++++++++++++++++++++ sound/soc/bcm/cygnus-ssp.h | 84 + 7 files changed, 2743 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt create mode 100644 sound/soc/bcm/cygnus-pcm.c create mode 100644 sound/soc/bcm/cygnus-pcm.h create mode 100644 sound/soc/bcm/cygnus-ssp.c create mode 100644 sound/soc/bcm/cygnus-ssp.h
From: Lori Hikichi lhikichi@broadcom.com
Add device tree binding documentation for the Cygnus SOC audio block
Reviewed-by: Jonathan Richardson jonathar@broadcom.com Reviewed-by: Ray Jui rjui@broadcom.com Signed-off-by: Scott Branden sbranden@broadcom.com Signed-off-by: Lori Hikichi lhikichi@broadcom.com --- .../bindings/sound/brcm,cygnus-audio.txt | 68 ++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt
diff --git a/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt b/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt new file mode 100644 index 0000000..5358cc3 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/brcm,cygnus-audio.txt @@ -0,0 +1,68 @@ +BROADCOM Cygnus Audio I2S/TDM/SPDIF controller + +Required properties: + - compatible : "brcm,cygnus-audio" + - #address-cells: 32bit valued, 1 cell. <1> + - #size-cells: 32bit valued, 1 cell. <1> + - reg : Should contain audio registers location and length + - interrupts: audio DMA interrupt number + +SSP Subnode properties: +- dai-name: The name of the DAI registered with ASOC +- ssp-port-id: The ssp port interface to use + Valid value are 0, 1, 2, or 3 (for spdif) +- mode: Controls if this port should be configured as I2S or TDM mode. + Valid values are: "tdm" or "i2s" +- tdm-bits-per-frame: only if mode is "tdm" then this property must set. + Valid values are (128/256/512) +- port-status: Controls if port is enabled or not + Valid values "enabled" or "disabled" +- channel-group: Control grouping of serial port + Valid values are "2_0", "3_1", or "5_1" + +Example: + cygnus_audio: audio@0x180ae000 { + compatible = "brcm,cygnus-audio"; + #address-cells = <1>; + #size-cells = <1>; + reg = <0x180ae000 0x1000>; + + interrupts = <GIC_SPI 143 IRQ_TYPE_LEVEL_HIGH>; + + ssp0: ssp_port@0 { + dai-name = "cygnus-ssp0"; + ssp-port-id = <0>; + + mode = "i2s"; /* "i2s" or "tdm" */ + channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */ + port-status = "enabled"; + }; + + ssp1: ssp_port@1 { + dai-name = "cygnus-ssp1"; + ssp-port-id = <1>; + + mode = "i2s"; /* "i2s" or "tdm" */ + channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */ + port-status = "disabled"; + }; + + ssp2: ssp_port@2 { + dai-name = "cygnus-ssp2"; + ssp-port-id = <2>; + + mode = "tdm"; /* "i2s" or "tdm" */ + tdm-bits-per-frame = <256>; + channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */ + port-status = "disabled"; + }; + + spdif: spdif_port@3 { + dai-name = "cygnus-spdif"; + ssp-port-id = <3>; + + mode = "i2s"; /* "i2s" or "tdm" */ + channel-group = "2_0"; /* Use 2_0, 3_1, 5_1 */ + port-status = "disabled"; + }; + };
On Mon, Mar 30, 2015 at 08:16:23PM -0700, Scott Branden wrote:
+SSP Subnode properties: +- dai-name: The name of the DAI registered with ASOC
ASoC.
Why is this in the DT - it sounds like this is just an internal implementation detail for Linux, not a property of the hardware.
+- mode: Controls if this port should be configured as I2S or TDM mode.
- Valid values are: "tdm" or "i2s"
+- tdm-bits-per-frame: only if mode is "tdm" then this property must set.
- Valid values are (128/256/512)
We'd normally leave these up to the machine driver to set as they're link wide things for system integration. The bits per frame in particular looks like something that's not going to be fixed by the hardware and could be varied at runtime.
+- port-status: Controls if port is enabled or not
- Valid values "enabled" or "disabled"
This sounds like it's replicating the DT standard status property?
+- channel-group: Control grouping of serial port
- Valid values are "2_0", "3_1", or "5_1"
What does this mean? It looks like it's setting the number of channels which again seems like a runtime thing.
On Mon, Mar 30, 2015 at 08:16:23PM -0700, Scott Branden wrote:
+Required properties:
- compatible : "brcm,cygnus-audio"
- #address-cells: 32bit valued, 1 cell. <1>
- #size-cells: 32bit valued, 1 cell. <1>
- reg : Should contain audio registers location and length
- interrupts: audio DMA interrupt number
+SSP Subnode properties:
This document does rather gloss over the fact that the binding requires subnodes, it should mention this requirement and say what they mean.
On 03/31/2015 05:16 AM, Scott Branden wrote: [...]
+- ssp-port-id: The ssp port interface to use
- Valid value are 0, 1, 2, or 3 (for spdif)
How about using 'reg' as the property name here. It is the standard property name for identifying or assigning an address to a sub-node.
On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:
The audio PLL is embedded in the audio block and only used by the audio block. The audio PLL registers are also in the middle of the audio register map.
When you say it's only used by the audio block do you mean to say that the audio block exposes no clock signals other than the bit and frame clocks?
Hi Mark,
On 15-03-30 11:43 PM, Mark Brown wrote:
On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:
The audio PLL is embedded in the audio block and only used by the audio block. The audio PLL registers are also in the middle of the audio register map.
When you say it's only used by the audio block do you mean to say that the audio block exposes no clock signals other than the bit and frame clocks?
The audio block exposes the MCLK in addition to the bit and frame clock.
Regards, Scott
On Fri, Apr 03, 2015 at 12:33:12PM -0700, Scott Branden wrote:
On 15-03-30 11:43 PM, Mark Brown wrote:
On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:
The audio PLL is embedded in the audio block and only used by the audio block. The audio PLL registers are also in the middle of the audio register map.
When you say it's only used by the audio block do you mean to say that the audio block exposes no clock signals other than the bit and frame clocks?
The audio block exposes the MCLK in addition to the bit and frame clock.
OK, then it's going to need to be a clock provider at some point - the clock will be going into external devices which are going to need to be able to interact with the clock (for example, to get the rate).
On 15-04-06 02:58 AM, Mark Brown wrote:
On Fri, Apr 03, 2015 at 12:33:12PM -0700, Scott Branden wrote:
On 15-03-30 11:43 PM, Mark Brown wrote:
On Mon, Mar 30, 2015 at 08:16:22PM -0700, Scott Branden wrote:
The audio PLL is embedded in the audio block and only used by the audio block. The audio PLL registers are also in the middle of the audio register map.
When you say it's only used by the audio block do you mean to say that the audio block exposes no clock signals other than the bit and frame clocks?
The audio block exposes the MCLK in addition to the bit and frame clock.
OK, then it's going to need to be a clock provider at some point - the clock will be going into external devices which are going to need to be able to interact with the clock (for example, to get the rate).
Currently, the ASoC machine driver is responsible for requesting a certain frequency of MCLK be generated from our driver and then also sending the frequency information along to the external device (codec). This is done via the snd_soc_dai_set_sysclk. That is the only clock interaction we have needed for the core part of the driver. For enhanced features, we also have the need to make minor adjustments (tweaks) to the PLL. The tweaks are used to make the PLLs output frequency match as closely as possible to a true reference frequency. As such, we would like to provide the finest adjustment resolution as possible. The clocking framework only seems to allow for a 1 Hz adjustment. This limitation and the fact that no other device seems to need to interact directly will the PLL are why we have not put it in the clocking framework.
On Tue, Apr 07, 2015 at 07:28:40PM -0700, Lori Hikichi wrote:
On 15-04-06 02:58 AM, Mark Brown wrote:
OK, then it's going to need to be a clock provider at some point - the clock will be going into external devices which are going to need to be able to interact with the clock (for example, to get the rate).
Currently, the ASoC machine driver is responsible for requesting a certain frequency of MCLK be generated from our driver and then also sending the frequency information along to the external device (codec). This is done via the snd_soc_dai_set_sysclk. That is the only clock interaction we have needed for the core part of the driver. For enhanced
I have some passing familiarity with ASoC... if you look at newer drivers, especially those for DT systems, you'll see that we're transitioning CODEC drivers to use the clock API for their clocks since this makes integrating with both generic ASoC things like simple card and non-ASoC clocks.
features, we also have the need to make minor adjustments (tweaks) to the PLL. The tweaks are used to make the PLLs output frequency match as closely as possible to a true reference frequency. As such, we would like to provide the finest adjustment resolution as possible. The clocking framework only seems to allow for a 1 Hz adjustment. This limitation and the fact that no other device seems to need to interact directly will the PLL are why we have not put it in the clocking framework.
That's going to be an issue no matter where you put the control - the ASoC specific clocking APIs don't have any control here either. I don't know if we want to add the functionality for doing very fine grained adjustments into the clock API or not (the use cases seem limited though I'm sure they exist), though I do think we should have that discussion if only to confirm, but that's a separate thing to how we expose any userspace control - the clock API is a kernel internal thing.
On 15-04-08 11:54 AM, Mark Brown wrote:
On Tue, Apr 07, 2015 at 07:28:40PM -0700, Lori Hikichi wrote:
On 15-04-06 02:58 AM, Mark Brown wrote:
OK, then it's going to need to be a clock provider at some point - the clock will be going into external devices which are going to need to be able to interact with the clock (for example, to get the rate).
Currently, the ASoC machine driver is responsible for requesting a certain frequency of MCLK be generated from our driver and then also sending the frequency information along to the external device (codec). This is done via the snd_soc_dai_set_sysclk. That is the only clock interaction we have needed for the core part of the driver. For enhanced
I have some passing familiarity with ASoC... if you look at newer drivers, especially those for DT systems, you'll see that we're transitioning CODEC drivers to use the clock API for their clocks since this makes integrating with both generic ASoC things like simple card and non-ASoC clocks.
features, we also have the need to make minor adjustments (tweaks) to the PLL. The tweaks are used to make the PLLs output frequency match as closely as possible to a true reference frequency. As such, we would like to provide the finest adjustment resolution as possible. The clocking framework only seems to allow for a 1 Hz adjustment. This limitation and the fact that no other device seems to need to interact directly will the PLL are why we have not put it in the clocking framework.
That's going to be an issue no matter where you put the control - the ASoC specific clocking APIs don't have any control here either. I don't know if we want to add the functionality for doing very fine grained adjustments into the clock API or not (the use cases seem limited though I'm sure they exist), though I do think we should have that discussion if only to confirm, but that's a separate thing to how we expose any userspace control - the clock API is a kernel internal thing.
Seems like there are some benefits to integrating with the clocking framework. I will have to consider what kernel APIs we want to expose as well. I believe we may have another kernel driver wanting to control this tweaking. Do you feel it is ok to have to PLL code reside in this driver for now, and then we can patch later after we get this clocking control sorted out.
participants (4)
-
Lars-Peter Clausen -
Lori Hikichi -
Mark Brown -
Scott Branden