| Message ID | 20220907142124.2532620-1-luca.ceresoli@bootlin.com |
|---|---|
| Headers | show |
| Series | Add support for the internal RK3308 audio codec | expand |
On 07/09/2022 16:21, luca.ceresoli@bootlin.com wrote: > From: Luca Ceresoli <luca.ceresoli@bootlin.com> > > Add device tree bindings document for the internal audio codec of the > Rockchip RK3308 SoC. > > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com> Use subject prefixes matching the subsystem (git log --oneline -- ...). > --- > .../bindings/sound/rockchip,rk3308-codec.yaml | 102 ++++++++++++++++++ > MAINTAINERS | 6 ++ > .../dt-bindings/sound/rockchip,rk3308-codec.h | 15 +++ > 3 files changed, 123 insertions(+) > create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml > create mode 100644 include/dt-bindings/sound/rockchip,rk3308-codec.h > > diff --git a/Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml b/Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml > new file mode 100644 > index 000000000000..f3458f86ef06 > --- /dev/null > +++ b/Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml > @@ -0,0 +1,102 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/sound/rockchip,rk3308-codec.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Rockchip RK3308 Internal Codec > + > +description: | > + This is the audio codec embedded in the Rockchip RK3308 > + SoC. It has 8 24-bit ADCs and 2 24-bit DACs. The maximum supported > + sampling rate is 192 kHz. > + > + It is connected internally to one out of a selection of the internal I2S > + controllers. > + > + The RK3308 audio codec has 8 independent capture channels, but some > + features work on stereo pairs called groups: > + * grp 0 -- MIC1 / MIC2 > + * grp 1 -- MIC3 / MIC4 > + * grp 2 -- MIC5 / MIC6 > + * grp 3 -- MIC7 / MIC8 > + > +maintainers: > + - Luca Ceresoli <luca.ceresoli@bootlin.com> > + > +properties: > + compatible: > + const: rockchip,rk3308-codec > + > + reg: > + maxItems: 1 > + > + rockchip,grf: > + $ref: /schemas/types.yaml#/definitions/phandle > + description: > + Phandle to the General Register Files (GRF) > + > + clocks: > + items: > + - description: clock for TX > + - description: clock for RX > + - description: AHB clock driving the interface > + > + clock-names: > + items: > + - const: mclk_tx > + - const: mclk_rx > + - const: hclk > + > + resets: true maxItems: 1 > + > + reset-names: > + items: > + - const: "acodec" No quotes. > + > + "#sound-dai-cells": > + const: 0 > + > + rockchip,micbias-avdd-multiplier: > + description: | > + Voltage setting for the MICBIAS pins expressed as a multiplier of > + AVDD. > + > + E.g. if rockchip,micbias-avdd-multiplier = 7 (x0.85) and AVDD = 3v3, > + then MIC BIAS voltage will be 3.3 V * 0.85 = 2.805 V. > + > + Value 0: multiplier = 0.50 > + Value N: multiplier = 0.50 + 0.05 * N > + Value 7: multiplier = 0.85 Use logical values/units. The units is 0.05, so "-percent" in node name. https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/property-units.yaml Then drop ref and use enum. > + > + $ref: /schemas/types.yaml#/definitions/uint32 > + maximum: 7 > + > +required: > + - compatible > + - reg > + - rockchip,grf > + - clocks > + - resets > + - "#sound-dai-cells" > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/clock/rk3308-cru.h> > + > + acodec: acodec@ff560000 { codec Node names should be generic. https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + compatible = "rockchip,rk3308-codec"; > + reg = <0xff560000 0x10000>; > + rockchip,grf = <&grf>; > + clock-names = "mclk_tx", "mclk_rx", "hclk"; > + clocks = <&cru SCLK_I2S2_8CH_TX_OUT>, > + <&cru SCLK_I2S2_8CH_RX_OUT>, > + <&cru PCLK_ACODEC>; > + reset-names = "acodec"; > + resets = <&cru SRST_ACODEC_P>; > + #sound-dai-cells = <0>; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 895e8ace80dd..d53a8e74cb1e 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -17588,6 +17588,12 @@ S: Maintained > F: Documentation/devicetree/bindings/media/rockchip-rga.yaml > F: drivers/media/platform/rockchip/rga/ > > +ROCKCHIP RK3308 INTERNAL AUDIO CODEC > +M: Luca Ceresoli <luca.ceresoli@bootlin.com> > +S: Maintained > +F: Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml > +F: include/dt-bindings/sound/rockchip,rk3308-codec.h > + > ROCKCHIP VIDEO DECODER DRIVER > M: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> > L: linux-media@vger.kernel.org > diff --git a/include/dt-bindings/sound/rockchip,rk3308-codec.h b/include/dt-bindings/sound/rockchip,rk3308-codec.h > new file mode 100644 > index 000000000000..9f1b210a048e > --- /dev/null > +++ b/include/dt-bindings/sound/rockchip,rk3308-codec.h > @@ -0,0 +1,15 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ Dual license. > +#ifndef __DT_BINDINGS_ROCKCHIP_RK3308_CODEC_H__ > +#define __DT_BINDINGS_ROCKCHIP_RK3308_CODEC_H__ > + > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_50 0 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_55 1 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_60 2 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_65 3 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_70 4 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_75 5 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_80 6 > +#define RK3308_CODEC_MICBIAS_AVDD_x_0_85 7 You store register values in the bindings. Nope. Bindings are not for this. Best regards, Krzysztof
On Wed, Sep 07, 2022 at 04:21:21PM +0200, luca.ceresoli@bootlin.com wrote: > -static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int stream, > +static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int clk_id, > unsigned int freq, int dir) > { > struct rk_i2s_tdm_dev *i2s_tdm = to_info(cpu_dai); > @@ -978,15 +981,18 @@ static int rockchip_i2s_tdm_set_sysclk(struct snd_soc_dai *cpu_dai, int stream, > if (i2s_tdm->clk_trcm) { > i2s_tdm->mclk_tx_freq = freq; > i2s_tdm->mclk_rx_freq = freq; > + > + dev_dbg(i2s_tdm->dev, "mclk freq: %u", freq); > } else { > - if (stream == SNDRV_PCM_STREAM_PLAYBACK) > + if (clk_id == CLK_IDX_MCLK_TX) > i2s_tdm->mclk_tx_freq = freq; > - else > + else if (clk_id == CLK_IDX_MCLK_RX) > i2s_tdm->mclk_rx_freq = freq; > - } > + else > + return -ENOTSUPP; This should be a switch statement for clarity and exensibility.
Hello Mark,
On Thu, 8 Sep 2022 17:27:36 +0100
Mark Brown <broonie@kernel.org> wrote:
> On Wed, Sep 07, 2022 at 04:21:23PM +0200, luca.ceresoli@bootlin.com wrote:
Thank you for taking the time to review my patch in such detail! This
is my first contribution to ALSA, and it was not clear to me which
parts of the existing vendor driver needed even more cleanups than I
have already done. I will probably get back to you with specific
questions later on, while addressing your comments.
Best regards,
Luca
From: Luca Ceresoli <luca.ceresoli@bootlin.com> This series of patches adds support to use the internal audio codec of the Rockchip RK3308 SoC. This codec is internally connected to the I2S peripherals on the same chip, and it has some peculiarities arising from that interconnection. For proper bidirectional operation with the internal codec, the I2S peripheral needs two clock sources (tx and rx), while connection with an external codec commonly needs only one. Since v5.16 there is a driver for the I2S in sound/soc/rockchip/rockchip_i2s_tdm.c, but it does not correctly handle receiving those two clocks via the .set_sysclk op. Patch 5 fixes that. However the simple-audio-card and audio-graph-card sound card drivers do not support calling .set_sysclk twice, thus patch 6 makes the .init op of struct asoc_simple_priv overridable by a driver in order to be able to call .set_sysclk twice and thus configure both clocks. Patch 7 adds the codec driver and patch 8 builds on top of all the above by implementing a simple RK3308-specific audio card, based on audio-graph-card. This card sets all the I2S input clocks. Patches 1-2 add DT bindings for the codec and the card. Patches 3-4 add to the SoC DT file two I2S controllers (those which are internally connected to the internal codec) and the codec itself. Luca Luca Ceresoli (8): ASoC: rockchip: rk3308: add internal audio codec bindings ASoC: rockchip: rk3308: add audio card bindings arm64: dts: rockchip: add i2s_8ch_2 and i2s_8ch_3 arm64: dts: rockchip: add the internal audio codec ASoC: rockchip: i2s-tdm: Fix clk_id usage in .set_sysclk() ASoC: audio-graph: let dai_link->init be overridable ASoC: codecs: Add RK3308 internal audio codec driver ASoC: rockchip: add new RK3308 sound card .../rockchip,rk3308-audio-graph-card.yaml | 50 + .../bindings/sound/rockchip,rk3308-codec.yaml | 102 + MAINTAINERS | 14 + arch/arm64/boot/dts/rockchip/rk3308.dtsi | 68 + .../dt-bindings/sound/rockchip,rk3308-codec.h | 15 + include/sound/simple_card_utils.h | 1 + sound/soc/codecs/Kconfig | 11 + sound/soc/codecs/Makefile | 2 + sound/soc/codecs/rk3308_codec.c | 2122 +++++++++++++++++ sound/soc/codecs/rk3308_codec.h | 648 +++++ sound/soc/generic/audio-graph-card.c | 2 + sound/soc/rockchip/Kconfig | 14 + sound/soc/rockchip/Makefile | 1 + sound/soc/rockchip/rockchip_i2s_tdm.c | 18 +- sound/soc/rockchip/rockchip_rk3308_card.c | 97 + 15 files changed, 3159 insertions(+), 6 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rk3308-audio-graph-card.yaml create mode 100644 Documentation/devicetree/bindings/sound/rockchip,rk3308-codec.yaml create mode 100644 include/dt-bindings/sound/rockchip,rk3308-codec.h create mode 100644 sound/soc/codecs/rk3308_codec.c create mode 100644 sound/soc/codecs/rk3308_codec.h create mode 100644 sound/soc/rockchip/rockchip_rk3308_card.c