Skip to contentSkip to conversation

Add the Renesas IDT821034 codec support

PATCH Superseded Herve Codina 4 participants 15 messages Jan 20, 2023 – Jan 23, 2023

Subsystems (5)
Device tree bindings Sound ASoC GPIO +1 more subsystems

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Herve Codina

author maintainer
#0 lore raw

v2 cover Add the Renesas IDT821034 codec support

Hi,

The Renesas IDT821034 codec is four channel PCM codec with on-chip filters and programmable gain setting.
It also provides SLIC (Subscriber Line Interface Circuit) signals as GPIOs.

Based on previous iteration:
https://lore.kernel.org/all/[email protected]/
the bigger change is the codec driver rework in order to remove the regmap virtual registers layer.

Best regards,
Herve Codina

Changes v1 -> v2:
- All patches
Reformat commit log

- Patch 1
Remove '$ref: /schemas/gpio/gpio.yaml#'
Use 'unevaluatedProperties: false'
Update the node name and remove the sound node in the example

- Patch 2
Change the file header comment format
Rework in order to remove the regmap virtual registers

Herve Codina (3):
dt-bindings: sound: Add Renesas IDT821034 codec
ASoC: codecs: Add support for the Renesas IDT821034 codec
MAINTAINERS: add the Renesas IDT821034 codec entry

 .../bindings/sound/renesas,idt821034.yaml     |   75 ++
 MAINTAINERS                                   |    7 +
 sound/soc/codecs/Kconfig                      |   12 +
 sound/soc/codecs/Makefile                     |    2 +
 sound/soc/codecs/idt821034.c                  | 1200 +++++++++++++++++
 5 files changed, 1296 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/renesas,idt821034.yaml
 create mode 100644 sound/soc/codecs/idt821034.c
Signature · 1 line
2.39.0
author maintainer
#1 lore raw

PATCH v2 1/3 dt-bindings: sound: Add Renesas IDT821034 codec

The Renesas IDT821034 codec is a quad PCM codec with programmable gain.

---
 .../bindings/sound/renesas,idt821034.yaml     | 75 +++++++++++++++++++
 1 file changed, 75 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/renesas,idt821034.yaml
Documentation/devicetree/bindings/sound/renesas,idt821034.yaml new file +75 -0
new file mode 100644
index 000000000000..10677d9d8f67
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/renesas,idt821034.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas IDT821034 codec device
+
+maintainers:
+  - Herve Codina <[email protected]>
+
+description: |
+  The IDT821034 codec is a four channel PCM codec with onchip filters and
+  programmable gain setting.
+
+  The time-slots used by the codec must be set and so, the properties
+  'dai-tdm-slot-num', 'dai-tdm-slot-width', 'dai-tdm-slot-tx-mask' and
+  'dai-tdm-slot-rx-mask' must be present in the ALSA sound card node for
+  sub-nodes that involve the codec. The codec uses one 8bit time-slot per
+  channel.
+  'dai-tdm-tdm-slot-with' must be set to 8.
+
+  The IDT821034 codec also supports 5 gpios (SLIC signals) per channel.
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    const: renesas,idt821034
+
+  reg:
+    description:
+      SPI device address.
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 8192000
+
+  spi-cpha: true
+
+  '#sound-dai-cells':
+    const: 0
+
+  '#gpio-cells':
+    const: 2
+
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg
+  - spi-cpha
+  - '#sound-dai-cells'
+  - gpio-controller
+  - '#gpio-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi0 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        audio-codec@0 {
+            compatible = "renesas,idt821034";
+            reg = <0>;
+            spi-max-frequency = <8192000>;
+            spi-cpha;
+            #sound-dai-cells = <0>;
+            gpio-controller;
+            #gpio-cells = <2>;
+        };
+    };
Signature · 1 line
2.39.0
Reviewed-by on v2 1/3
#2 lore raw

Re: [PATCH v2 1/3] dt-bindings: sound: Add Renesas IDT821034 codec

Quoted 5 lines · Herve Codina

The Renesas IDT821034 codec is a quad PCM codec with programmable gain.

---

Thank you for your patch. There is something to discuss/improve.

Quoted 5 lines
Documentation/devicetree/bindings/sound/renesas,idt821034.yaml new file +5 -0
+  gpio-controller: true
+
+required:
+  - compatible
+  - reg

Documentation/devicetree/bindings/sound/renesas,idt821034.yaml line 63

Quoted 10 lines
Documentation/devicetree/bindings/sound/renesas,idt821034.yaml new file +10 -0
+  - spi-cpha
+  - '#sound-dai-cells'
+  - gpio-controller
+  - '#gpio-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi0 {

I didn't notice it earlier - this should be just "spi".

With above:

Best regards,
Krzysztof

author maintainer
#3 lore raw

PATCH v2 2/3 ASoC: codecs: Add support for the Renesas IDT821034 codec

In reply to Herve Codina

The Renesas IDT821034 codec is four channel PCM codec with on-chip filters and programmable gain setting.
It also provides SLIC (Subscriber Line Interface Circuit) signals as GPIOs.

---
 sound/soc/codecs/Kconfig     |   12 +
 sound/soc/codecs/Makefile    |    2 +
 sound/soc/codecs/idt821034.c | 1200 ++++++++++++++++++++++++++++++++++
 3 files changed, 1214 insertions(+)
 create mode 100644 sound/soc/codecs/idt821034.c
sound/soc/codecs/Kconfig +12 -0
index 0f9d71490075..67489b2ebd9f 100644
@@ -107,6 +107,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_HDAC_HDMI
 	imply SND_SOC_HDAC_HDA
 	imply SND_SOC_ICS43432
+	imply SND_SOC_IDT821034
 	imply SND_SOC_INNO_RK3036
 	imply SND_SOC_ISABELLE
 	imply SND_SOC_JZ4740_CODEC
@@ -972,6 +973,17 @@ config SND_SOC_HDA
 config SND_SOC_ICS43432
 	tristate "ICS43423 and compatible i2s microphones"
 
+config SND_SOC_IDT821034
+	tristate "Renesas IDT821034 quad PCM codec"
+	depends on SPI
+	select REGMAP_SPI
+	help
+	  Enable support for the Renesas IDT821034 quad PCM with
+	  programmable gain codec.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called snd-soc-idt821034.
+
 config SND_SOC_INNO_RK3036
 	tristate "Inno codec driver for RK3036 SoC"
 	select REGMAP_MMIO
sound/soc/codecs/Makefile +2 -0
index 71d3ce5867e4..bcf95de654fd 100644
@@ -111,6 +111,7 @@ snd-soc-hdac-hdmi-objs := hdac_hdmi.o
 snd-soc-hdac-hda-objs := hdac_hda.o
 snd-soc-hda-codec-objs := hda.o hda-dai.o
 snd-soc-ics43432-objs := ics43432.o
+snd-soc-idt821034-objs := idt821034.o
 snd-soc-inno-rk3036-objs := inno_rk3036.o
 snd-soc-isabelle-objs := isabelle.o
 snd-soc-jz4740-codec-objs := jz4740.o
@@ -472,6 +473,7 @@ obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
 obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
 obj-$(CONFIG_SND_SOC_HDA) += snd-soc-hda-codec.o
 obj-$(CONFIG_SND_SOC_ICS43432)	+= snd-soc-ics43432.o
+obj-$(CONFIG_SND_SOC_IDT821034)	+= snd-soc-idt821034.o
 obj-$(CONFIG_SND_SOC_INNO_RK3036)	+= snd-soc-inno-rk3036.o
 obj-$(CONFIG_SND_SOC_ISABELLE)	+= snd-soc-isabelle.o
 obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
sound/soc/codecs/idt821034.c new file +1200 -0

1,200 lines not on this page · raw message

Signature · 1 line
2.39.0
maintainer
#4 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Quoted 4 lines · Herve Codina
sound/soc/codecs/idt821034.c new file +4 -0
+static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value;

sound/soc/codecs/idt821034.c line 502

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +9 -0
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	struct idt821034_amp *amp;
 
+
+	amp->gain = val;
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;

_put() methods should return 1 if the value changed to generate events - if you use the mixer-test selftest it'll spot this and other issues for you.

Otherwise this looks fine.

author maintainer
#5 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Hi Mark,

Quoted 10 lines · On Fri, 20 Jan 2023 12:12:44 +0000 Mark Brown <[email protected]> wrote:
Quoted 7 lines · Herve Codina
sound/soc/codecs/idt821034.c new file +7 -0
+static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	struct idt821034_amp *amp;  
Quoted 11 lines

sound/soc/codecs/idt821034.c line 502

Quoted 6 lines
sound/soc/codecs/idt821034.c new file +6 -0
+
+	amp->gain = val;
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;  

_put() methods should return 1 if the value changed to generate events - if you use the mixer-test selftest it'll spot this and other issues for you.

Thanks for pointing this. I will look at mixer-test and fix the _put() methods return code in v3.

Quoted 1 line

Otherwise this looks fine.

Thanks for the review.

Best regards,
Hervé

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#6 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

In reply to Herve Codina

Le 20/01/2023 à 10:50, Herve Codina a écrit :

Quoted 26 lines

The Renesas IDT821034 codec is four channel PCM codec with on-chip filters and programmable gain setting.
It also provides SLIC (Subscriber Line Interface Circuit) signals as GPIOs.

---
  sound/soc/codecs/Kconfig     |   12 +
  sound/soc/codecs/Makefile    |    2 +
  sound/soc/codecs/idt821034.c | 1200 ++++++++++++++++++++++++++++++++++
  3 files changed, 1214 insertions(+)

create mode 100644 sound/soc/codecs/idt821034.c

sound/soc/codecs/Kconfig +1 -0
index 0f9d71490075..67489b2ebd9f 100644
@@ -107,6 +107,7 @@ config SND_SOC_ALL_CODECS
  	imply SND_SOC_HDAC_HDMI
  	imply SND_SOC_HDAC_HDA
  	imply SND_SOC_ICS43432
+	imply SND_SOC_IDT821034
  	imply SND_SOC_INNO_RK3036
  	imply SND_SOC_ISABELLE
  	imply SND_SOC_JZ4740_CODEC

sound/soc/codecs/Kconfig config SND_SOC_HDA · line 979

Quoted 7 lines
sound/soc/codecs/Kconfig +4 -0
@@ -972,6 +973,17 @@ config SND_SOC_HDA
  config SND_SOC_ICS43432
  	tristate "ICS43423 and compatible i2s microphones"
 
+config SND_SOC_IDT821034
+	tristate "Renesas IDT821034 quad PCM codec"
+	depends on SPI
+	select REGMAP_SPI

Is selecting regmap still necessary ?

Quoted 99 lines
sound/soc/codecs/Kconfig +7 -0
+	help
+	  Enable support for the Renesas IDT821034 quad PCM with
+	  programmable gain codec.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called snd-soc-idt821034.
+
  config SND_SOC_INNO_RK3036
  	tristate "Inno codec driver for RK3036 SoC"
  	select REGMAP_MMIO
sound/soc/codecs/Makefile +2 -0
index 71d3ce5867e4..bcf95de654fd 100644
@@ -111,6 +111,7 @@ snd-soc-hdac-hdmi-objs := hdac_hdmi.o
  snd-soc-hdac-hda-objs := hdac_hda.o
  snd-soc-hda-codec-objs := hda.o hda-dai.o
  snd-soc-ics43432-objs := ics43432.o
+snd-soc-idt821034-objs := idt821034.o
  snd-soc-inno-rk3036-objs := inno_rk3036.o
  snd-soc-isabelle-objs := isabelle.o
  snd-soc-jz4740-codec-objs := jz4740.o
@@ -472,6 +473,7 @@ obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
  obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
  obj-$(CONFIG_SND_SOC_HDA) += snd-soc-hda-codec.o
  obj-$(CONFIG_SND_SOC_ICS43432)	+= snd-soc-ics43432.o
+obj-$(CONFIG_SND_SOC_IDT821034)	+= snd-soc-idt821034.o
  obj-$(CONFIG_SND_SOC_INNO_RK3036)	+= snd-soc-inno-rk3036.o
  obj-$(CONFIG_SND_SOC_ISABELLE)	+= snd-soc-isabelle.o
  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
sound/soc/codecs/idt821034.c new file +63 -0
new file mode 100644
index 000000000000..5eb93fec6042
@@ -0,0 +1,1200 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// IDT821034 ALSA SoC driver
+//
+// Copyright 2022 CS GROUP France
+//
+// Author: Herve Codina <[email protected]>
+
+#include <linux/bitrev.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#define IDT821034_NB_CHANNEL	4
+
+struct idt821034_amp {
+	u16 gain;
+	bool is_muted;
+};
+
+struct idt821034 {
+	struct spi_device *spi;
+	struct mutex mutex;
+	u8 spi_tx_buf; /* Cannot use stack area for SPI (dma-safe memory) */
+	u8 spi_rx_buf; /* Cannot use stack area for SPI (dma-safe memory) */
+	struct {
+		u8 codec_conf;
+		struct {
+			u8 power;
+			u8 tx_slot;
+			u8 rx_slot;
+			u8 slic_conf;
+			u8 slic_control;
+		} ch[IDT821034_NB_CHANNEL];
+	} cache;
+	struct {
+		struct {
+			struct idt821034_amp amp_out;
+			struct idt821034_amp amp_in;
+		} ch[IDT821034_NB_CHANNEL];
+	} amps;
+	int max_ch_playback;
+	int max_ch_capture;
+	struct gpio_chip gpio_chip;
+};
+
+static int idt821034_8bit_write(struct idt821034 *idt821034, u8 val)
+{
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = &idt821034->spi_tx_buf,
+			.len = 1,
+		}, {
+			.cs_off = 1,
+			.tx_buf = &idt821034->spi_tx_buf,
+			.len = 1,
+		}
+	};
+	int ret;

sound/soc/codecs/idt821034.c line 73

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+
+	idt821034->spi_tx_buf = val;
+
+	dev_vdbg(&idt821034->spi->dev, "spi xfer wr 0x%x\n", val);
+
+	ret = spi_sync_transfer(idt821034->spi, xfer, 2);
+	if (ret)
+		return ret;
+
+	return 0;

Looks like you could just do:

	return spi_sync_transfer(idt821034->spi, xfer, 2);
