2023-01-20 09:55:38

by Herve Codina

[permalink] [raw]
Subject: [PATCH v2 0/3] 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

--
2.39.0


2023-01-20 09:56:22

by Herve Codina

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

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

Signed-off-by: Herve Codina <[email protected]>
---
.../bindings/sound/renesas,idt821034.yaml | 75 +++++++++++++++++++
1 file changed, 75 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/renesas,idt821034.yaml

diff --git a/Documentation/devicetree/bindings/sound/renesas,idt821034.yaml b/Documentation/devicetree/bindings/sound/renesas,idt821034.yaml
new file mode 100644
index 000000000000..10677d9d8f67
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/renesas,idt821034.yaml
@@ -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>;
+ };
+ };
--
2.39.0

2023-01-20 09:56:27

by Herve Codina

[permalink] [raw]
Subject: [PATCH v2 3/3] MAINTAINERS: add the Renesas IDT821034 codec entry

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

Signed-off-by: Herve Codina <[email protected]>
---
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 9dcfadec5aa3..31115a7e01c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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]
--
2.39.0

2023-01-20 10:13:43

by Herve Codina

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

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.

Signed-off-by: Herve Codina <[email protected]>
---
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

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 0f9d71490075..67489b2ebd9f 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -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
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 71d3ce5867e4..bcf95de654fd 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -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
diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
new file mode 100644
index 000000000000..5eb93fec6042
--- /dev/null
+++ b/sound/soc/codecs/idt821034.c
@@ -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;
+
+ 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;
+}
+
+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",
+ valw, *valr);
+
+ return 0;
+}
+
+/* Available mode the programming sequence */
+#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)
+ 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;
+ }
+
+ 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,
+ .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)
+#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;
+
+ ret = idt821034_reset_gpio(idt821034);
+ if (ret)
+ return ret;
+
+ idt821034->gpio_chip.owner = THIS_MODULE;
+ 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");
--
2.39.0

2023-01-20 12:24:02

by Mark Brown

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

On Fri, Jan 20, 2023 at 10:50:35AM +0100, Herve Codina wrote:

> +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;

> +
> + 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.


Attachments:
(No filename) (762.00 B)
signature.asc (499.00 B)
Download all attachments

2023-01-20 13:38:24

by Herve Codina

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

Hi Mark,

On Fri, 20 Jan 2023 12:12:44 +0000
Mark Brown <[email protected]> wrote:

> On Fri, Jan 20, 2023 at 10:50:35AM +0100, Herve Codina wrote:
>
> > +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;
>
> > +
> > + 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.

> Otherwise this looks fine.

Thanks for the review.

Best regards,
Hervé

--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-01-22 13:46:30

by Krzysztof Kozlowski

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

On 20/01/2023 10:50, Herve Codina wrote:
> The Renesas IDT821034 codec is a quad PCM codec with programmable
> gain.
>
> Signed-off-by: Herve Codina <[email protected]>
> ---

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

> + gpio-controller: true
> +
> +required:
> + - compatible
> + - reg
> + - 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:

Reviewed-by: Krzysztof Kozlowski <[email protected]>


Best regards,
Krzysztof


2023-01-23 07:54:02

by Christophe Leroy

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



Le 20/01/2023 à 10:50, Herve Codina a écrit :
> 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.
>
> Signed-off-by: Herve Codina <[email protected]>
> ---
> 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
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index 0f9d71490075..67489b2ebd9f 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -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

Is selecting regmap still necessary ?

> + 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
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 71d3ce5867e4..bcf95de654fd 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -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
> diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> new file mode 100644
> index 000000000000..5eb93fec6042
> --- /dev/null
> +++ b/sound/soc/codecs/idt821034.c
> @@ -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;
> +
> + 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);

> +}
> +
> +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",
> + valw, *valr);
> +
> + return 0;
> +}
> +
> +/* Available mode the programming sequence */

s/the/for/ ?

> +#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)
> + 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);
}

> +
> + 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,
> + .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;

> +#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;
> +
> + ret = idt821034_reset_gpio(idt821034);
> + if (ret)
> + return ret;
> +
> + idt821034->gpio_chip.owner = THIS_MODULE;

Is the owner really needed ?

> + 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");

2023-01-23 08:56:47

by Herve Codina

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

Hi Christophe,

On Mon, 23 Jan 2023 07:53:49 +0000
Christophe Leroy <[email protected]> wrote:

> Le 20/01/2023 à 10:50, Herve Codina a écrit :
> > 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.
> >
> > Signed-off-by: Herve Codina <[email protected]>
> > ---
> > 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
> >
> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > index 0f9d71490075..67489b2ebd9f 100644
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -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
>
> Is selecting regmap still necessary ?

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

>
> > + 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
> > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> > index 71d3ce5867e4..bcf95de654fd 100644
> > --- a/sound/soc/codecs/Makefile
> > +++ b/sound/soc/codecs/Makefile
> > @@ -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
> > diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> > new file mode 100644
> > index 000000000000..5eb93fec6042
> > --- /dev/null
> > +++ b/sound/soc/codecs/idt821034.c
> > @@ -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;
> > +
> > + 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);

Yes, I will change in v3.

>
> > +}
> > +
> > +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",
> > + valw, *valr);
> > +
> > + return 0;
> > +}
> > +
> > +/* Available mode the programming sequence */
>
> s/the/for/ ?

Sure, I will change in v3.

>
> > +#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)
> > + 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);
> }

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 ?

>
> > +
> > + 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,
> > + .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;

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.

>
> > +#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;
> > +
> > + 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.

>
> > + 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");

Thanks for the review,

Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-01-23 10:47:30

by Christophe Leroy

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



Le 23/01/2023 à 09:56, Herve Codina a écrit :
>>> +
>>> +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)
>>> + 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);
>> }
>
> 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 ?

idt821034_2x8bit_write() looks good to me.

Christophe

2023-01-23 11:13:32

by Christophe Leroy

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

Hi Hervé,

Le 23/01/2023 à 09:56, Herve Codina a écrit :
>
> 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()

diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
index 5eb93fec6042..8b75388e22ce 100644
--- a/sound/soc/codecs/idt821034.c
+++ b/sound/soc/codecs/idt821034.c
@@ -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

2023-01-23 12:18:05

by Herve Codina

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

Hi Christophe,

On Mon, 23 Jan 2023 11:13:23 +0000
Christophe Leroy <[email protected]> wrote:

> Hi Hervé,
>
> Le 23/01/2023 à 09:56, Herve Codina a écrit :
> >
> > 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()
>
> diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> index 5eb93fec6042..8b75388e22ce 100644
> --- a/sound/soc/codecs/idt821034.c
> +++ b/sound/soc/codecs/idt821034.c
> @@ -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

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é
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-01-23 12:30:41

by Christophe Leroy

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



Le 23/01/2023 à 13:17, Herve Codina a écrit :
> Hi Christophe,
>
> On Mon, 23 Jan 2023 11:13:23 +0000
> Christophe Leroy <[email protected]> wrote:
>
>> Hi Hervé,
>>
>> Le 23/01/2023 à 09:56, Herve Codina a écrit :
>>>
>>> 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()
>>
>> diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
>> index 5eb93fec6042..8b75388e22ce 100644
>> --- a/sound/soc/codecs/idt821034.c
>> +++ b/sound/soc/codecs/idt821034.c
>> @@ -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
>
> 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;
> }
>


I guess you mean :

if (IS_ENABLED(CONFIG_GPIOLIB))


> Is that ok for you ?



What about:

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

Christophe

2023-01-23 12:59:56

by Herve Codina

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

On Mon, 23 Jan 2023 12:30:32 +0000
Christophe Leroy <[email protected]> wrote:

> Le 23/01/2023 à 13:17, Herve Codina a écrit :
> > Hi Christophe,
> >
> > On Mon, 23 Jan 2023 11:13:23 +0000
> > Christophe Leroy <[email protected]> wrote:
> >
> >> Hi Hervé,
> >>
> >> Le 23/01/2023 à 09:56, Herve Codina a écrit :
> >>>
> >>> 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()
> >>
> >> diff --git a/sound/soc/codecs/idt821034.c b/sound/soc/codecs/idt821034.c
> >> index 5eb93fec6042..8b75388e22ce 100644
> >> --- a/sound/soc/codecs/idt821034.c
> >> +++ b/sound/soc/codecs/idt821034.c
> >> @@ -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
> >
> > 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;
> > }
> >
>
>
> I guess you mean :
>
> if (IS_ENABLED(CONFIG_GPIOLIB))

Yes of course. Sorry for the typo.

>
>
> > Is that ok for you ?
>
>
>
> 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é

--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

2023-01-23 13:54:00

by Christophe Leroy

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

>
> 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;
> }
>


Yes that's fine for me.

Christophe