Quoted 22 lines
sound/soc/codecs/idt821034.c new file +22 -0
+}
+
+static int idt821034_8bit_read(struct idt821034 *idt821034, u8 valw, u8 *valr)
+{
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = &idt821034->spi_tx_buf,
+			.rx_buf = &idt821034->spi_rx_buf,
+			.len = 1,
+		}, {
+			.cs_off = 1,
+			.tx_buf = &idt821034->spi_tx_buf,
+			.len = 1,
+		}
+	};
+	int ret;
+
+	idt821034->spi_tx_buf = valw;
+
+	ret = spi_sync_transfer(idt821034->spi, xfer, 2);
+	if (ret)
+		return ret;

sound/soc/codecs/idt821034.c line 105

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+
+	*valr = idt821034->spi_rx_buf;
+
+	dev_vdbg(&idt821034->spi->dev, "spi xfer wr 0x%x, rd 0x%x\n",
+		 valw, *valr);
+
+	return 0;
+}
+
+/* Available mode the programming sequence */

s/the/for/ ?

Quoted 31 lines
sound/soc/codecs/idt821034.c new file +31 -0
+#define IDT821034_MODE_CODEC(_ch) (0x80 | ((_ch) << 2))
+#define IDT821034_MODE_SLIC(_ch)  (0xD0 | ((_ch) << 2))
+#define IDT821034_MODE_GAIN(_ch)  (0xC0 | ((_ch) << 2))
+
+/* Power values that can be used in 'power' (can be ORed) */
+#define IDT821034_CONF_PWRUP_TX		BIT(1) /* from analog input to PCM */
+#define IDT821034_CONF_PWRUP_RX		BIT(0) /* from PCM to analog output */
+
+static int idt821034_set_channel_power(struct idt821034 *idt821034, u8 ch, u8 power)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_channel_power(%u, 0x%x)\n", ch, power);
+
+	conf = IDT821034_MODE_CODEC(ch) | idt821034->cache.codec_conf;
+
+	if (power & IDT821034_CONF_PWRUP_RX) {
+		ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_RX);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].rx_slot);
+		if (ret)
+			return ret;
+	}
+	if (power & IDT821034_CONF_PWRUP_TX) {
+		ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_TX);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].tx_slot);
+		if (ret)

sound/soc/codecs/idt821034.c line 146

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+			return ret;
+	}
+	if (!(power & (IDT821034_CONF_PWRUP_TX | IDT821034_CONF_PWRUP_RX))) {
+		ret = idt821034_8bit_write(idt821034, conf);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, 0x00);
+		if (ret)
+			return ret;
+	}

Can we refactor the three actions with an helper, that could also be reused for idt821034_set_codec_conf() and idt821034_set_channel_ts() and idt821034_set_slic_conf() and idt821034_write_slic_raw() and idt821034_set_gain_channel, something like for instance:

static int idt821034_set_conf(struct idt821034 *idt821034, u8 conf, u8 val)

{
	ret = idt821034_8bit_write(idt821034, conf);
	if (ret)
		return ret;
	return idt821034_8bit_write(idt821034, val);
}
Quoted 815 lines
sound/soc/codecs/idt821034.c new file +815 -0
+
+	idt821034->cache.ch[ch].power = power;
+
+	return 0;
+}
+
+static u8 idt821034_get_channel_power(struct idt821034 *idt821034, u8 ch)
+{
+	return idt821034->cache.ch[ch].power;
+}
+
+/* Codec configuration values that can be used in 'codec_conf' (can be ORed) */
+#define IDT821034_CONF_ALAW_MODE	BIT(5)
+#define IDT821034_CONF_DELAY_MODE	BIT(4)
+
+static int idt821034_set_codec_conf(struct idt821034 *idt821034, u8 codec_conf)
+{
+	u8 conf;
+	u8 ts;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_codec_conf(0x%x)\n", codec_conf);
+
+	/* codec conf fields are common to all channel.
+	 * Arbitrary use of channel 0 for this configuration.
+	 */
+
+	/* Set Configuration Register */
+	conf = IDT821034_MODE_CODEC(0) | codec_conf;
+
+	/* Update conf value and timeslot register value according
+	 * to cache values
+	 */
+	if (idt821034->cache.ch[0].power & IDT821034_CONF_PWRUP_RX) {
+		conf |= IDT821034_CONF_PWRUP_RX;
+		ts = idt821034->cache.ch[0].rx_slot;
+	} else if (idt821034->cache.ch[0].power & IDT821034_CONF_PWRUP_TX) {
+		conf |= IDT821034_CONF_PWRUP_TX;
+		ts = idt821034->cache.ch[0].tx_slot;
+	} else {
+		ts = 0x00;
+	}
+
+	/* Write configuration register and time-slot register */
+	ret = idt821034_8bit_write(idt821034, conf);
+	if (ret)
+		return ret;
+	ret = idt821034_8bit_write(idt821034, ts);
+	if (ret)
+		return ret;
+
+	idt821034->cache.codec_conf = codec_conf;
+	return 0;
+}
+
+static u8 idt821034_get_codec_conf(struct idt821034 *idt821034)
+{
+	return idt821034->cache.codec_conf;
+}
+
+/* Channel direction values that can be used in 'ch_dir' (can be ORed) */
+#define IDT821034_CH_RX		BIT(0) /* from PCM to analog output */
+#define IDT821034_CH_TX		BIT(1) /* from analog input to PCM */
+
+static int idt821034_set_channel_ts(struct idt821034 *idt821034, u8 ch, u8 ch_dir, u8 ts_num)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_channel_ts(%u, 0x%x, %d)\n", ch, ch_dir, ts_num);
+
+	conf = IDT821034_MODE_CODEC(ch) | idt821034->cache.codec_conf;
+
+	if (ch_dir & IDT821034_CH_RX) {
+		if (idt821034->cache.ch[ch].power & IDT821034_CONF_PWRUP_RX) {
+			ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_RX);
+			if (ret)
+				return ret;
+			ret = idt821034_8bit_write(idt821034, ts_num);
+			if (ret)
+				return ret;
+		}
+		idt821034->cache.ch[ch].rx_slot = ts_num;
+	}
+	if (ch_dir & IDT821034_CH_TX) {
+		if (idt821034->cache.ch[ch].power & IDT821034_CONF_PWRUP_TX) {
+			ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_TX);
+			if (ret)
+				return ret;
+			ret = idt821034_8bit_write(idt821034, ts_num);
+			if (ret)
+				return ret;
+		}
+		idt821034->cache.ch[ch].tx_slot = ts_num;
+	}
+
+	return 0;
+}
+
+/* SLIC direction values that can be used in 'slic_dir' (can be ORed) */
+#define IDT821034_SLIC_IO1_IN       BIT(1)
+#define IDT821034_SLIC_IO0_IN       BIT(0)
+
+static int idt821034_set_slic_conf(struct idt821034 *idt821034, u8 ch, u8 slic_dir)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_slic_conf(%u, 0x%x)\n", ch, slic_dir);
+
+	conf = IDT821034_MODE_SLIC(ch) | slic_dir;
+	ret = idt821034_8bit_write(idt821034, conf);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].slic_control);
+	if (ret)
+		return ret;
+
+	idt821034->cache.ch[ch].slic_conf = slic_dir;
+
+	return 0;
+}
+
+static u8 idt821034_get_slic_conf(struct idt821034 *idt821034, u8 ch)
+{
+	return idt821034->cache.ch[ch].slic_conf;
+}
+
+static int idt821034_write_slic_raw(struct idt821034 *idt821034, u8 ch, u8 slic_raw)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "write_slic_raw(%u, 0x%x)\n", ch, slic_raw);
+
+	/*
+	 * On write, slic_raw is mapped as follow :
+	 *   b4: O_4
+	 *   b3: O_3
+	 *   b2: O_2
+	 *   b1: I/O_1
+	 *   b0: I/O_0
+	 */
+
+	conf = IDT821034_MODE_SLIC(ch) | idt821034->cache.ch[ch].slic_conf;
+	ret = idt821034_8bit_write(idt821034, conf);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, slic_raw);
+	if (ret)
+		return ret;
+
+	idt821034->cache.ch[ch].slic_control = slic_raw;
+	return 0;
+}
+
+static u8 idt821034_get_written_slic_raw(struct idt821034 *idt821034, u8 ch)
+{
+	return idt821034->cache.ch[ch].slic_control;
+}
+
+static int idt821034_read_slic_raw(struct idt821034 *idt821034, u8 ch, u8 *slic_raw)
+{
+	u8 val;
+	int ret;
+
+	/*
+	 * On read, slic_raw is mapped as follow :
+	 *   b7: I/O_0
+	 *   b6: I/O_1
+	 *   b5: O_2
+	 *   b4: O_3
+	 *   b3: O_4
+	 *   b2: I/O1_0, I/O_0 from channel 1 (no matter ch value)
+	 *   b1: I/O2_0, I/O_0 from channel 2 (no matter ch value)
+	 *   b2: I/O3_0, I/O_0 from channel 3 (no matter ch value)
+	 */
+
+	val = IDT821034_MODE_SLIC(ch) | idt821034->cache.ch[ch].slic_conf;
+	ret = idt821034_8bit_write(idt821034, val);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_read(idt821034, idt821034->cache.ch[ch].slic_control, slic_raw);
+	if (ret)
+		return ret;
+
+	dev_dbg(&idt821034->spi->dev, "read_slic_raw(%i) 0x%x\n", ch, *slic_raw);
+
+	return 0;
+}
+
+/* Gain type values that can be used in 'gain_type' (cannot be ORed) */
+#define IDT821034_GAIN_RX		(0 << 1) /* from PCM to analog output */
+#define IDT821034_GAIN_TX		(1 << 1) /* from analog input to PCM */
+
+static int idt821034_set_gain_channel(struct idt821034 *idt821034, u8 ch,
+				      u8 gain_type, u16 gain_val)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_gain_channel(%u, 0x%x, 0x%x-%d)\n",
+		ch, gain_type, gain_val, gain_val);
+
+	/*
+	 * The gain programming coefficients should be calculated as:
+	 *   Transmit : Coeff_X = round [ gain_X0dB × gain_X ]
+	 *   Receive: Coeff_R = round [ gain_R0dB × gain_R ]
+	 * where:
+	 *   gain_X0dB = 1820;
+	 *   gain_X is the target gain;
+	 *   Coeff_X should be in the range of 0 to 8192.
+	 *   gain_R0dB = 2506;
+	 *   gain_R is the target gain;
+	 *   Coeff_R should be in the range of 0 to 8192.
+	 *
+	 * A gain programming coefficient is 14-bit wide and in binary format.
+	 * The 7 Most Significant Bits of the coefficient is called
+	 * GA_MSB_Transmit for transmit path, or is called GA_MSB_Receive for
+	 * receive path; The 7 Least Significant Bits of the coefficient is
+	 * called GA_LSB_ Transmit for transmit path, or is called
+	 * GA_LSB_Receive for receive path.
+	 *
+	 * An example is given below to clarify the calculation of the
+	 * coefficient. To program a +3 dB gain in transmit path and a -3.5 dB
+	 * gain in receive path:
+	 *
+	 * Linear Code of +3dB = 10^(3/20)= 1.412537545
+	 * Coeff_X = round (1820 × 1.412537545) = 2571
+	 *                                      = 0b001010_00001011
+	 * GA_MSB_Transmit = 0b0010100
+	 * GA_LSB_Transmit = 0b0001011
+	 *
+	 * Linear Code of -3.5dB = 10^(-3.5/20) = 0.668343917
+	 * Coeff_R= round (2506 × 0.668343917) = 1675
+	 *                                     = 0b0001101_0001011
+	 * GA_MSB_Receive = 0b0001101
+	 * GA_LSB_Receive = 0b0001011
+	 */
+
+	conf = IDT821034_MODE_GAIN(ch) | gain_type;
+
+	ret = idt821034_8bit_write(idt821034, conf | 0x00);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, gain_val & 0x007F);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, conf | 0x01);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, (gain_val >> 7) & 0x7F);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Id helpers used in controls and dapm */
+#define IDT821034_DIR_OUT (1 << 3)
+#define IDT821034_DIR_IN  (0 << 3)
+#define IDT821034_ID(_ch, _dir) (((_ch) & 0x03) | (_dir))
+#define IDT821034_ID_OUT(_ch) IDT821034_ID(_ch, IDT821034_DIR_OUT)
+#define IDT821034_ID_IN(_ch)  IDT821034_ID(_ch, IDT821034_DIR_IN)
+
+#define IDT821034_ID_GET_CHAN(_id) ((_id) & 0x03)
+#define IDT821034_ID_GET_DIR(_id) ((_id) & (1 << 3))
+#define IDT821034_ID_IS_OUT(_id) (IDT821034_ID_GET_DIR(_id) == IDT821034_DIR_OUT)
+
+static int idt821034_kctrl_gain_get(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int min = mc->min;
+	int max = mc->max;
+	unsigned int mask = (1 << fls(max)) - 1;
+	unsigned int invert = mc->invert;
+	int val;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(mc->reg);
+
+	mutex_lock(&idt821034->mutex);
+	if (IDT821034_ID_IS_OUT(mc->reg))
+		val = idt821034->amps.ch[ch].amp_out.gain;
+	else
+		val = idt821034->amps.ch[ch].amp_in.gain;
+	mutex_unlock(&idt821034->mutex);
+
+	ucontrol->value.integer.value[0] = val & mask;
+	if (invert)
+		ucontrol->value.integer.value[0] = max - ucontrol->value.integer.value[0];
+	else
+		ucontrol->value.integer.value[0] = ucontrol->value.integer.value[0] - min;
+
+	return 0;
+}
+
+static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	struct idt821034_amp *amp;
+	int min = mc->min;
+	int max = mc->max;
+	unsigned int mask = (1 << fls(max)) - 1;
+	unsigned int invert = mc->invert;
+	unsigned int val;
+	int ret;
+	u8 gain_type;
+	u8 ch;
+
+	val = ucontrol->value.integer.value[0];
+	if (val < 0)
+		return -EINVAL;
+	if (val > max - min)
+		return -EINVAL;
+
+	if (invert)
+		val = (max - val) & mask;
+	else
+		val = (val + min) & mask;
+
+	ch = IDT821034_ID_GET_CHAN(mc->reg);
+
+	mutex_lock(&idt821034->mutex);
+
+	if (IDT821034_ID_IS_OUT(mc->reg)) {
+		amp = &idt821034->amps.ch[ch].amp_out;
+		gain_type = IDT821034_GAIN_RX;
+	} else {
+		amp = &idt821034->amps.ch[ch].amp_in;
+		gain_type = IDT821034_GAIN_TX;
+	}
+
+	if (!amp->is_muted) {
+		ret = idt821034_set_gain_channel(idt821034, ch, gain_type, val);
+		if (ret)
+			goto end;
+	}
+
+	amp->gain = val;
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_kctrl_mute_get(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int id = kcontrol->private_value;
+	bool is_muted;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(id);
+
+	mutex_lock(&idt821034->mutex);
+	is_muted = IDT821034_ID_IS_OUT(id) ?
+			idt821034->amps.ch[ch].amp_out.is_muted:
+			idt821034->amps.ch[ch].amp_in.is_muted;
+	mutex_unlock(&idt821034->mutex);
+
+	ucontrol->value.integer.value[0] = !is_muted;
+
+	return 0;
+}
+
+static int idt821034_kctrl_mute_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int id = kcontrol->private_value;
+	struct idt821034_amp *amp;
+	bool is_mute;
+	u8 gain_type;
+	int ret;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(id);
+	is_mute = !ucontrol->value.integer.value[0];
+
+	mutex_lock(&idt821034->mutex);
+
+	if (IDT821034_ID_IS_OUT(id)) {
+		amp = &idt821034->amps.ch[ch].amp_out;
+		gain_type = IDT821034_GAIN_RX;
+	} else {
+		amp = &idt821034->amps.ch[ch].amp_in;
+		gain_type = IDT821034_GAIN_TX;
+	}
+
+	ret = idt821034_set_gain_channel(idt821034, ch, gain_type,
+					 is_mute ? 0 : amp->gain);
+	if (ret)
+		goto end;
+
+	amp->is_muted = is_mute;
+
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static const DECLARE_TLV_DB_LINEAR(idt821034_gain_in, -6520, 1306);
+#define IDT821034_GAIN_IN_MIN_RAW	1 /* -65.20 dB -> 10^(-65.2/20.0) * 1820 = 1 */
+#define IDT821034_GAIN_IN_MAX_RAW	8191 /* 13.06 dB -> 10^(13.06/20.0) * 1820 = 8191 */
+#define IDT821034_GAIN_IN_INIT_RAW	1820 /* 0dB -> 10^(0/20) * 1820 = 1820 */
+
+static const DECLARE_TLV_DB_LINEAR(idt821034_gain_out, -6798, 1029);
+#define IDT821034_GAIN_OUT_MIN_RAW	1 /* -67.98 dB -> 10^(-67.98/20.0) * 2506 = 1*/
+#define IDT821034_GAIN_OUT_MAX_RAW	8191 /* 10.29 dB -> 10^(10.29/20.0) * 2506 = 8191 */
+#define IDT821034_GAIN_OUT_INIT_RAW	2506 /* 0dB -> 10^(0/20) * 2506 = 2506 */
+
+static const struct snd_kcontrol_new idt821034_controls[] = {
+	/* DAC volume control */
+	SOC_SINGLE_RANGE_EXT_TLV("DAC0 Playback Volume", IDT821034_ID_OUT(0), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+	SOC_SINGLE_RANGE_EXT_TLV("DAC1 Playback Volume", IDT821034_ID_OUT(1), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+	SOC_SINGLE_RANGE_EXT_TLV("DAC2 Playback Volume", IDT821034_ID_OUT(2), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+	SOC_SINGLE_RANGE_EXT_TLV("DAC3 Playback Volume", IDT821034_ID_OUT(3), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+
+	/* DAC mute control */
+	SOC_SINGLE_BOOL_EXT("DAC0 Playback Switch", IDT821034_ID_OUT(0),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("DAC1 Playback Switch", IDT821034_ID_OUT(1),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("DAC2 Playback Switch", IDT821034_ID_OUT(2),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("DAC3 Playback Switch", IDT821034_ID_OUT(3),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+
+	/* ADC volume control */
+	SOC_SINGLE_RANGE_EXT_TLV("ADC0 Capture Volume", IDT821034_ID_IN(0), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+	SOC_SINGLE_RANGE_EXT_TLV("ADC1 Capture Volume", IDT821034_ID_IN(1), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+	SOC_SINGLE_RANGE_EXT_TLV("ADC2 Capture Volume", IDT821034_ID_IN(2), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+	SOC_SINGLE_RANGE_EXT_TLV("ADC3 Capture Volume", IDT821034_ID_IN(3), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+
+	/* ADC mute control */
+	SOC_SINGLE_BOOL_EXT("ADC0 Capture Switch", IDT821034_ID_IN(0),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("ADC1 Capture Switch", IDT821034_ID_IN(1),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("ADC2 Capture Switch", IDT821034_ID_IN(2),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("ADC3 Capture Switch", IDT821034_ID_IN(3),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+};
+
+static int idt821034_power_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	unsigned int id = w->shift;
+	u8 power, mask;
+	int ret;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(id);
+	mask = IDT821034_ID_IS_OUT(id) ? IDT821034_CONF_PWRUP_RX : IDT821034_CONF_PWRUP_TX;
+
+	mutex_lock(&idt821034->mutex);
+
+	power = idt821034_get_channel_power(idt821034, ch);
+	if (SND_SOC_DAPM_EVENT_ON(event))
+		power |= mask;
+	else
+		power &= ~mask;
+	ret = idt821034_set_channel_power(idt821034, ch, power);
+
+	mutex_unlock(&idt821034->mutex);
+
+	return ret;
+}
+
+static const struct snd_soc_dapm_widget idt821034_dapm_widgets[] = {
+	SND_SOC_DAPM_DAC_E("DAC0", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(0), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("DAC1", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(1), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("DAC2", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(2), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("DAC3", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(3), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+
+	SND_SOC_DAPM_OUTPUT("OUT0"),
+	SND_SOC_DAPM_OUTPUT("OUT1"),
+	SND_SOC_DAPM_OUTPUT("OUT2"),
+	SND_SOC_DAPM_OUTPUT("OUT3"),
+
+	SND_SOC_DAPM_DAC_E("ADC0", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(0), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("ADC1", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(1), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("ADC2", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(2), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("ADC3", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(3), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+
+	SND_SOC_DAPM_INPUT("IN0"),
+	SND_SOC_DAPM_INPUT("IN1"),
+	SND_SOC_DAPM_INPUT("IN2"),
+	SND_SOC_DAPM_INPUT("IN3"),
+};
+
+static const struct snd_soc_dapm_route idt821034_dapm_routes[] = {
+	{ "OUT0", NULL, "DAC0" },
+	{ "OUT1", NULL, "DAC1" },
+	{ "OUT2", NULL, "DAC2" },
+	{ "OUT3", NULL, "DAC3" },
+
+	{ "ADC0", NULL, "IN0" },
+	{ "ADC1", NULL, "IN1" },
+	{ "ADC2", NULL, "IN2" },
+	{ "ADC3", NULL, "IN3" },
+};
+
+static int idt821034_dai_set_tdm_slot(struct snd_soc_dai *dai,
+				      unsigned int tx_mask, unsigned int rx_mask,
+				      int slots, int width)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int mask;
+	u8 slot;
+	int ret;
+	u8 ch;
+
+	switch (width) {
+	case 0: /* Not set -> default 8 */
+	case 8:
+		break;
+	default:
+		dev_err(dai->dev, "tdm slot width %d not supported\n", width);
+		return -EINVAL;
+	}
+
+	mask = tx_mask;
+	slot = 0;
+	ch = 0;
+	while (mask && ch < IDT821034_NB_CHANNEL) {
+		if (mask & 0x1) {
+			mutex_lock(&idt821034->mutex);
+			ret = idt821034_set_channel_ts(idt821034, ch, IDT821034_CH_RX, slot);
+			mutex_unlock(&idt821034->mutex);
+			if (ret) {
+				dev_err(dai->dev, "ch%u set tx tdm slot failed (%d)\n",
+					ch, ret);
+				return ret;
+			}
+			ch++;
+		}
+		mask >>= 1;
+		slot++;
+	}
+	if (mask) {
+		dev_err(dai->dev, "too much tx slots defined (mask = 0x%x) support max %d\n",
+			tx_mask, IDT821034_NB_CHANNEL);
+		return -EINVAL;
+	}
+	idt821034->max_ch_playback = ch;
+
+	mask = rx_mask;
+	slot = 0;
+	ch = 0;
+	while (mask && ch < IDT821034_NB_CHANNEL) {
+		if (mask & 0x1) {
+			mutex_lock(&idt821034->mutex);
+			ret = idt821034_set_channel_ts(idt821034, ch, IDT821034_CH_TX, slot);
+			mutex_unlock(&idt821034->mutex);
+			if (ret) {
+				dev_err(dai->dev, "ch%u set rx tdm slot failed (%d)\n",
+					ch, ret);
+				return ret;
+			}
+			ch++;
+		}
+		mask >>= 1;
+		slot++;
+	}
+	if (mask) {
+		dev_err(dai->dev, "too much rx slots defined (mask = 0x%x) support max %d\n",
+			rx_mask, IDT821034_NB_CHANNEL);
+		return -EINVAL;
+	}
+	idt821034->max_ch_capture = ch;
+
+	return 0;
+}
+
+static int idt821034_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	u8 conf;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+
+	conf = idt821034_get_codec_conf(idt821034);
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_DSP_A:
+		conf |= IDT821034_CONF_DELAY_MODE;
+		break;
+	case SND_SOC_DAIFMT_DSP_B:
+		conf &= ~IDT821034_CONF_DELAY_MODE;
+		break;
+	default:
+		dev_err(dai->dev, "Unsupported DAI format 0x%x\n",
+			fmt & SND_SOC_DAIFMT_FORMAT_MASK);
+		ret = -EINVAL;
+		goto end;
+	}
+	ret = idt821034_set_codec_conf(idt821034, conf);
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_dai_hw_params(struct snd_pcm_substream *substream,
+				   struct snd_pcm_hw_params *params,
+				   struct snd_soc_dai *dai)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	u8 conf;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+
+	conf = idt821034_get_codec_conf(idt821034);
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_A_LAW:
+		conf |= IDT821034_CONF_ALAW_MODE;
+		break;
+	case SNDRV_PCM_FORMAT_MU_LAW:
+		conf &= ~IDT821034_CONF_ALAW_MODE;
+		break;
+	default:
+		dev_err(dai->dev, "Unsupported PCM format 0x%x\n",
+			params_format(params));
+		ret = -EINVAL;
+		goto end;
+	}
+	ret = idt821034_set_codec_conf(idt821034, conf);
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static const unsigned int idt821034_sample_bits[] = {8};
+
+static struct snd_pcm_hw_constraint_list idt821034_sample_bits_constr = {
+	.list = idt821034_sample_bits,
+	.count = ARRAY_SIZE(idt821034_sample_bits),
+};
+
+static int idt821034_dai_startup(struct snd_pcm_substream *substream,
+				 struct snd_soc_dai *dai)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int max_ch = 0;
+	int ret;
+
+	max_ch = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+		idt821034->max_ch_playback : idt821034->max_ch_capture;
+
+	/*
+	 * Disable stream support (min = 0, max = 0) if no timeslots were
+	 * configured otherwise, limit the number of channels to those
+	 * configured.
+	 */
+	ret = snd_pcm_hw_constraint_minmax(substream->runtime, SNDRV_PCM_HW_PARAM_CHANNELS,
+					   max_ch ? 1 : 0, max_ch);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_SAMPLE_BITS,
+					 &idt821034_sample_bits_constr);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static u64 idt821034_dai_formats[] = {
+	SND_SOC_POSSIBLE_DAIFMT_DSP_A	|
+	SND_SOC_POSSIBLE_DAIFMT_DSP_B,
+};
+
+static const struct snd_soc_dai_ops idt821034_dai_ops = {
+	.startup      = idt821034_dai_startup,
+	.hw_params    = idt821034_dai_hw_params,
+	.set_tdm_slot = idt821034_dai_set_tdm_slot,
+	.set_fmt      = idt821034_dai_set_fmt,
+	.auto_selectable_formats     = idt821034_dai_formats,
+	.num_auto_selectable_formats = ARRAY_SIZE(idt821034_dai_formats),
+};
+
+static struct snd_soc_dai_driver idt821034_dai_driver = {
+	.name = "idt821034",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 1,
+		.channels_max = IDT821034_NB_CHANNEL,
+		.rates = SNDRV_PCM_RATE_8000,
+		.formats = SNDRV_PCM_FMTBIT_MU_LAW | SNDRV_PCM_FMTBIT_A_LAW,
+	},
+	.capture = {
+		.stream_name = "Capture",
+		.channels_min = 1,
+		.channels_max = IDT821034_NB_CHANNEL,
+		.rates = SNDRV_PCM_RATE_8000,
+		.formats = SNDRV_PCM_FMTBIT_MU_LAW | SNDRV_PCM_FMTBIT_A_LAW,
+	},
+	.ops = &idt821034_dai_ops,
+};
+
+static int idt821034_reset_audio(struct idt821034 *idt821034)
+{
+	int ret;
+	u8 i;
+
+	mutex_lock(&idt821034->mutex);
+
+	ret = idt821034_set_codec_conf(idt821034, 0);
+	if (ret)
+		goto end;
+
+	for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
+		idt821034->amps.ch[i].amp_out.gain = IDT821034_GAIN_OUT_INIT_RAW;
+		idt821034->amps.ch[i].amp_out.is_muted = false;
+		ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_RX,
+						 idt821034->amps.ch[i].amp_out.gain);
+		if (ret)
+			goto end;
+
+		idt821034->amps.ch[i].amp_in.gain = IDT821034_GAIN_IN_INIT_RAW;
+		idt821034->amps.ch[i].amp_in.is_muted = false;
+		ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_TX,
+						 idt821034->amps.ch[i].amp_in.gain);
+		if (ret)
+			goto end;
+
+		ret = idt821034_set_channel_power(idt821034, i, 0);
+		if (ret)
+			goto end;
+	}
+
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_component_probe(struct snd_soc_component *component)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	/* reset idt821034 audio part*/
+	ret = idt821034_reset_audio(idt821034);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver idt821034_component_driver = {
+	.probe			= idt821034_component_probe,

sound/soc/codecs/idt821034.c line 971

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+	.controls		= idt821034_controls,
+	.num_controls		= ARRAY_SIZE(idt821034_controls),
+	.dapm_widgets		= idt821034_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(idt821034_dapm_widgets),
+	.dapm_routes		= idt821034_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(idt821034_dapm_routes),
+	.endianness		= 1,
+};
+
+#if IS_ENABLED(CONFIG_GPIOLIB)

#ifdef CONFIG_GPIOLIB ?

Is that #if section needed at all ? Isn't it possible to have it all the time and check IS_ENABLED(CONFIG_GPIOLIB) in the probe before calling the init ? Something like :

         if (IS_ENABLED(CONFIG_GPIOLIB))
		return idt821034_gpio_init(idt821034);
	return 0;
Quoted 140 lines
sound/soc/codecs/idt821034.c new file +140 -0
+#define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) / 5) % 4)
+#define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset)    BIT((_offset) % 5)
+
+static void idt821034_chip_gpio_set(struct gpio_chip *c, unsigned int offset, int val)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_raw;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+
+	slic_raw = idt821034_get_written_slic_raw(idt821034, ch);
+	if (val)
+		slic_raw |= mask;
+	else
+		slic_raw &= ~mask;
+	ret = idt821034_write_slic_raw(idt821034, ch, slic_raw);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "set gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+	}
+
+	mutex_unlock(&idt821034->mutex);
+}
+
+static int idt821034_chip_gpio_get(struct gpio_chip *c, unsigned int offset)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_raw;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+	ret = idt821034_read_slic_raw(idt821034, ch, &slic_raw);
+	mutex_unlock(&idt821034->mutex);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "get gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+		return ret;
+	}
+
+	/*
+	 * SLIC IOs are read in reverse order compared to write.
+	 * Reverse the read value here in order to have IO0 at lsb (ie same
+	 * order as write)
+	 */
+	return !!(bitrev8(slic_raw) & mask);
+}
+
+static int idt821034_chip_get_direction(struct gpio_chip *c, unsigned int offset)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_dir;
+
+	mutex_lock(&idt821034->mutex);
+	slic_dir = idt821034_get_slic_conf(idt821034, ch);
+	mutex_unlock(&idt821034->mutex);
+
+	return slic_dir & mask ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;
+}
+
+static int idt821034_chip_direction_input(struct gpio_chip *c, unsigned int offset)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_conf;
+	int ret;
+
+	/* Only IO0 and IO1 can be set as input */
+	if (mask & ~(IDT821034_SLIC_IO1_IN | IDT821034_SLIC_IO0_IN))
+		return -EPERM;
+
+	mutex_lock(&idt821034->mutex);
+
+	slic_conf = idt821034_get_slic_conf(idt821034, ch) | mask;
+
+	ret = idt821034_set_slic_conf(idt821034, ch, slic_conf);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "dir in gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+	}
+
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_chip_direction_output(struct gpio_chip *c, unsigned int offset, int val)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_conf;
+	int ret;
+
+	idt821034_chip_gpio_set(c, offset, val);
+
+	mutex_lock(&idt821034->mutex);
+
+	slic_conf = idt821034_get_slic_conf(idt821034, ch) & ~mask;
+
+	ret = idt821034_set_slic_conf(idt821034, ch, slic_conf);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "dir in gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+	}
+
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_reset_gpio(struct idt821034 *idt821034)
+{
+	int ret;
+	u8 i;
+
+	mutex_lock(&idt821034->mutex);
+
+	/* IO0 and IO1 as input for all channels and output IO set to 0 */
+	for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
+		ret = idt821034_set_slic_conf(idt821034, i,
+					      IDT821034_SLIC_IO1_IN | IDT821034_SLIC_IO0_IN);
+		if (ret)
+			goto end;
+
+		ret = idt821034_write_slic_raw(idt821034, i, 0);
+		if (ret)
+			goto end;
+
+	}
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}

sound/soc/codecs/idt821034.c line 1121

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+
+static int idt821034_gpio_init(struct idt821034 *idt821034)
+{
+	int ret;
+
+	ret = idt821034_reset_gpio(idt821034);
+	if (ret)
+		return ret;
+
+	idt821034->gpio_chip.owner = THIS_MODULE;

Is the owner really needed ?

Quoted 79 lines
sound/soc/codecs/idt821034.c new file +79 -0
+	idt821034->gpio_chip.label = dev_name(&idt821034->spi->dev);
+	idt821034->gpio_chip.parent = &idt821034->spi->dev;
+	idt821034->gpio_chip.base = -1;
+	idt821034->gpio_chip.ngpio = 5 * 4; /* 5 GPIOs on 4 channels */
+	idt821034->gpio_chip.get_direction = idt821034_chip_get_direction;
+	idt821034->gpio_chip.direction_input = idt821034_chip_direction_input;
+	idt821034->gpio_chip.direction_output = idt821034_chip_direction_output;
+	idt821034->gpio_chip.get = idt821034_chip_gpio_get;
+	idt821034->gpio_chip.set = idt821034_chip_gpio_set;
+	idt821034->gpio_chip.can_sleep = true;
+
+	return devm_gpiochip_add_data(&idt821034->spi->dev, &idt821034->gpio_chip,
+				      idt821034);
+}
+#else /* IS_ENABLED(CONFIG_GPIOLIB) */
+static int idt821034_gpio_init(struct idt821034 *idt821034)
+{
+	return 0;
+}
+#endif
+
+static int idt821034_spi_probe(struct spi_device *spi)
+{
+	struct idt821034 *idt821034;
+	int ret;
+
+	spi->bits_per_word = 8;
+	ret = spi_setup(spi);
+	if (ret < 0)
+		return ret;
+
+	idt821034 = devm_kzalloc(&spi->dev, sizeof(*idt821034), GFP_KERNEL);
+	if (!idt821034)
+		return -ENOMEM;
+
+	idt821034->spi = spi;
+
+	mutex_init(&idt821034->mutex);
+
+	spi_set_drvdata(spi, idt821034);
+
+	ret = devm_snd_soc_register_component(&spi->dev, &idt821034_component_driver,
+					      &idt821034_dai_driver, 1);
+	if (ret)
+		return ret;
+
+	ret = idt821034_gpio_init(idt821034);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id idt821034_of_match[] = {
+	{ .compatible = "renesas,idt821034", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, idt821034_of_match);
+
+static const struct spi_device_id idt821034_id_table[] = {
+	{ "idt821034", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, idt821034_id_table);
+
+static struct spi_driver idt821034_spi_driver = {
+	.driver  = {
+		.name   = "idt821034",
+		.of_match_table = idt821034_of_match,
+	},
+	.id_table = idt821034_id_table,
+	.probe  = idt821034_spi_probe,
+};
+
+module_spi_driver(idt821034_spi_driver);
+
+MODULE_AUTHOR("Herve Codina <[email protected]>");
+MODULE_DESCRIPTION("IDT821034 ALSA SoC driver");
+MODULE_LICENSE("GPL");
author maintainer
#7 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Hi Christophe,

Quoted 30 lines · On Mon, 23 Jan 2023 07:53:49 +0000 Christophe Leroy…

Le 20/01/2023 à 10:50, Herve Codina a écrit :

Quoted 29 lines

The Renesas IDT821034 codec is four channel PCM codec with on-chip filters and programmable gain setting.
It also provides SLIC (Subscriber Line Interface Circuit) signals as GPIOs.

---
  sound/soc/codecs/Kconfig     |   12 +
  sound/soc/codecs/Makefile    |    2 +
  sound/soc/codecs/idt821034.c | 1200 ++++++++++++++++++++++++++++++++++
  3 files changed, 1214 insertions(+)

create mode 100644 sound/soc/codecs/idt821034.c

sound/soc/codecs/Kconfig +1 -0
index 0f9d71490075..67489b2ebd9f 100644
@@ -107,6 +107,7 @@ config SND_SOC_ALL_CODECS
  	imply SND_SOC_HDAC_HDMI
  	imply SND_SOC_HDAC_HDA
  	imply SND_SOC_ICS43432
+	imply SND_SOC_IDT821034
  	imply SND_SOC_INNO_RK3036
  	imply SND_SOC_ISABELLE
  	imply SND_SOC_JZ4740_CODEC
@@ -972,6 +973,17 @@ config SND_SOC_HDA
  config SND_SOC_ICS43432
  	tristate "ICS43423 and compatible i2s microphones"
 
Quoted 6 lines

sound/soc/codecs/Kconfig config SND_SOC_HDA · line 979

Quoted 4 lines
sound/soc/codecs/Kconfig +4 -0
+config SND_SOC_IDT821034
+	tristate "Renesas IDT821034 quad PCM codec"
+	depends on SPI
+	select REGMAP_SPI  

Is selecting regmap still necessary ?

Indeed, REGMAP_SPI is not needed anymore.
Will be fixed in v3.

Quoted 113 lines
Quoted 100 lines
sound/soc/codecs/Kconfig +7 -0
+	help
+	  Enable support for the Renesas IDT821034 quad PCM with
+	  programmable gain codec.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called snd-soc-idt821034.
+
  config SND_SOC_INNO_RK3036
  	tristate "Inno codec driver for RK3036 SoC"
  	select REGMAP_MMIO
sound/soc/codecs/Makefile +2 -0
index 71d3ce5867e4..bcf95de654fd 100644
@@ -111,6 +111,7 @@ snd-soc-hdac-hdmi-objs := hdac_hdmi.o
  snd-soc-hdac-hda-objs := hdac_hda.o
  snd-soc-hda-codec-objs := hda.o hda-dai.o
  snd-soc-ics43432-objs := ics43432.o
+snd-soc-idt821034-objs := idt821034.o
  snd-soc-inno-rk3036-objs := inno_rk3036.o
  snd-soc-isabelle-objs := isabelle.o
  snd-soc-jz4740-codec-objs := jz4740.o
@@ -472,6 +473,7 @@ obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
  obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
  obj-$(CONFIG_SND_SOC_HDA) += snd-soc-hda-codec.o
  obj-$(CONFIG_SND_SOC_ICS43432)	+= snd-soc-ics43432.o
+obj-$(CONFIG_SND_SOC_IDT821034)	+= snd-soc-idt821034.o
  obj-$(CONFIG_SND_SOC_INNO_RK3036)	+= snd-soc-inno-rk3036.o
  obj-$(CONFIG_SND_SOC_ISABELLE)	+= snd-soc-isabelle.o
  obj-$(CONFIG_SND_SOC_JZ4740_CODEC)	+= snd-soc-jz4740-codec.o
sound/soc/codecs/idt821034.c new file +63 -0
new file mode 100644
index 000000000000..5eb93fec6042
@@ -0,0 +1,1200 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// IDT821034 ALSA SoC driver
+//
+// Copyright 2022 CS GROUP France
+//
+// Author: Herve Codina <[email protected]>
+
+#include <linux/bitrev.h>
+#include <linux/gpio/driver.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/spi/spi.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#define IDT821034_NB_CHANNEL	4
+
+struct idt821034_amp {
+	u16 gain;
+	bool is_muted;
+};
+
+struct idt821034 {
+	struct spi_device *spi;
+	struct mutex mutex;
+	u8 spi_tx_buf; /* Cannot use stack area for SPI (dma-safe memory) */
+	u8 spi_rx_buf; /* Cannot use stack area for SPI (dma-safe memory) */
+	struct {
+		u8 codec_conf;
+		struct {
+			u8 power;
+			u8 tx_slot;
+			u8 rx_slot;
+			u8 slic_conf;
+			u8 slic_control;
+		} ch[IDT821034_NB_CHANNEL];
+	} cache;
+	struct {
+		struct {
+			struct idt821034_amp amp_out;
+			struct idt821034_amp amp_in;
+		} ch[IDT821034_NB_CHANNEL];
+	} amps;
+	int max_ch_playback;
+	int max_ch_capture;
+	struct gpio_chip gpio_chip;
+};
+
+static int idt821034_8bit_write(struct idt821034 *idt821034, u8 val)
+{
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = &idt821034->spi_tx_buf,
+			.len = 1,
+		}, {
+			.cs_off = 1,
+			.tx_buf = &idt821034->spi_tx_buf,
+			.len = 1,
+		}
+	};
+	int ret;

sound/soc/codecs/idt821034.c line 73

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+
+	idt821034->spi_tx_buf = val;
+
+	dev_vdbg(&idt821034->spi->dev, "spi xfer wr 0x%x\n", val);
+
+	ret = spi_sync_transfer(idt821034->spi, xfer, 2);
+	if (ret)
+		return ret;
+
+	return 0;  

Looks like you could just do:

Quoted 1 line
	return spi_sync_transfer(idt821034->spi, xfer, 2);

Yes, I will change in v3.

Quoted 27 lines
Quoted 27 lines
sound/soc/codecs/idt821034.c new file +26 -0
+}
+
+static int idt821034_8bit_read(struct idt821034 *idt821034, u8 valw, u8 *valr)
+{
+	struct spi_transfer xfer[] = {
+		{
+			.tx_buf = &idt821034->spi_tx_buf,
+			.rx_buf = &idt821034->spi_rx_buf,
+			.len = 1,
+		}, {
+			.cs_off = 1,
+			.tx_buf = &idt821034->spi_tx_buf,
+			.len = 1,
+		}
+	};
+	int ret;
+
+	idt821034->spi_tx_buf = valw;
+
+	ret = spi_sync_transfer(idt821034->spi, xfer, 2);
+	if (ret)
+		return ret;
+
+	*valr = idt821034->spi_rx_buf;
+
+	dev_vdbg(&idt821034->spi->dev, "spi xfer wr 0x%x, rd 0x%x\n",
Quoted 8 lines

sound/soc/codecs/idt821034.c line 105

Quoted 6 lines
sound/soc/codecs/idt821034.c new file +6 -0
+		 valw, *valr);
+
+	return 0;
+}
+
+/* Available mode the programming sequence */  

s/the/for/ ?

Sure, I will change in v3.

Quoted 48 lines
Quoted 32 lines
sound/soc/codecs/idt821034.c new file +31 -0
+#define IDT821034_MODE_CODEC(_ch) (0x80 | ((_ch) << 2))
+#define IDT821034_MODE_SLIC(_ch)  (0xD0 | ((_ch) << 2))
+#define IDT821034_MODE_GAIN(_ch)  (0xC0 | ((_ch) << 2))
+
+/* Power values that can be used in 'power' (can be ORed) */
+#define IDT821034_CONF_PWRUP_TX		BIT(1) /* from analog input to PCM */
+#define IDT821034_CONF_PWRUP_RX		BIT(0) /* from PCM to analog output */
+
+static int idt821034_set_channel_power(struct idt821034 *idt821034, u8 ch, u8 power)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_channel_power(%u, 0x%x)\n", ch, power);
+
+	conf = IDT821034_MODE_CODEC(ch) | idt821034->cache.codec_conf;
+
+	if (power & IDT821034_CONF_PWRUP_RX) {
+		ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_RX);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].rx_slot);
+		if (ret)
+			return ret;
+	}
+	if (power & IDT821034_CONF_PWRUP_TX) {
+		ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_TX);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].tx_slot);
+		if (ret)

sound/soc/codecs/idt821034.c line 146

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+			return ret;
+	}
+	if (!(power & (IDT821034_CONF_PWRUP_TX | IDT821034_CONF_PWRUP_RX))) {
+		ret = idt821034_8bit_write(idt821034, conf);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, 0x00);
+		if (ret)
+			return ret;
+	}  

Can we refactor the three actions with an helper, that could also be reused for idt821034_set_codec_conf() and idt821034_set_channel_ts() and idt821034_set_slic_conf() and idt821034_write_slic_raw() and idt821034_set_gain_channel, something like for instance:

Quoted 7 lines

static int idt821034_set_conf(struct idt821034 *idt821034, u8 conf, u8 val)

{
	ret = idt821034_8bit_write(idt821034, conf);
	if (ret)
		return ret;
	return idt821034_8bit_write(idt821034, val);
}

It can be changed.
The function name will not be idt821034_set_conf() as it is not the same kind of funtion as the idt821031_set_*() already available in the code. What do you think about:
static int idt821034_2x8bit_write(struct idt821034 *idt821034, u8 val1, u8 val2)
or
static int idt821034_conf_write(struct idt821034 *idt821034, u8 conf, u8 val)

I prefer the first one but it is only a personal preference. On your side, what do you prefer ?

Quoted 829 lines
Quoted 816 lines
sound/soc/codecs/idt821034.c new file +815 -0
+
+	idt821034->cache.ch[ch].power = power;
+
+	return 0;
+}
+
+static u8 idt821034_get_channel_power(struct idt821034 *idt821034, u8 ch)
+{
+	return idt821034->cache.ch[ch].power;
+}
+
+/* Codec configuration values that can be used in 'codec_conf' (can be ORed) */
+#define IDT821034_CONF_ALAW_MODE	BIT(5)
+#define IDT821034_CONF_DELAY_MODE	BIT(4)
+
+static int idt821034_set_codec_conf(struct idt821034 *idt821034, u8 codec_conf)
+{
+	u8 conf;
+	u8 ts;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_codec_conf(0x%x)\n", codec_conf);
+
+	/* codec conf fields are common to all channel.
+	 * Arbitrary use of channel 0 for this configuration.
+	 */
+
+	/* Set Configuration Register */
+	conf = IDT821034_MODE_CODEC(0) | codec_conf;
+
+	/* Update conf value and timeslot register value according
+	 * to cache values
+	 */
+	if (idt821034->cache.ch[0].power & IDT821034_CONF_PWRUP_RX) {
+		conf |= IDT821034_CONF_PWRUP_RX;
+		ts = idt821034->cache.ch[0].rx_slot;
+	} else if (idt821034->cache.ch[0].power & IDT821034_CONF_PWRUP_TX) {
+		conf |= IDT821034_CONF_PWRUP_TX;
+		ts = idt821034->cache.ch[0].tx_slot;
+	} else {
+		ts = 0x00;
+	}
+
+	/* Write configuration register and time-slot register */
+	ret = idt821034_8bit_write(idt821034, conf);
+	if (ret)
+		return ret;
+	ret = idt821034_8bit_write(idt821034, ts);
+	if (ret)
+		return ret;
+
+	idt821034->cache.codec_conf = codec_conf;
+	return 0;
+}
+
+static u8 idt821034_get_codec_conf(struct idt821034 *idt821034)
+{
+	return idt821034->cache.codec_conf;
+}
+
+/* Channel direction values that can be used in 'ch_dir' (can be ORed) */
+#define IDT821034_CH_RX		BIT(0) /* from PCM to analog output */
+#define IDT821034_CH_TX		BIT(1) /* from analog input to PCM */
+
+static int idt821034_set_channel_ts(struct idt821034 *idt821034, u8 ch, u8 ch_dir, u8 ts_num)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_channel_ts(%u, 0x%x, %d)\n", ch, ch_dir, ts_num);
+
+	conf = IDT821034_MODE_CODEC(ch) | idt821034->cache.codec_conf;
+
+	if (ch_dir & IDT821034_CH_RX) {
+		if (idt821034->cache.ch[ch].power & IDT821034_CONF_PWRUP_RX) {
+			ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_RX);
+			if (ret)
+				return ret;
+			ret = idt821034_8bit_write(idt821034, ts_num);
+			if (ret)
+				return ret;
+		}
+		idt821034->cache.ch[ch].rx_slot = ts_num;
+	}
+	if (ch_dir & IDT821034_CH_TX) {
+		if (idt821034->cache.ch[ch].power & IDT821034_CONF_PWRUP_TX) {
+			ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_TX);
+			if (ret)
+				return ret;
+			ret = idt821034_8bit_write(idt821034, ts_num);
+			if (ret)
+				return ret;
+		}
+		idt821034->cache.ch[ch].tx_slot = ts_num;
+	}
+
+	return 0;
+}
+
+/* SLIC direction values that can be used in 'slic_dir' (can be ORed) */
+#define IDT821034_SLIC_IO1_IN       BIT(1)
+#define IDT821034_SLIC_IO0_IN       BIT(0)
+
+static int idt821034_set_slic_conf(struct idt821034 *idt821034, u8 ch, u8 slic_dir)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_slic_conf(%u, 0x%x)\n", ch, slic_dir);
+
+	conf = IDT821034_MODE_SLIC(ch) | slic_dir;
+	ret = idt821034_8bit_write(idt821034, conf);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].slic_control);
+	if (ret)
+		return ret;
+
+	idt821034->cache.ch[ch].slic_conf = slic_dir;
+
+	return 0;
+}
+
+static u8 idt821034_get_slic_conf(struct idt821034 *idt821034, u8 ch)
+{
+	return idt821034->cache.ch[ch].slic_conf;
+}
+
+static int idt821034_write_slic_raw(struct idt821034 *idt821034, u8 ch, u8 slic_raw)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "write_slic_raw(%u, 0x%x)\n", ch, slic_raw);
+
+	/*
+	 * On write, slic_raw is mapped as follow :
+	 *   b4: O_4
+	 *   b3: O_3
+	 *   b2: O_2
+	 *   b1: I/O_1
+	 *   b0: I/O_0
+	 */
+
+	conf = IDT821034_MODE_SLIC(ch) | idt821034->cache.ch[ch].slic_conf;
+	ret = idt821034_8bit_write(idt821034, conf);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, slic_raw);
+	if (ret)
+		return ret;
+
+	idt821034->cache.ch[ch].slic_control = slic_raw;
+	return 0;
+}
+
+static u8 idt821034_get_written_slic_raw(struct idt821034 *idt821034, u8 ch)
+{
+	return idt821034->cache.ch[ch].slic_control;
+}
+
+static int idt821034_read_slic_raw(struct idt821034 *idt821034, u8 ch, u8 *slic_raw)
+{
+	u8 val;
+	int ret;
+
+	/*
+	 * On read, slic_raw is mapped as follow :
+	 *   b7: I/O_0
+	 *   b6: I/O_1
+	 *   b5: O_2
+	 *   b4: O_3
+	 *   b3: O_4
+	 *   b2: I/O1_0, I/O_0 from channel 1 (no matter ch value)
+	 *   b1: I/O2_0, I/O_0 from channel 2 (no matter ch value)
+	 *   b2: I/O3_0, I/O_0 from channel 3 (no matter ch value)
+	 */
+
+	val = IDT821034_MODE_SLIC(ch) | idt821034->cache.ch[ch].slic_conf;
+	ret = idt821034_8bit_write(idt821034, val);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_read(idt821034, idt821034->cache.ch[ch].slic_control, slic_raw);
+	if (ret)
+		return ret;
+
+	dev_dbg(&idt821034->spi->dev, "read_slic_raw(%i) 0x%x\n", ch, *slic_raw);
+
+	return 0;
+}
+
+/* Gain type values that can be used in 'gain_type' (cannot be ORed) */
+#define IDT821034_GAIN_RX		(0 << 1) /* from PCM to analog output */
+#define IDT821034_GAIN_TX		(1 << 1) /* from analog input to PCM */
+
+static int idt821034_set_gain_channel(struct idt821034 *idt821034, u8 ch,
+				      u8 gain_type, u16 gain_val)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_gain_channel(%u, 0x%x, 0x%x-%d)\n",
+		ch, gain_type, gain_val, gain_val);
+
+	/*
+	 * The gain programming coefficients should be calculated as:
+	 *   Transmit : Coeff_X = round [ gain_X0dB × gain_X ]
+	 *   Receive: Coeff_R = round [ gain_R0dB × gain_R ]
+	 * where:
+	 *   gain_X0dB = 1820;
+	 *   gain_X is the target gain;
+	 *   Coeff_X should be in the range of 0 to 8192.
+	 *   gain_R0dB = 2506;
+	 *   gain_R is the target gain;
+	 *   Coeff_R should be in the range of 0 to 8192.
+	 *
+	 * A gain programming coefficient is 14-bit wide and in binary format.
+	 * The 7 Most Significant Bits of the coefficient is called
+	 * GA_MSB_Transmit for transmit path, or is called GA_MSB_Receive for
+	 * receive path; The 7 Least Significant Bits of the coefficient is
+	 * called GA_LSB_ Transmit for transmit path, or is called
+	 * GA_LSB_Receive for receive path.
+	 *
+	 * An example is given below to clarify the calculation of the
+	 * coefficient. To program a +3 dB gain in transmit path and a -3.5 dB
+	 * gain in receive path:
+	 *
+	 * Linear Code of +3dB = 10^(3/20)= 1.412537545
+	 * Coeff_X = round (1820 × 1.412537545) = 2571
+	 *                                      = 0b001010_00001011
+	 * GA_MSB_Transmit = 0b0010100
+	 * GA_LSB_Transmit = 0b0001011
+	 *
+	 * Linear Code of -3.5dB = 10^(-3.5/20) = 0.668343917
+	 * Coeff_R= round (2506 × 0.668343917) = 1675
+	 *                                     = 0b0001101_0001011
+	 * GA_MSB_Receive = 0b0001101
+	 * GA_LSB_Receive = 0b0001011
+	 */
+
+	conf = IDT821034_MODE_GAIN(ch) | gain_type;
+
+	ret = idt821034_8bit_write(idt821034, conf | 0x00);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, gain_val & 0x007F);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, conf | 0x01);
+	if (ret)
+		return ret;
+
+	ret = idt821034_8bit_write(idt821034, (gain_val >> 7) & 0x7F);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+/* Id helpers used in controls and dapm */
+#define IDT821034_DIR_OUT (1 << 3)
+#define IDT821034_DIR_IN  (0 << 3)
+#define IDT821034_ID(_ch, _dir) (((_ch) & 0x03) | (_dir))
+#define IDT821034_ID_OUT(_ch) IDT821034_ID(_ch, IDT821034_DIR_OUT)
+#define IDT821034_ID_IN(_ch)  IDT821034_ID(_ch, IDT821034_DIR_IN)
+
+#define IDT821034_ID_GET_CHAN(_id) ((_id) & 0x03)
+#define IDT821034_ID_GET_DIR(_id) ((_id) & (1 << 3))
+#define IDT821034_ID_IS_OUT(_id) (IDT821034_ID_GET_DIR(_id) == IDT821034_DIR_OUT)
+
+static int idt821034_kctrl_gain_get(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int min = mc->min;
+	int max = mc->max;
+	unsigned int mask = (1 << fls(max)) - 1;
+	unsigned int invert = mc->invert;
+	int val;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(mc->reg);
+
+	mutex_lock(&idt821034->mutex);
+	if (IDT821034_ID_IS_OUT(mc->reg))
+		val = idt821034->amps.ch[ch].amp_out.gain;
+	else
+		val = idt821034->amps.ch[ch].amp_in.gain;
+	mutex_unlock(&idt821034->mutex);
+
+	ucontrol->value.integer.value[0] = val & mask;
+	if (invert)
+		ucontrol->value.integer.value[0] = max - ucontrol->value.integer.value[0];
+	else
+		ucontrol->value.integer.value[0] = ucontrol->value.integer.value[0] - min;
+
+	return 0;
+}
+
+static int idt821034_kctrl_gain_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	struct idt821034_amp *amp;
+	int min = mc->min;
+	int max = mc->max;
+	unsigned int mask = (1 << fls(max)) - 1;
+	unsigned int invert = mc->invert;
+	unsigned int val;
+	int ret;
+	u8 gain_type;
+	u8 ch;
+
+	val = ucontrol->value.integer.value[0];
+	if (val < 0)
+		return -EINVAL;
+	if (val > max - min)
+		return -EINVAL;
+
+	if (invert)
+		val = (max - val) & mask;
+	else
+		val = (val + min) & mask;
+
+	ch = IDT821034_ID_GET_CHAN(mc->reg);
+
+	mutex_lock(&idt821034->mutex);
+
+	if (IDT821034_ID_IS_OUT(mc->reg)) {
+		amp = &idt821034->amps.ch[ch].amp_out;
+		gain_type = IDT821034_GAIN_RX;
+	} else {
+		amp = &idt821034->amps.ch[ch].amp_in;
+		gain_type = IDT821034_GAIN_TX;
+	}
+
+	if (!amp->is_muted) {
+		ret = idt821034_set_gain_channel(idt821034, ch, gain_type, val);
+		if (ret)
+			goto end;
+	}
+
+	amp->gain = val;
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_kctrl_mute_get(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int id = kcontrol->private_value;
+	bool is_muted;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(id);
+
+	mutex_lock(&idt821034->mutex);
+	is_muted = IDT821034_ID_IS_OUT(id) ?
+			idt821034->amps.ch[ch].amp_out.is_muted:
+			idt821034->amps.ch[ch].amp_in.is_muted;
+	mutex_unlock(&idt821034->mutex);
+
+	ucontrol->value.integer.value[0] = !is_muted;
+
+	return 0;
+}
+
+static int idt821034_kctrl_mute_put(struct snd_kcontrol *kcontrol,
+				    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_soc_kcontrol_component(kcontrol);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int id = kcontrol->private_value;
+	struct idt821034_amp *amp;
+	bool is_mute;
+	u8 gain_type;
+	int ret;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(id);
+	is_mute = !ucontrol->value.integer.value[0];
+
+	mutex_lock(&idt821034->mutex);
+
+	if (IDT821034_ID_IS_OUT(id)) {
+		amp = &idt821034->amps.ch[ch].amp_out;
+		gain_type = IDT821034_GAIN_RX;
+	} else {
+		amp = &idt821034->amps.ch[ch].amp_in;
+		gain_type = IDT821034_GAIN_TX;
+	}
+
+	ret = idt821034_set_gain_channel(idt821034, ch, gain_type,
+					 is_mute ? 0 : amp->gain);
+	if (ret)
+		goto end;
+
+	amp->is_muted = is_mute;
+
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static const DECLARE_TLV_DB_LINEAR(idt821034_gain_in, -6520, 1306);
+#define IDT821034_GAIN_IN_MIN_RAW	1 /* -65.20 dB -> 10^(-65.2/20.0) * 1820 = 1 */
+#define IDT821034_GAIN_IN_MAX_RAW	8191 /* 13.06 dB -> 10^(13.06/20.0) * 1820 = 8191 */
+#define IDT821034_GAIN_IN_INIT_RAW	1820 /* 0dB -> 10^(0/20) * 1820 = 1820 */
+
+static const DECLARE_TLV_DB_LINEAR(idt821034_gain_out, -6798, 1029);
+#define IDT821034_GAIN_OUT_MIN_RAW	1 /* -67.98 dB -> 10^(-67.98/20.0) * 2506 = 1*/
+#define IDT821034_GAIN_OUT_MAX_RAW	8191 /* 10.29 dB -> 10^(10.29/20.0) * 2506 = 8191 */
+#define IDT821034_GAIN_OUT_INIT_RAW	2506 /* 0dB -> 10^(0/20) * 2506 = 2506 */
+
+static const struct snd_kcontrol_new idt821034_controls[] = {
+	/* DAC volume control */
+	SOC_SINGLE_RANGE_EXT_TLV("DAC0 Playback Volume", IDT821034_ID_OUT(0), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+	SOC_SINGLE_RANGE_EXT_TLV("DAC1 Playback Volume", IDT821034_ID_OUT(1), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+	SOC_SINGLE_RANGE_EXT_TLV("DAC2 Playback Volume", IDT821034_ID_OUT(2), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+	SOC_SINGLE_RANGE_EXT_TLV("DAC3 Playback Volume", IDT821034_ID_OUT(3), 0,
+				 IDT821034_GAIN_OUT_MIN_RAW, IDT821034_GAIN_OUT_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_out),
+
+	/* DAC mute control */
+	SOC_SINGLE_BOOL_EXT("DAC0 Playback Switch", IDT821034_ID_OUT(0),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("DAC1 Playback Switch", IDT821034_ID_OUT(1),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("DAC2 Playback Switch", IDT821034_ID_OUT(2),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("DAC3 Playback Switch", IDT821034_ID_OUT(3),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+
+	/* ADC volume control */
+	SOC_SINGLE_RANGE_EXT_TLV("ADC0 Capture Volume", IDT821034_ID_IN(0), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+	SOC_SINGLE_RANGE_EXT_TLV("ADC1 Capture Volume", IDT821034_ID_IN(1), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+	SOC_SINGLE_RANGE_EXT_TLV("ADC2 Capture Volume", IDT821034_ID_IN(2), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+	SOC_SINGLE_RANGE_EXT_TLV("ADC3 Capture Volume", IDT821034_ID_IN(3), 0,
+				 IDT821034_GAIN_IN_MIN_RAW, IDT821034_GAIN_IN_MAX_RAW,
+				 0, idt821034_kctrl_gain_get, idt821034_kctrl_gain_put,
+				 idt821034_gain_in),
+
+	/* ADC mute control */
+	SOC_SINGLE_BOOL_EXT("ADC0 Capture Switch", IDT821034_ID_IN(0),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("ADC1 Capture Switch", IDT821034_ID_IN(1),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("ADC2 Capture Switch", IDT821034_ID_IN(2),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+	SOC_SINGLE_BOOL_EXT("ADC3 Capture Switch", IDT821034_ID_IN(3),
+			    idt821034_kctrl_mute_get, idt821034_kctrl_mute_put),
+};
+
+static int idt821034_power_event(struct snd_soc_dapm_widget *w,
+				 struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	unsigned int id = w->shift;
+	u8 power, mask;
+	int ret;
+	u8 ch;
+
+	ch = IDT821034_ID_GET_CHAN(id);
+	mask = IDT821034_ID_IS_OUT(id) ? IDT821034_CONF_PWRUP_RX : IDT821034_CONF_PWRUP_TX;
+
+	mutex_lock(&idt821034->mutex);
+
+	power = idt821034_get_channel_power(idt821034, ch);
+	if (SND_SOC_DAPM_EVENT_ON(event))
+		power |= mask;
+	else
+		power &= ~mask;
+	ret = idt821034_set_channel_power(idt821034, ch, power);
+
+	mutex_unlock(&idt821034->mutex);
+
+	return ret;
+}
+
+static const struct snd_soc_dapm_widget idt821034_dapm_widgets[] = {
+	SND_SOC_DAPM_DAC_E("DAC0", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(0), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("DAC1", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(1), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("DAC2", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(2), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("DAC3", "Playback", SND_SOC_NOPM, IDT821034_ID_OUT(3), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+
+	SND_SOC_DAPM_OUTPUT("OUT0"),
+	SND_SOC_DAPM_OUTPUT("OUT1"),
+	SND_SOC_DAPM_OUTPUT("OUT2"),
+	SND_SOC_DAPM_OUTPUT("OUT3"),
+
+	SND_SOC_DAPM_DAC_E("ADC0", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(0), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("ADC1", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(1), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("ADC2", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(2), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_DAC_E("ADC3", "Capture", SND_SOC_NOPM, IDT821034_ID_IN(3), 0,
+			   idt821034_power_event,
+			   SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+
+	SND_SOC_DAPM_INPUT("IN0"),
+	SND_SOC_DAPM_INPUT("IN1"),
+	SND_SOC_DAPM_INPUT("IN2"),
+	SND_SOC_DAPM_INPUT("IN3"),
+};
+
+static const struct snd_soc_dapm_route idt821034_dapm_routes[] = {
+	{ "OUT0", NULL, "DAC0" },
+	{ "OUT1", NULL, "DAC1" },
+	{ "OUT2", NULL, "DAC2" },
+	{ "OUT3", NULL, "DAC3" },
+
+	{ "ADC0", NULL, "IN0" },
+	{ "ADC1", NULL, "IN1" },
+	{ "ADC2", NULL, "IN2" },
+	{ "ADC3", NULL, "IN3" },
+};
+
+static int idt821034_dai_set_tdm_slot(struct snd_soc_dai *dai,
+				      unsigned int tx_mask, unsigned int rx_mask,
+				      int slots, int width)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int mask;
+	u8 slot;
+	int ret;
+	u8 ch;
+
+	switch (width) {
+	case 0: /* Not set -> default 8 */
+	case 8:
+		break;
+	default:
+		dev_err(dai->dev, "tdm slot width %d not supported\n", width);
+		return -EINVAL;
+	}
+
+	mask = tx_mask;
+	slot = 0;
+	ch = 0;
+	while (mask && ch < IDT821034_NB_CHANNEL) {
+		if (mask & 0x1) {
+			mutex_lock(&idt821034->mutex);
+			ret = idt821034_set_channel_ts(idt821034, ch, IDT821034_CH_RX, slot);
+			mutex_unlock(&idt821034->mutex);
+			if (ret) {
+				dev_err(dai->dev, "ch%u set tx tdm slot failed (%d)\n",
+					ch, ret);
+				return ret;
+			}
+			ch++;
+		}
+		mask >>= 1;
+		slot++;
+	}
+	if (mask) {
+		dev_err(dai->dev, "too much tx slots defined (mask = 0x%x) support max %d\n",
+			tx_mask, IDT821034_NB_CHANNEL);
+		return -EINVAL;
+	}
+	idt821034->max_ch_playback = ch;
+
+	mask = rx_mask;
+	slot = 0;
+	ch = 0;
+	while (mask && ch < IDT821034_NB_CHANNEL) {
+		if (mask & 0x1) {
+			mutex_lock(&idt821034->mutex);
+			ret = idt821034_set_channel_ts(idt821034, ch, IDT821034_CH_TX, slot);
+			mutex_unlock(&idt821034->mutex);
+			if (ret) {
+				dev_err(dai->dev, "ch%u set rx tdm slot failed (%d)\n",
+					ch, ret);
+				return ret;
+			}
+			ch++;
+		}
+		mask >>= 1;
+		slot++;
+	}
+	if (mask) {
+		dev_err(dai->dev, "too much rx slots defined (mask = 0x%x) support max %d\n",
+			rx_mask, IDT821034_NB_CHANNEL);
+		return -EINVAL;
+	}
+	idt821034->max_ch_capture = ch;
+
+	return 0;
+}
+
+static int idt821034_dai_set_fmt(struct snd_soc_dai *dai, unsigned int fmt)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	u8 conf;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+
+	conf = idt821034_get_codec_conf(idt821034);
+
+	switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
+	case SND_SOC_DAIFMT_DSP_A:
+		conf |= IDT821034_CONF_DELAY_MODE;
+		break;
+	case SND_SOC_DAIFMT_DSP_B:
+		conf &= ~IDT821034_CONF_DELAY_MODE;
+		break;
+	default:
+		dev_err(dai->dev, "Unsupported DAI format 0x%x\n",
+			fmt & SND_SOC_DAIFMT_FORMAT_MASK);
+		ret = -EINVAL;
+		goto end;
+	}
+	ret = idt821034_set_codec_conf(idt821034, conf);
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_dai_hw_params(struct snd_pcm_substream *substream,
+				   struct snd_pcm_hw_params *params,
+				   struct snd_soc_dai *dai)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	u8 conf;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+
+	conf = idt821034_get_codec_conf(idt821034);
+
+	switch (params_format(params)) {
+	case SNDRV_PCM_FORMAT_A_LAW:
+		conf |= IDT821034_CONF_ALAW_MODE;
+		break;
+	case SNDRV_PCM_FORMAT_MU_LAW:
+		conf &= ~IDT821034_CONF_ALAW_MODE;
+		break;
+	default:
+		dev_err(dai->dev, "Unsupported PCM format 0x%x\n",
+			params_format(params));
+		ret = -EINVAL;
+		goto end;
+	}
+	ret = idt821034_set_codec_conf(idt821034, conf);
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static const unsigned int idt821034_sample_bits[] = {8};
+
+static struct snd_pcm_hw_constraint_list idt821034_sample_bits_constr = {
+	.list = idt821034_sample_bits,
+	.count = ARRAY_SIZE(idt821034_sample_bits),
+};
+
+static int idt821034_dai_startup(struct snd_pcm_substream *substream,
+				 struct snd_soc_dai *dai)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(dai->component);
+	unsigned int max_ch = 0;
+	int ret;
+
+	max_ch = (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ?
+		idt821034->max_ch_playback : idt821034->max_ch_capture;
+
+	/*
+	 * Disable stream support (min = 0, max = 0) if no timeslots were
+	 * configured otherwise, limit the number of channels to those
+	 * configured.
+	 */
+	ret = snd_pcm_hw_constraint_minmax(substream->runtime, SNDRV_PCM_HW_PARAM_CHANNELS,
+					   max_ch ? 1 : 0, max_ch);
+	if (ret < 0)
+		return ret;
+
+	ret = snd_pcm_hw_constraint_list(substream->runtime, 0, SNDRV_PCM_HW_PARAM_SAMPLE_BITS,
+					 &idt821034_sample_bits_constr);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static u64 idt821034_dai_formats[] = {
+	SND_SOC_POSSIBLE_DAIFMT_DSP_A	|
+	SND_SOC_POSSIBLE_DAIFMT_DSP_B,
+};
+
+static const struct snd_soc_dai_ops idt821034_dai_ops = {
+	.startup      = idt821034_dai_startup,
+	.hw_params    = idt821034_dai_hw_params,
+	.set_tdm_slot = idt821034_dai_set_tdm_slot,
+	.set_fmt      = idt821034_dai_set_fmt,
+	.auto_selectable_formats     = idt821034_dai_formats,
+	.num_auto_selectable_formats = ARRAY_SIZE(idt821034_dai_formats),
+};
+
+static struct snd_soc_dai_driver idt821034_dai_driver = {
+	.name = "idt821034",
+	.playback = {
+		.stream_name = "Playback",
+		.channels_min = 1,
+		.channels_max = IDT821034_NB_CHANNEL,
+		.rates = SNDRV_PCM_RATE_8000,
+		.formats = SNDRV_PCM_FMTBIT_MU_LAW | SNDRV_PCM_FMTBIT_A_LAW,
+	},
+	.capture = {
+		.stream_name = "Capture",
+		.channels_min = 1,
+		.channels_max = IDT821034_NB_CHANNEL,
+		.rates = SNDRV_PCM_RATE_8000,
+		.formats = SNDRV_PCM_FMTBIT_MU_LAW | SNDRV_PCM_FMTBIT_A_LAW,
+	},
+	.ops = &idt821034_dai_ops,
+};
+
+static int idt821034_reset_audio(struct idt821034 *idt821034)
+{
+	int ret;
+	u8 i;
+
+	mutex_lock(&idt821034->mutex);
+
+	ret = idt821034_set_codec_conf(idt821034, 0);
+	if (ret)
+		goto end;
+
+	for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
+		idt821034->amps.ch[i].amp_out.gain = IDT821034_GAIN_OUT_INIT_RAW;
+		idt821034->amps.ch[i].amp_out.is_muted = false;
+		ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_RX,
+						 idt821034->amps.ch[i].amp_out.gain);
+		if (ret)
+			goto end;
+
+		idt821034->amps.ch[i].amp_in.gain = IDT821034_GAIN_IN_INIT_RAW;
+		idt821034->amps.ch[i].amp_in.is_muted = false;
+		ret = idt821034_set_gain_channel(idt821034, i, IDT821034_GAIN_TX,
+						 idt821034->amps.ch[i].amp_in.gain);
+		if (ret)
+			goto end;
+
+		ret = idt821034_set_channel_power(idt821034, i, 0);
+		if (ret)
+			goto end;
+	}
+
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_component_probe(struct snd_soc_component *component)
+{
+	struct idt821034 *idt821034 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	/* reset idt821034 audio part*/
+	ret = idt821034_reset_audio(idt821034);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver idt821034_component_driver = {
+	.probe			= idt821034_component_probe,

sound/soc/codecs/idt821034.c line 971

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+	.controls		= idt821034_controls,
+	.num_controls		= ARRAY_SIZE(idt821034_controls),
+	.dapm_widgets		= idt821034_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(idt821034_dapm_widgets),
+	.dapm_routes		= idt821034_dapm_routes,
+	.num_dapm_routes	= ARRAY_SIZE(idt821034_dapm_routes),
+	.endianness		= 1,
+};
+
+#if IS_ENABLED(CONFIG_GPIOLIB)  

#ifdef CONFIG_GPIOLIB ?

Quoted 7 lines

Is that #if section needed at all ? Isn't it possible to have it all the time and check IS_ENABLED(CONFIG_GPIOLIB) in the probe before calling the init ? Something like :

         if (IS_ENABLED(CONFIG_GPIOLIB))
		return idt821034_gpio_init(idt821034);
	return 0;

The following functions:
- idt821034_chip_gpio_set(),
- idt821034_chip_gpio_get(),
- idt821034_chip_get_direction(),
- idt821034_chip_direction_input(),
- idt821034_chip_direction_output()
use gpiochip_get_data() to retrieve private data.

gpiochip_get_data() is defined only when CONFIG_GPIOLIB is set. That's why the #if section is used.

Quoted 145 lines
Quoted 145 lines
sound/soc/codecs/idt821034.c new file +144 -0
+#define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) / 5) % 4)
+#define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset)    BIT((_offset) % 5)
+
+static void idt821034_chip_gpio_set(struct gpio_chip *c, unsigned int offset, int val)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_raw;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+
+	slic_raw = idt821034_get_written_slic_raw(idt821034, ch);
+	if (val)
+		slic_raw |= mask;
+	else
+		slic_raw &= ~mask;
+	ret = idt821034_write_slic_raw(idt821034, ch, slic_raw);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "set gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+	}
+
+	mutex_unlock(&idt821034->mutex);
+}
+
+static int idt821034_chip_gpio_get(struct gpio_chip *c, unsigned int offset)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_raw;
+	int ret;
+
+	mutex_lock(&idt821034->mutex);
+	ret = idt821034_read_slic_raw(idt821034, ch, &slic_raw);
+	mutex_unlock(&idt821034->mutex);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "get gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+		return ret;
+	}
+
+	/*
+	 * SLIC IOs are read in reverse order compared to write.
+	 * Reverse the read value here in order to have IO0 at lsb (ie same
+	 * order as write)
+	 */
+	return !!(bitrev8(slic_raw) & mask);
+}
+
+static int idt821034_chip_get_direction(struct gpio_chip *c, unsigned int offset)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_dir;
+
+	mutex_lock(&idt821034->mutex);
+	slic_dir = idt821034_get_slic_conf(idt821034, ch);
+	mutex_unlock(&idt821034->mutex);
+
+	return slic_dir & mask ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;
+}
+
+static int idt821034_chip_direction_input(struct gpio_chip *c, unsigned int offset)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_conf;
+	int ret;
+
+	/* Only IO0 and IO1 can be set as input */
+	if (mask & ~(IDT821034_SLIC_IO1_IN | IDT821034_SLIC_IO0_IN))
+		return -EPERM;
+
+	mutex_lock(&idt821034->mutex);
+
+	slic_conf = idt821034_get_slic_conf(idt821034, ch) | mask;
+
+	ret = idt821034_set_slic_conf(idt821034, ch, slic_conf);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "dir in gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+	}
+
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_chip_direction_output(struct gpio_chip *c, unsigned int offset, int val)
+{
+	u8 ch = IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(offset);
+	u8 mask = IDT821034_GPIO_OFFSET_TO_SLIC_MASK(offset);
+	struct idt821034 *idt821034 = gpiochip_get_data(c);
+	u8 slic_conf;
+	int ret;
+
+	idt821034_chip_gpio_set(c, offset, val);
+
+	mutex_lock(&idt821034->mutex);
+
+	slic_conf = idt821034_get_slic_conf(idt821034, ch) & ~mask;
+
+	ret = idt821034_set_slic_conf(idt821034, ch, slic_conf);
+	if (ret) {
+		dev_err(&idt821034->spi->dev, "dir in gpio %d (%u, 0x%x) failed (%d)\n",
+			offset, ch, mask, ret);
+	}
+
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_reset_gpio(struct idt821034 *idt821034)
+{
+	int ret;
+	u8 i;
+
+	mutex_lock(&idt821034->mutex);
+
+	/* IO0 and IO1 as input for all channels and output IO set to 0 */
+	for (i = 0; i < IDT821034_NB_CHANNEL; i++) {
+		ret = idt821034_set_slic_conf(idt821034, i,
+					      IDT821034_SLIC_IO1_IN | IDT821034_SLIC_IO0_IN);
+		if (ret)
+			goto end;
+
+		ret = idt821034_write_slic_raw(idt821034, i, 0);
+		if (ret)
+			goto end;
+
+	}
+	ret = 0;
+end:
+	mutex_unlock(&idt821034->mutex);
+	return ret;
+}
+
+static int idt821034_gpio_init(struct idt821034 *idt821034)
+{
+	int ret;
Quoted 8 lines

sound/soc/codecs/idt821034.c line 1121

Quoted 6 lines
sound/soc/codecs/idt821034.c new file +6 -0
+
+	ret = idt821034_reset_gpio(idt821034);
+	if (ret)
+		return ret;
+
+	idt821034->gpio_chip.owner = THIS_MODULE;  

Is the owner really needed ?

I think it is:
https://elixir.bootlin.com/linux/latest/source/include/linux/gpio/driver.h#L319

and look at drivers/gpio/, a lot of drivers (all ?) set the owner value.

Quoted 72 lines
Quoted 72 lines
sound/soc/codecs/idt821034.c new file +71 -0
+	idt821034->gpio_chip.label = dev_name(&idt821034->spi->dev);
+	idt821034->gpio_chip.parent = &idt821034->spi->dev;
+	idt821034->gpio_chip.base = -1;
+	idt821034->gpio_chip.ngpio = 5 * 4; /* 5 GPIOs on 4 channels */
+	idt821034->gpio_chip.get_direction = idt821034_chip_get_direction;
+	idt821034->gpio_chip.direction_input = idt821034_chip_direction_input;
+	idt821034->gpio_chip.direction_output = idt821034_chip_direction_output;
+	idt821034->gpio_chip.get = idt821034_chip_gpio_get;
+	idt821034->gpio_chip.set = idt821034_chip_gpio_set;
+	idt821034->gpio_chip.can_sleep = true;
+
+	return devm_gpiochip_add_data(&idt821034->spi->dev, &idt821034->gpio_chip,
+				      idt821034);
+}
+#else /* IS_ENABLED(CONFIG_GPIOLIB) */
+static int idt821034_gpio_init(struct idt821034 *idt821034)
+{
+	return 0;
+}
+#endif
+
+static int idt821034_spi_probe(struct spi_device *spi)
+{
+	struct idt821034 *idt821034;
+	int ret;
+
+	spi->bits_per_word = 8;
+	ret = spi_setup(spi);
+	if (ret < 0)
+		return ret;
+
+	idt821034 = devm_kzalloc(&spi->dev, sizeof(*idt821034), GFP_KERNEL);
+	if (!idt821034)
+		return -ENOMEM;
+
+	idt821034->spi = spi;
+
+	mutex_init(&idt821034->mutex);
+
+	spi_set_drvdata(spi, idt821034);
+
+	ret = devm_snd_soc_register_component(&spi->dev, &idt821034_component_driver,
+					      &idt821034_dai_driver, 1);
+	if (ret)
+		return ret;
+
+	ret = idt821034_gpio_init(idt821034);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id idt821034_of_match[] = {
+	{ .compatible = "renesas,idt821034", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, idt821034_of_match);
+
+static const struct spi_device_id idt821034_id_table[] = {
+	{ "idt821034", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, idt821034_id_table);
+
+static struct spi_driver idt821034_spi_driver = {
+	.driver  = {
+		.name   = "idt821034",
+		.of_match_table = idt821034_of_match,
+	},
+	.id_table = idt821034_id_table,
Quoted 8 lines

sound/soc/codecs/idt821034.c line 1200

Quoted 8 lines
sound/soc/codecs/idt821034.c new file +8 -0
+	.probe  = idt821034_spi_probe,
+};
+
+module_spi_driver(idt821034_spi_driver);
+
+MODULE_AUTHOR("Herve Codina <[email protected]>");
+MODULE_DESCRIPTION("IDT821034 ALSA SoC driver");
+MODULE_LICENSE("GPL");  

Thanks for the review,

Hervé

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#8 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Le 23/01/2023 à 09:56, Herve Codina a écrit :

Quoted 52 lines
Quoted 40 lines
Quoted 24 lines
sound/soc/codecs/idt821034.c new file +24 -0
+
+static int idt821034_set_channel_power(struct idt821034 *idt821034, u8 ch, u8 power)
+{
+	u8 conf;
+	int ret;
+
+	dev_dbg(&idt821034->spi->dev, "set_channel_power(%u, 0x%x)\n", ch, power);
+
+	conf = IDT821034_MODE_CODEC(ch) | idt821034->cache.codec_conf;
+
+	if (power & IDT821034_CONF_PWRUP_RX) {
+		ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_RX);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].rx_slot);
+		if (ret)
+			return ret;
+	}
+	if (power & IDT821034_CONF_PWRUP_TX) {
+		ret = idt821034_8bit_write(idt821034, conf | IDT821034_CONF_PWRUP_TX);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, idt821034->cache.ch[ch].tx_slot);
+		if (ret)

sound/soc/codecs/idt821034.c line 146

Quoted 10 lines
sound/soc/codecs/idt821034.c new file +10 -0
+			return ret;
+	}
+	if (!(power & (IDT821034_CONF_PWRUP_TX | IDT821034_CONF_PWRUP_RX))) {
+		ret = idt821034_8bit_write(idt821034, conf);
+		if (ret)
+			return ret;
+		ret = idt821034_8bit_write(idt821034, 0x00);
+		if (ret)
+			return ret;
+	}

Can we refactor the three actions with an helper, that could also be reused for idt821034_set_codec_conf() and idt821034_set_channel_ts() and idt821034_set_slic_conf() and idt821034_write_slic_raw() and idt821034_set_gain_channel, something like for instance:

Quoted 7 lines

static int idt821034_set_conf(struct idt821034 *idt821034, u8 conf, u8 val)

{
	ret = idt821034_8bit_write(idt821034, conf);
	if (ret)
		return ret;
	return idt821034_8bit_write(idt821034, val);
}

It can be changed.
The function name will not be idt821034_set_conf() as it is not the same kind of funtion as the idt821031_set_*() already available in the code. What do you think about:

Quoted 6 lines
   static int idt821034_2x8bit_write(struct idt821034 *idt821034, u8 val1, u8 val2)

or
static int idt821034_conf_write(struct idt821034 *idt821034, u8 conf, u8 val)

I prefer the first one but it is only a personal preference. On your side, what do you prefer ?

idt821034_2x8bit_write() looks good to me.

Christophe

#9 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

In reply to Herve Codina

Hi Hervé,

Le 23/01/2023 à 09:56, Herve Codina a écrit :

Quoted 3 lines

gpiochip_get_data() is defined only when CONFIG_GPIOLIB is set. That's why the #if section is used.

gpiochip_get_data() is still declared when CONFIG_GPIOLIB is not set, so it is not a problem, the call to it will be eliminated at buildtime.

By the way, at the time being I get the following warnings:

CC sound/soc/codecs/idt821034.o
sound/soc/codecs/idt821034.c:310:12: warning: 'idt821034_read_slic_raw' defined but not used [-Wunused-function]
310 | static int idt821034_read_slic_raw(struct idt821034 *idt821034,
u8 ch, u8 *slic_raw)

       |            ^~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:305:11: warning:
'idt821034_get_written_slic_raw' defined but not used [-Wunused-function]
305 | static u8 idt821034_get_written_slic_raw(struct idt821034
*idt821034, u8 ch)

       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:276:12: warning: 'idt821034_write_slic_raw' defined but not used [-Wunused-function]
276 | static int idt821034_write_slic_raw(struct idt821034
*idt821034, u8 ch, u8 slic_raw)

       |            ^~~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:271:11: warning: 'idt821034_get_slic_conf' defined but not used [-Wunused-function]
271 | static u8 idt821034_get_slic_conf(struct idt821034 *idt821034,
u8 ch)

       |           ^~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:250:12: warning: 'idt821034_set_slic_conf' defined but not used [-Wunused-function]
250 | static int idt821034_set_slic_conf(struct idt821034 *idt821034,
u8 ch, u8 slic_dir)

       |            ^~~~~~~~~~~~~~~~~~~~~~~

With the following changes I have no warning and an objdump -x on idt821034.o shows no reference to gpiochip_get_data()

sound/soc/codecs/idt821034.c +0 -0
index 5eb93fec6042..8b75388e22ce 100644
@@ -968,7 +968,6 @@ static const struct snd_soc_component_driver 

idt821034_component_driver = {
.endianness = 1,
};

-#if IS_ENABLED(CONFIG_GPIOLIB)
#define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) /
5) % 4)
#define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset) BIT((_offset) % 5)

@@ -1133,12 +1132,6 @@ static int idt821034_gpio_init(struct idt821034 *idt821034)
return devm_gpiochip_add_data(&idt821034->spi->dev,
&idt821034->gpio_chip,
idt821034);
}
-#else /* IS_ENABLED(CONFIG_GPIOLIB) */
-static int idt821034_gpio_init(struct idt821034 *idt821034) -{
- return 0;
-}
-#endif

static int idt821034_spi_probe(struct spi_device *spi)
{
@@ -1165,6 +1158,9 @@ static int idt821034_spi_probe(struct spi_device *spi)
if (ret)
return ret;

+ if (!IS_ENABLED(CONFIG_GPIOLIB))
+ return 0;
+
ret = idt821034_gpio_init(idt821034);
if (ret)
return ret;

Christophe

author maintainer
#10 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Hi Christophe,

Quoted 84 lines · On Mon, 23 Jan 2023 11:13:23 +0000 Christophe Leroy…

Hi Hervé,

Le 23/01/2023 à 09:56, Herve Codina a écrit :

Quoted 3 lines

gpiochip_get_data() is defined only when CONFIG_GPIOLIB is set. That's why the #if section is used.

gpiochip_get_data() is still declared when CONFIG_GPIOLIB is not set, so it is not a problem, the call to it will be eliminated at buildtime.

By the way, at the time being I get the following warnings:

   CC      sound/soc/codecs/idt821034.o

sound/soc/codecs/idt821034.c:310:12: warning: 'idt821034_read_slic_raw' defined but not used [-Wunused-function]
310 | static int idt821034_read_slic_raw(struct idt821034 *idt821034,
u8 ch, u8 *slic_raw)

       |            ^~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:305:11: warning:
'idt821034_get_written_slic_raw' defined but not used [-Wunused-function]
305 | static u8 idt821034_get_written_slic_raw(struct idt821034
*idt821034, u8 ch)

       |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:276:12: warning: 'idt821034_write_slic_raw' defined but not used [-Wunused-function]
276 | static int idt821034_write_slic_raw(struct idt821034
*idt821034, u8 ch, u8 slic_raw)

       |            ^~~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:271:11: warning: 'idt821034_get_slic_conf' defined but not used [-Wunused-function]
271 | static u8 idt821034_get_slic_conf(struct idt821034 *idt821034,
u8 ch)

       |           ^~~~~~~~~~~~~~~~~~~~~~~

sound/soc/codecs/idt821034.c:250:12: warning: 'idt821034_set_slic_conf' defined but not used [-Wunused-function]
250 | static int idt821034_set_slic_conf(struct idt821034 *idt821034,
u8 ch, u8 slic_dir)

       |            ^~~~~~~~~~~~~~~~~~~~~~~

With the following changes I have no warning and an objdump -x on idt821034.o shows no reference to gpiochip_get_data()

sound/soc/codecs/idt821034.c +0 -1
index 5eb93fec6042..8b75388e22ce 100644
@@ -968,7 +968,6 @@ static const struct snd_soc_component_driver 
 idt821034_component_driver = {
  	.endianness		= 1,
  };
 
-#if IS_ENABLED(CONFIG_GPIOLIB)
  #define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) / 
 5) % 4)
  #define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset)    BIT((_offset) % 5)
sound/soc/codecs/idt821034.c +0 -6
@@ -1133,12 +1132,6 @@ static int idt821034_gpio_init(struct idt821034 
 *idt821034)
  	return devm_gpiochip_add_data(&idt821034->spi->dev, 
 &idt821034->gpio_chip,
  				      idt821034);
  }
-#else /* IS_ENABLED(CONFIG_GPIOLIB) */
-static int idt821034_gpio_init(struct idt821034 *idt821034)
-{
-	return 0;
-}
-#endif
 
  static int idt821034_spi_probe(struct spi_device *spi)
  {
sound/soc/codecs/idt821034.c +3 -0
@@ -1165,6 +1158,9 @@ static int idt821034_spi_probe(struct spi_device *spi)
  	if (ret)
  		return ret;
 
+	if (!IS_ENABLED(CONFIG_GPIOLIB))
+		return 0;
+
  	ret = idt821034_gpio_init(idt821034);
  	if (ret)
  		return ret;
Quoted 1 line

Christophe

Right, I did the test too and indeed, I can remove the #if section.

I will use (I think is clearer) at idt821034_spi_probe():

	if (!IS_ENABLED(CONFIG_GPIOLIB)) {
   		ret = idt821034_gpio_init(idt821034);
		if (ret)
   			return ret;
	}

Is that ok for you ?

Thanks,
Hervé

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#11 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Le 23/01/2023 à 13:17, Herve Codina a écrit :

Quoted 93 lines

Hi Christophe,

Quoted 84 lines · On Mon, 23 Jan 2023 11:13:23 +0000 Christophe Leroy…

Hi Hervé,

Le 23/01/2023 à 09:56, Herve Codina a écrit :

Quoted 3 lines

gpiochip_get_data() is defined only when CONFIG_GPIOLIB is set. That's why the #if section is used.

gpiochip_get_data() is still declared when CONFIG_GPIOLIB is not set, so it is not a problem, the call to it will be eliminated at buildtime.

By the way, at the time being I get the following warnings:

    CC      sound/soc/codecs/idt821034.o
sound/soc/codecs/idt821034.c:310:12: warning: 'idt821034_read_slic_raw'
defined but not used [-Wunused-function]
    310 | static int idt821034_read_slic_raw(struct idt821034 *idt821034,
u8 ch, u8 *slic_raw)
        |            ^~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:305:11: warning:
'idt821034_get_written_slic_raw' defined but not used [-Wunused-function]
    305 | static u8 idt821034_get_written_slic_raw(struct idt821034
*idt821034, u8 ch)
        |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:276:12: warning: 'idt821034_write_slic_raw'
defined but not used [-Wunused-function]
    276 | static int idt821034_write_slic_raw(struct idt821034
*idt821034, u8 ch, u8 slic_raw)
        |            ^~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:271:11: warning: 'idt821034_get_slic_conf'
defined but not used [-Wunused-function]
    271 | static u8 idt821034_get_slic_conf(struct idt821034 *idt821034,
u8 ch)
        |           ^~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:250:12: warning: 'idt821034_set_slic_conf'
defined but not used [-Wunused-function]
    250 | static int idt821034_set_slic_conf(struct idt821034 *idt821034,
u8 ch, u8 slic_dir)
        |            ^~~~~~~~~~~~~~~~~~~~~~~

With the following changes I have no warning and an objdump -x on idt821034.o shows no reference to gpiochip_get_data()

sound/soc/codecs/idt821034.c +0 -1
index 5eb93fec6042..8b75388e22ce 100644
@@ -968,7 +968,6 @@ static const struct snd_soc_component_driver
 idt821034_component_driver = {
   	.endianness		= 1,
   };
 
-#if IS_ENABLED(CONFIG_GPIOLIB)
   #define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) /
 5) % 4)
   #define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset)    BIT((_offset) % 5)
sound/soc/codecs/idt821034.c +0 -6
@@ -1133,12 +1132,6 @@ static int idt821034_gpio_init(struct idt821034
 *idt821034)
   	return devm_gpiochip_add_data(&idt821034->spi->dev,
 &idt821034->gpio_chip,
   				      idt821034);
   }
-#else /* IS_ENABLED(CONFIG_GPIOLIB) */
-static int idt821034_gpio_init(struct idt821034 *idt821034)
-{
-	return 0;
-}
-#endif
 
   static int idt821034_spi_probe(struct spi_device *spi)
   {
sound/soc/codecs/idt821034.c +3 -0
@@ -1165,6 +1158,9 @@ static int idt821034_spi_probe(struct spi_device *spi)
   	if (ret)
   		return ret;
 
+	if (!IS_ENABLED(CONFIG_GPIOLIB))
+		return 0;
+
   	ret = idt821034_gpio_init(idt821034);
   	if (ret)
   		return ret;
Quoted 1 line

Christophe

Right, I did the test too and indeed, I can remove the #if section.

Quoted 7 lines

I will use (I think is clearer) at idt821034_spi_probe():

	if (!IS_ENABLED(CONFIG_GPIOLIB)) {
    		ret = idt821034_gpio_init(idt821034);
		if (ret)
    			return ret;
	}

I guess you mean :

if (IS_ENABLED(CONFIG_GPIOLIB))

Quoted 1 line

Is that ok for you ?

What about:

	if (IS_ENABLED(CONFIG_GPIOLIB))
		return idt821034_gpio_init(idt821034);
	else
		return 0;

Christophe

author maintainer
#12 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Quoted 103 lines · On Mon, 23 Jan 2023 12:30:32 +0000 Christophe Leroy…

Le 23/01/2023 à 13:17, Herve Codina a écrit :

Quoted 93 lines

Hi Christophe,

Quoted 84 lines · On Mon, 23 Jan 2023 11:13:23 +0000 Christophe Leroy…

Hi Hervé,

Le 23/01/2023 à 09:56, Herve Codina a écrit :

Quoted 3 lines

gpiochip_get_data() is defined only when CONFIG_GPIOLIB is set. That's why the #if section is used.

gpiochip_get_data() is still declared when CONFIG_GPIOLIB is not set, so it is not a problem, the call to it will be eliminated at buildtime.

By the way, at the time being I get the following warnings:

    CC      sound/soc/codecs/idt821034.o
sound/soc/codecs/idt821034.c:310:12: warning: 'idt821034_read_slic_raw'
defined but not used [-Wunused-function]
    310 | static int idt821034_read_slic_raw(struct idt821034 *idt821034,
u8 ch, u8 *slic_raw)
        |            ^~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:305:11: warning:
'idt821034_get_written_slic_raw' defined but not used [-Wunused-function]
    305 | static u8 idt821034_get_written_slic_raw(struct idt821034
*idt821034, u8 ch)
        |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:276:12: warning: 'idt821034_write_slic_raw'
defined but not used [-Wunused-function]
    276 | static int idt821034_write_slic_raw(struct idt821034
*idt821034, u8 ch, u8 slic_raw)
        |            ^~~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:271:11: warning: 'idt821034_get_slic_conf'
defined but not used [-Wunused-function]
    271 | static u8 idt821034_get_slic_conf(struct idt821034 *idt821034,
u8 ch)
        |           ^~~~~~~~~~~~~~~~~~~~~~~
sound/soc/codecs/idt821034.c:250:12: warning: 'idt821034_set_slic_conf'
defined but not used [-Wunused-function]
    250 | static int idt821034_set_slic_conf(struct idt821034 *idt821034,
u8 ch, u8 slic_dir)
        |            ^~~~~~~~~~~~~~~~~~~~~~~

With the following changes I have no warning and an objdump -x on idt821034.o shows no reference to gpiochip_get_data()

sound/soc/codecs/idt821034.c +0 -1
index 5eb93fec6042..8b75388e22ce 100644
@@ -968,7 +968,6 @@ static const struct snd_soc_component_driver
 idt821034_component_driver = {
   	.endianness		= 1,
   };
 
-#if IS_ENABLED(CONFIG_GPIOLIB)
   #define IDT821034_GPIO_OFFSET_TO_SLIC_CHANNEL(_offset) (((_offset) /
 5) % 4)
   #define IDT821034_GPIO_OFFSET_TO_SLIC_MASK(_offset)    BIT((_offset) % 5)
sound/soc/codecs/idt821034.c +0 -6
@@ -1133,12 +1132,6 @@ static int idt821034_gpio_init(struct idt821034
 *idt821034)
   	return devm_gpiochip_add_data(&idt821034->spi->dev,
 &idt821034->gpio_chip,
   				      idt821034);
   }
-#else /* IS_ENABLED(CONFIG_GPIOLIB) */
-static int idt821034_gpio_init(struct idt821034 *idt821034)
-{
-	return 0;
-}
-#endif
 
   static int idt821034_spi_probe(struct spi_device *spi)
   {
sound/soc/codecs/idt821034.c +3 -0
@@ -1165,6 +1158,9 @@ static int idt821034_spi_probe(struct spi_device *spi)
   	if (ret)
   		return ret;
 
+	if (!IS_ENABLED(CONFIG_GPIOLIB))
+		return 0;
+
   	ret = idt821034_gpio_init(idt821034);
   	if (ret)
   		return ret;
Quoted 1 line

Christophe

Right, I did the test too and indeed, I can remove the #if section.

Quoted 7 lines

I will use (I think is clearer) at idt821034_spi_probe():

	if (!IS_ENABLED(CONFIG_GPIOLIB)) {
    		ret = idt821034_gpio_init(idt821034);
		if (ret)
    			return ret;
	}
Quoted 3 lines

I guess you mean :

if (IS_ENABLED(CONFIG_GPIOLIB))

Yes of course. Sorry for the typo.

Quoted 6 lines
Quoted 3 lines

Is that ok for you ?

Quoted 8 lines

What about:

	if (IS_ENABLED(CONFIG_GPIOLIB))
		return idt821034_gpio_init(idt821034);
	else
		return 0;

Christophe

Well, maybe this version ?

static int idt821034_spi_probe(struct spi_device *spi)

{
	...

	if (IS_ENABLED(CONFIG_GPIOLIB))

return idt821034_gpio_init(idt821034);

	return 0;
}

Thanks,
Hervé

Signature · 3 lines
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
#13 lore raw

Re: [PATCH v2 2/3] ASoC: codecs: Add support for the Renesas IDT821034 codec

Quoted 4 lines

Well, maybe this version ?

static int idt821034_spi_probe(struct spi_device *spi)

Quoted 9 lines
{
	...

	if (IS_ENABLED(CONFIG_GPIOLIB))

return idt821034_gpio_init(idt821034);

	return 0;
}

Yes that's fine for me.

Christophe

author maintainer
#14 lore raw

PATCH v2 3/3 MAINTAINERS: add the Renesas IDT821034 codec entry

In reply to Herve Codina

After contributing the driver, add myself as the maintainer for the Renesas IDT821034 codec.

---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)
MAINTAINERS +7 -0
index 9dcfadec5aa3..31115a7e01c1 100644
@@ -17809,6 +17809,13 @@ F:	Documentation/devicetree/bindings/net/renesas,*.yaml
 F:	drivers/net/ethernet/renesas/
 F:	include/linux/sh_eth.h
 
+RENESAS IDT821034 ASoC CODEC
+M:	Herve Codina <[email protected]>
+L:	[email protected] (moderated for non-subscribers)
+S:	Maintained
+F:	Documentation/devicetree/bindings/sound/renesas,idt821034.yaml
+F:	sound/soc/codecs/idt821034.c
+
 RENESAS R-CAR GYROADC DRIVER
 M:	Marek Vasut <[email protected]>
 L:	[email protected]
Signature · 1 line
2.39.0