Devicetree
 help / color / mirror / Atom feed
* [PATCH v7 4/4] ASoc: dt-bindings: PCM6240: Add initial DT binding
From: Shenghao Ding @ 2024-03-31  2:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-sound, devicetree, perex, tiwai, 13916275206, mohit.chawla,
	soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal, s-hari, aviel,
	hnagalla, praneeth, Baojun.Xu, Shenghao Ding, Rob Herring
In-Reply-To: <20240331021835.1470-1-shenghao-ding@ti.com>

PCM6240 family chips are popular among audio customers, in spite of only a
portion of the functionality of codec, such as ADC or DAC, and so on, for
different Specifications, range from Personal Electric to Automotive
Electric, even some professional fields. Yet their audio performance is far
superior to the codec's, and cost is lower than codec, and much easier to
program than codec.

Reviewed-by: Rob Herring <robh@kernel.org>

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
Change in v7:
 - Rewrite the subject to match something similar to other commits.
 - And none of them are compatible with something.
 - minItems, then maxItems.
 - Drop reset-gpios description
 - Remove the repeated reg descriptions and reg constraints.
 - Drop redundant spaces.
 - Add missing line breaks between blocks and additionalProperties.
 - Correct compatibility issue on adc6120 and pcm6240.
 - All these chips have only a portion of the functionality of codec,
   such as ADC or DAC, and so on, but their audio performance is far
   superior to the codec's, and cost is lower than codec, and much easier
   to program than codec. Simply one or two register settings can enable
   them to work. Init for these chips are hardware reset or software reset.
   As to some audio filter params for internal filters, it is up to the
   special user cases, which can be saved into the bin file. The default
   value also can work well.
 - Add blank line before reg.
 - remove unneeded items and if branches.
 - Add missing compatible devices, such as adc6120, etc.
 - Add necessary people into the list for DTS review
 - correct misaligned.
 - simplify the compatibility
 - remove sound-name-prefix and revert back
 - Add review information
---
 .../devicetree/bindings/sound/ti,pcm6240.yaml | 177 ++++++++++++++++++
 1 file changed, 177 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm6240.yaml

diff --git a/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml
new file mode 100644
index 000000000000..dd5b08e3d7a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ti,pcm6240.yaml
@@ -0,0 +1,177 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 - 2024 Texas Instruments Incorporated
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,pcm6240.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments PCM6240 Family Audio ADC/DAC
+
+maintainers:
+  - Shenghao Ding <shenghao-ding@ti.com>
+
+description: |
+  The PCM6240 Family is a big family of Audio ADC/DAC for
+  different Specifications, range from Personal Electric
+  to Automotive Electric, even some professional fields.
+
+  Specifications about the audio chip can be found at:
+    https://www.ti.com/lit/gpn/tlv320adc3120
+    https://www.ti.com/lit/gpn/tlv320adc5120
+    https://www.ti.com/lit/gpn/tlv320adc6120
+    https://www.ti.com/lit/gpn/dix4192
+    https://www.ti.com/lit/gpn/pcm1690
+    https://www.ti.com/lit/gpn/pcm3120-q1
+    https://www.ti.com/lit/gpn/pcm3140-q1
+    https://www.ti.com/lit/gpn/pcm5120-q1
+    https://www.ti.com/lit/gpn/pcm6120-q1
+    https://www.ti.com/lit/gpn/pcm6260-q1
+    https://www.ti.com/lit/gpn/pcm9211
+    https://www.ti.com/lit/gpn/pcmd3140
+    https://www.ti.com/lit/gpn/pcmd3180
+    https://www.ti.com/lit/gpn/taa5212
+    https://www.ti.com/lit/gpn/tad5212
+
+properties:
+  compatible:
+    description: |
+      ti,adc3120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to-
+      digital converter (ADC) with 106-dB SNR.
+
+      ti,adc5120: 2-Channel, 768-kHz, Burr-Brown™ Audio ADC with 120-dB SNR.
+
+      ti,adc6120: Stereo-channel, 768-kHz, Burr-Brown™ audio analog-to-
+      digital converter (ADC) with 123-dB SNR.
+
+      ti,dix4192: 216-kHz digital audio converter with Quad-Channel In
+      and One-Channel Out.
+
+      ti,pcm1690: Automotive Catalog 113dB SNR 8-Channel Audio DAC with
+      Differential Outputs.
+
+      ti,pcm3120: Automotive, stereo, 106-dB SNR, 768-kHz, low-power
+      software-controlled audio ADC.
+
+      ti,pcm3140: Automotive, Quad-Channel, 768-kHz, Burr-Brown™ Audio ADC
+      with 106-dB SNR.
+
+      ti,pcm5120: Automotive, stereo, 120-dB SNR, 768-kHz, low-power
+      software-controlled audio ADC.
+
+      ti,pcm5140: Automotive, Quad-Channel, 768-kHz, Burr-Brown™ Audio ADC
+      with 120-dB SNR.
+
+      ti,pcm6120: Automotive, stereo, 123-dB SNR, 768-kHz, low-power
+      software-controlled audio ADC.
+
+      ti,pcm6140: Automotive, Quad-Channel, 768-kHz, Burr-Brown™ Audio ADC
+      with 123-dB SNR.
+
+      ti,pcm6240: Automotive 4-ch audio ADC with integrated programmable mic
+      bias, boost and input diagnostics.
+
+      ti,pcm6260: Automotive 6-ch audio ADC with integrated programmable mic
+      bias, boost and input diagnostics.
+
+      ti,pcm9211: 216-kHz digital audio converter With Stereo ADC and
+      Routing.
+
+      ti,pcmd3140: Four-channel PDM-input to TDM or I2S output converter.
+
+      ti,pcmd3180: Eight-channel pulse-density-modulation input to TDM or
+      I2S output converter.
+
+      ti,taa5212: Low-power high-performance stereo audio ADC with 118-dB
+      dynamic range.
+
+      ti,tad5212: Low-power stereo audio DAC with 120-dB dynamic range.
+    oneOf:
+      - items:
+          - enum:
+              - ti,adc3120
+              - ti,adc5120
+              - ti,pcm3120
+              - ti,pcm5120
+              - ti,pcm6120
+          - const: ti,adc6120
+      - items:
+          - enum:
+              - ti,pcmd512x
+              - ti,pcm9211
+              - ti,taa5212
+              - ti,tad5212
+          - const: ti,adc6120
+      - items:
+          - enum:
+              - ti,pcm3140
+              - ti,pcm5140
+              - ti,dix4192
+              - ti,pcm6140
+              - ti,pcm6260
+          - const: ti,pcm6240
+      - items:
+          - enum:
+              - ti,pcmd3140
+              - ti,pcmd3180
+              - ti,pcm1690
+              - ti,taa5412
+              - ti,tad5412
+          - const: ti,pcm6240
+      - enum:
+          - ti,adc6120
+          - ti,pcm6240
+
+  reg:
+    description:
+      I2C address, in multiple pcmdevices case, all the i2c address
+      aggregate as one Audio Device to support multiple audio slots.
+    minItems: 1
+    maxItems: 4
+
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+    description:
+      Invalid only for ti,pcm1690 because of no INT pin.
+
+  '#sound-dai-cells':
+    const: 0
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: dai-common.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - ti,pcm1690
+    then:
+      properties:
+        interrupts: false
+
+additionalProperties: false
+
+examples:
+  - |
+   #include <dt-bindings/gpio/gpio.h>
+   i2c {
+     /* example for two devices with interrupt support */
+     #address-cells = <1>;
+     #size-cells = <0>;
+     pcm6240: audio-codec@48 {
+       compatible = "ti,pcm6240";
+       reg = <0x48>, /* primary-device */
+             <0x4b>; /* secondary-device */
+       #sound-dai-cells = <0>;
+       reset-gpios = <&gpio1 10 GPIO_ACTIVE_HIGH>;
+       interrupt-parent = <&gpio1>;
+       interrupts = <15>;
+     };
+   };
+...
-- 
2.34.1


^ permalink raw reply related

* [PATCH v7 2/4] ASoc: PCM6240: Create header file for PCM6240 Family driver code
From: Shenghao Ding @ 2024-03-31  2:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-sound, devicetree, perex, tiwai, 13916275206, mohit.chawla,
	soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal, s-hari, aviel,
	hnagalla, praneeth, Baojun.Xu, Shenghao Ding
In-Reply-To: <20240331021835.1470-1-shenghao-ding@ti.com>

PCM6240 driver implements a flexible and configurable setting for register
and filter coefficients, to one, two or even multiple PCM6240 Family Audio
chips.

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
Change in v7:
 - remove unused data structure.
---
 sound/soc/codecs/pcm6240.h | 236 +++++++++++++++++++++++++++++++++++++
 1 file changed, 236 insertions(+)
 create mode 100644 sound/soc/codecs/pcm6240.h

diff --git a/sound/soc/codecs/pcm6240.h b/sound/soc/codecs/pcm6240.h
new file mode 100644
index 000000000000..49a313305b26
--- /dev/null
+++ b/sound/soc/codecs/pcm6240.h
@@ -0,0 +1,236 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+//
+// ALSA SoC Texas Instruments PCM6240 Family Audio ADC/DAC/Router
+//
+// Copyright (C) 2022 - 2024 Texas Instruments Incorporated
+// https://www.ti.com
+//
+// The PCM6240 driver implements a flexible and configurable
+// algo coefficient setting for one, two, or even multiple
+// PCM6240 Family Audio chips.
+//
+// Author: Shenghao Ding <shenghao-ding@ti.com>
+//
+
+#ifndef __PCM6240_H__
+#define __PCM6240_H__
+
+enum pcm_device {
+	ADC3120,
+	ADC5120,
+	ADC6120,
+	DIX4192,
+	PCM1690,
+	PCM3120,
+	PCM3140,
+	PCM5120,
+	PCM5140,
+	PCM6120,
+	PCM6140,
+	PCM6240,
+	PCM6260,
+	PCM9211,
+	PCMD3140,
+	PCMD3180,
+	PCMD512X,
+	TAA5212,
+	TAA5412,
+	TAD5212,
+	TAD5412,
+	MAX_DEVICE,
+};
+
+#define PCMDEV_GENERIC_VOL_CTRL			0x0
+#define PCMDEV_PCM1690_VOL_CTRL			0x1
+#define PCMDEV_PCM1690_FINE_VOL_CTRL		0x2
+
+/* Maximum number of I2C addresses */
+#define PCMDEVICE_MAX_I2C_DEVICES		4
+/* Maximum number defined in REGBIN protocol */
+#define PCMDEVICE_MAX_REGBIN_DEVICES		8
+#define PCMDEVICE_CONFIG_SUM			64
+#define PCMDEVICE_BIN_FILENAME_LEN		64
+
+#define PCMDEVICE_RATES	(SNDRV_PCM_RATE_44100 | \
+	SNDRV_PCM_RATE_48000)
+#define PCMDEVICE_MAX_CHANNELS			8
+#define PCMDEVICE_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
+	SNDRV_PCM_FMTBIT_S20_3LE | \
+	SNDRV_PCM_FMTBIT_S24_3LE | \
+	SNDRV_PCM_FMTBIT_S24_LE | \
+	SNDRV_PCM_FMTBIT_S32_LE)
+
+/* PAGE Control Register (available in page0 of each book) */
+#define PCMDEVICE_PAGE_SELECT			0x00
+#define PCMDEVICE_REG(page, reg)		((page * 128) + reg)
+#define PCMDEVICE_REG_SWRESET			PCMDEVICE_REG(0X0, 0x01)
+#define PCMDEVICE_REG_SWRESET_RESET		BIT(0)
+
+#define ADC5120_REG_CH1_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x3d)
+#define ADC5120_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x3e)
+#define ADC5120_REG_CH2_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x42)
+#define ADC5120_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x43)
+
+#define PCM1690_REG_MODE_CTRL			PCMDEVICE_REG(0X0, 0x46)
+#define PCM1690_REG_MODE_CTRL_DAMS_MSK		BIT(7)
+#define PCM1690_REG_MODE_CTRL_DAMS_FINE_STEP	0x0
+#define PCM1690_REG_MODE_CTRL_DAMS_WIDE_RANGE	0x80
+
+#define PCM1690_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x48)
+#define PCM1690_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x49)
+#define PCM1690_REG_CH3_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4a)
+#define PCM1690_REG_CH4_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4b)
+#define PCM1690_REG_CH5_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4c)
+#define PCM1690_REG_CH6_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4d)
+#define PCM1690_REG_CH7_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4e)
+#define PCM1690_REG_CH8_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4f)
+
+#define PCM6240_REG_CH1_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x3d)
+#define PCM6240_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x3e)
+#define PCM6240_REG_CH2_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x42)
+#define PCM6240_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x43)
+#define PCM6240_REG_CH3_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x47)
+#define PCM6240_REG_CH3_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x48)
+#define PCM6240_REG_CH4_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x4c)
+#define PCM6240_REG_CH4_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4d)
+
+#define PCM6260_REG_CH1_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x3d)
+#define PCM6260_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x3e)
+#define PCM6260_REG_CH2_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x42)
+#define PCM6260_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x43)
+#define PCM6260_REG_CH3_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x47)
+#define PCM6260_REG_CH3_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x48)
+#define PCM6260_REG_CH4_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x4c)
+#define PCM6260_REG_CH4_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4d)
+#define PCM6260_REG_CH5_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x51)
+#define PCM6260_REG_CH5_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x52)
+#define PCM6260_REG_CH6_ANALOG_GAIN		PCMDEVICE_REG(0X0, 0x56)
+#define PCM6260_REG_CH6_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x57)
+
+#define PCM9211_REG_SW_CTRL			PCMDEVICE_REG(0X0, 0x40)
+#define PCM9211_REG_SW_CTRL_MRST_MSK		BIT(7)
+#define PCM9211_REG_SW_CTRL_MRST		0x0
+
+#define PCM9211_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x46)
+#define PCM9211_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x47)
+
+#define PCMD3140_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x3E)
+#define PCMD3140_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x43)
+#define PCMD3140_REG_CH3_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x48)
+#define PCMD3140_REG_CH4_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4D)
+
+#define PCMD3180_REG_CH1_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x3E)
+#define PCMD3180_REG_CH2_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x43)
+#define PCMD3180_REG_CH3_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x48)
+#define PCMD3180_REG_CH4_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x4D)
+#define PCMD3180_REG_CH5_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x52)
+#define PCMD3180_REG_CH6_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x57)
+#define PCMD3180_REG_CH7_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x5C)
+#define PCMD3180_REG_CH8_DIGITAL_GAIN		PCMDEVICE_REG(0X0, 0x61)
+
+#define TAA5412_REG_CH1_DIGITAL_VOLUME		PCMDEVICE_REG(0X0, 0x52)
+#define TAA5412_REG_CH2_DIGITAL_VOLUME		PCMDEVICE_REG(0X0, 0x57)
+#define TAA5412_REG_CH3_DIGITAL_VOLUME		PCMDEVICE_REG(0X0, 0x5B)
+#define TAA5412_REG_CH4_DIGITAL_VOLUME		PCMDEVICE_REG(0X0, 0x5F)
+
+#define TAA5412_REG_CH1_FINE_GAIN		PCMDEVICE_REG(0X0, 0x53)
+#define TAA5412_REG_CH2_FINE_GAIN		PCMDEVICE_REG(0X0, 0x58)
+#define TAA5412_REG_CH3_FINE_GAIN		PCMDEVICE_REG(0X0, 0x5C)
+#define TAA5412_REG_CH4_FINE_GAIN		PCMDEVICE_REG(0X0, 0x60)
+
+#define PCMDEVICE_CMD_SING_W		0x1
+#define PCMDEVICE_CMD_BURST		0x2
+#define PCMDEVICE_CMD_DELAY		0x3
+#define PCMDEVICE_CMD_FIELD_W		0x4
+
+enum pcmdevice_bin_blk_type {
+	PCMDEVICE_BIN_BLK_COEFF = 1,
+	PCMDEVICE_BIN_BLK_POST_POWER_UP,
+	PCMDEVICE_BIN_BLK_PRE_SHUTDOWN,
+	PCMDEVICE_BIN_BLK_PRE_POWER_UP,
+	PCMDEVICE_BIN_BLK_POST_SHUTDOWN
+};
+
+enum pcmdevice_fw_state {
+	PCMDEVICE_FW_LOAD_OK = 0,
+	PCMDEVICE_FW_LOAD_FAILED
+};
+
+struct pcmdevice_regbin_hdr {
+	unsigned int img_sz;
+	unsigned int checksum;
+	unsigned int binary_version_num;
+	unsigned int drv_fw_version;
+	unsigned int timestamp;
+	unsigned char plat_type;
+	unsigned char dev_family;
+	unsigned char reserve;
+	unsigned char ndev;
+	unsigned char devs[PCMDEVICE_MAX_REGBIN_DEVICES];
+	unsigned int nconfig;
+	unsigned int config_size[PCMDEVICE_CONFIG_SUM];
+};
+
+struct pcmdevice_block_data {
+	unsigned char dev_idx;
+	unsigned char block_type;
+	unsigned short yram_checksum;
+	unsigned int block_size;
+	unsigned int n_subblks;
+	unsigned char *regdata;
+};
+
+struct pcmdevice_config_info {
+	char cfg_name[64];
+	unsigned int nblocks;
+	unsigned int real_nblocks;
+	unsigned char active_dev;
+	struct pcmdevice_block_data **blk_data;
+};
+
+struct pcmdevice_regbin {
+	struct pcmdevice_regbin_hdr fw_hdr;
+	int ncfgs;
+	struct pcmdevice_config_info **cfg_info;
+};
+
+struct pcmdevice_irqinfo {
+	int gpio;
+	int nmb;
+};
+
+struct pcmdevice_priv {
+	struct snd_soc_component *component;
+	struct i2c_client *client;
+	struct device *dev;
+	struct mutex codec_lock;
+	struct gpio_desc *hw_rst;
+	struct regmap *regmap;
+	struct pcmdevice_regbin regbin;
+	struct pcmdevice_irqinfo irq_info;
+	unsigned int addr[PCMDEVICE_MAX_I2C_DEVICES];
+	unsigned int chip_id;
+	int cur_conf;
+	int fw_state;
+	int ndev;
+	unsigned char bin_name[PCMDEVICE_BIN_FILENAME_LEN];
+	unsigned char dev_name[I2C_NAME_SIZE];
+};
+
+/* mixer control */
+struct pcmdevice_mixer_control {
+	int max;
+	int reg;
+	unsigned int dev_no;
+	unsigned int shift;
+	unsigned int invert;
+};
+struct pcmdev_ctrl_info {
+	const unsigned int *gain;
+	const struct pcmdevice_mixer_control *pcmdev_ctrl;
+	unsigned int ctrl_array_size;
+	snd_kcontrol_get_t *get;
+	snd_kcontrol_put_t *put;
+	int pcmdev_ctrl_name_id;
+};
+#endif /* __PCM6240_H__ */
-- 
2.34.1


^ permalink raw reply related

* [PATCH v7 0/4] mixer-test report
From: Shenghao Ding @ 2024-03-31  2:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-sound, devicetree, perex, tiwai, 13916275206, mohit.chawla,
	soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal, s-hari, aviel,
	hnagalla, praneeth, Baojun.Xu, Shenghao Ding

*** BLURB HERE ***
mixer-test report:
 root@am335x-evm:/bin# mixer-test
 TAP version 13
 # Card 0 - TI BeagleBone Black (TI BeagleBone Black)
 1..7
 ok 1 get_value.0.0
 # 0.0 pcmd3180-i2c-2 Profile id
 ok 2 name.0.0
 ok 3 write_default.0.0
 ok 4 write_valid.0.0
 ok 5 write_invalid.0.0
 ok 6 event_missing.0.0
 ok 7 event_spurious.0.0
 # Totals: pass:7 fail:0 xfail:0 xpass:0 skip:0 error:0
 root@am335x-evm:/bin#

Shenghao Ding (4):
  ASoc: PCM6240: Create PCM6240 Family driver code
  ASoc: PCM6240: Create header file for PCM6240 Family driver code
  ASoc: PCM6240: Add compile item for PCM6240 Family driver
  ASoc: dt-bindings: PCM6240: Add initial DT binding

 .../devicetree/bindings/sound/ti,pcm6240.yaml |  177 ++
 sound/soc/codecs/Kconfig                      |   10 +
 sound/soc/codecs/Makefile                     |    2 +
 sound/soc/codecs/pcm6240.c                    | 2071 +++++++++++++++++
 sound/soc/codecs/pcm6240.h                    |  236 ++
 5 files changed, 2496 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm6240.yaml
 create mode 100644 sound/soc/codecs/pcm6240.c
 create mode 100644 sound/soc/codecs/pcm6240.h

-- 
2.34.1


^ permalink raw reply

* [PATCH v7 1/4] ASoc: PCM6240: Create PCM6240 Family driver code
From: Shenghao Ding @ 2024-03-31  2:18 UTC (permalink / raw)
  To: linux-kernel
  Cc: lgirdwood, broonie, robh+dt, krzysztof.kozlowski+dt, conor+dt,
	linux-sound, devicetree, perex, tiwai, 13916275206, mohit.chawla,
	soyer, jkhuang3, tiwai, pdjuandi, manisha.agrawal, s-hari, aviel,
	hnagalla, praneeth, Baojun.Xu, Shenghao Ding
In-Reply-To: <20240331021835.1470-1-shenghao-ding@ti.com>

PCM6240 driver implements a flexible and configurable setting for register
and filter coefficients, to one, two or even multiple PCM6240 Family Audio
chips.

Signed-off-by: Shenghao Ding <shenghao-ding@ti.com>

---
Change in v7:
 - Use _MAPLE as cache_type for new devices, it's a more modern design with
   tradeoffs that work better for most current systems.
 - Rewrite error checks to return immediately on error in pcmdev_dev_read,
   pcmdev_dev_write, pcmdev_dev_update_bits, and pcmdev_dev_bulk_write,
   that way there's less indentation and fewer paths later on.
 - Add profileid check in pcmdevice_set_profile_id, if invalid, set to
   max-value or 0.
 - fixed the return value as -EBUSY in pcmdevice_startup
 - remove unused chn and chenend in pcmdevice_select_cfg_blk
 - Be consistent with name, ret, rc, err.
 - fixed compiling warning on iteration 4 invokes undefined behavior in
   pcmdev_regbin_ready.
 - remove unnecessary blank line between
   "module_i2c_driver(pcmdevice_i2c_driver);" and
   "static struct i2c_driver pcmdevice_i2c_driver".
 - Move reset into i2c_probe and remove sw_rst callback.
 - extract the put_vol and get_vol function as pcmdev_put_volsw and
   pcmdev_get_volsw to reduce the red
 - Simplify and rewirte gain_control_add code.
 - Move gain kcontrol into codec_probe.
 - Unified ret and rc to ret.
 - correct of_gpio.h to gpio.h
 - Add an array pcmdev_ctrl_name to store the gain control name.
 - Move i2c_set_clientdata before sw_reset.
 - Remove pcmdevice_startup, it seemed unnecessary once firmware_request was
   move to i2c_probe.
 - Add return value into pcmdev_profile_ctrl_add.
 - If one side of the if/else has a braces both should.
 - mixer-test kselftest to check the Kcontrol
 - follow the same convention for error log
 - Add sound-name-prefix for bin file name and control name
 - use get_unaligned_be16 instead of be16_to_cpup and get_unaligned_be32
   instead of be32_to_cpup for potentially broken alignment
 - remove gain kcontrol for each device
 - send the output of mixer-test from a machine with this driver in it as
   part of the cover letter, it'll make it easier to tell what's going on.
---
 sound/soc/codecs/pcm6240.c | 2071 ++++++++++++++++++++++++++++++++++++
 1 file changed, 2071 insertions(+)
 create mode 100644 sound/soc/codecs/pcm6240.c

diff --git a/sound/soc/codecs/pcm6240.c b/sound/soc/codecs/pcm6240.c
new file mode 100644
index 000000000000..f8b713f9cad1
--- /dev/null
+++ b/sound/soc/codecs/pcm6240.c
@@ -0,0 +1,2071 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// ALSA SoC Texas Instruments PCM6240 Family Audio ADC/DAC Device
+//
+// Copyright (C) 2022 - 2024 Texas Instruments Incorporated
+// https://www.ti.com
+//
+// The PCM6240 driver implements a flexible and configurable
+// algo coefficient setting for one, two, or even multiple
+// PCM6240 Family chips.
+//
+// Author: Shenghao Ding <shenghao-ding@ti.com>
+//
+
+#include <asm/unaligned.h>
+#include <linux/firmware.h>
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of_irq.h>
+#include <linux/regmap.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/tlv.h>
+
+#include "pcm6240.h"
+
+static const struct i2c_device_id pcmdevice_i2c_id[] = {
+	{ "adc3120",  ADC3120  },
+	{ "adc5120",  ADC5120  },
+	{ "adc6120",  ADC6120  },
+	{ "dix4192",  DIX4192  },
+	{ "pcm1690",  PCM1690  },
+	{ "pcm3120",  PCM3120  },
+	{ "pcm3140",  PCM3140  },
+	{ "pcm5120",  PCM5120  },
+	{ "pcm5140",  PCM5140  },
+	{ "pcm6120",  PCM6120  },
+	{ "pcm6140",  PCM6140  },
+	{ "pcm6240",  PCM6240  },
+	{ "pcm6260",  PCM6260  },
+	{ "pcm9211",  PCM9211  },
+	{ "pcmd3140", PCMD3140 },
+	{ "pcmd3180", PCMD3180 },
+	{ "pcmd512x", PCMD512X },
+	{ "taa5212",  TAA5212  },
+	{ "taa5412",  TAA5412  },
+	{ "tad5212",  TAD5212  },
+	{ "tad5412",  TAD5412  },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, pcmdevice_i2c_id);
+
+static const char *const pcmdev_ctrl_name[] = {
+	"%s-i2c-%d-dev%d-ch%d-ana-gain",
+	"%s-i2c-%d-dev%d-ch%d-digi-gain",
+	"%s-i2c-%d-dev%d-ch%d-fine-gain",
+};
+
+static const char *const pcmdev_ctrl_name_with_prefix[] = {
+	"%s-dev%d-ch%d-ana-gain",
+	"%s-dev%d-ch%d-digi-gain",
+	"%s-dev%d-ch%d-fine-gain",
+};
+
+static const struct pcmdevice_mixer_control adc5120_analog_gain_ctl[] = {
+	{
+		.shift = 1,
+		.reg = ADC5120_REG_CH1_ANALOG_GAIN,
+		.max = 0x54,
+		.invert = 0,
+	},
+	{
+		.shift = 1,
+		.reg = ADC5120_REG_CH2_ANALOG_GAIN,
+		.max = 0x54,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control adc5120_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = ADC5120_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = ADC5120_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcm1690_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH3_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH4_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH5_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH6_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH7_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM1690_REG_CH8_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcm6240_analog_gain_ctl[] = {
+	{
+		.shift = 2,
+		.reg = PCM6240_REG_CH1_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6240_REG_CH2_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6240_REG_CH3_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6240_REG_CH4_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcm6240_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = PCM6240_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6240_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6240_REG_CH3_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6240_REG_CH4_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcm6260_analog_gain_ctl[] = {
+	{
+		.shift = 2,
+		.reg = PCM6260_REG_CH1_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6260_REG_CH2_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6260_REG_CH3_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6260_REG_CH4_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6260_REG_CH5_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	},
+	{
+		.shift = 2,
+		.reg = PCM6260_REG_CH6_ANALOG_GAIN,
+		.max = 0x42,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcm6260_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = PCM6260_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6260_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6260_REG_CH3_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6260_REG_CH4_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6260_REG_CH5_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM6260_REG_CH6_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcm9211_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = PCM9211_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCM9211_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcmd3140_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = PCMD3140_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3140_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3140_REG_CH3_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3140_REG_CH4_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control pcmd3180_digi_gain_ctl[] = {
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH1_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH2_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH3_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH4_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH5_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH6_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH7_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = PCMD3180_REG_CH8_DIGITAL_GAIN,
+		.max = 0xff,
+		.invert = 0,
+	}
+};
+
+static const struct pcmdevice_mixer_control taa5412_digi_vol_ctl[] = {
+	{
+		.shift = 0,
+		.reg = TAA5412_REG_CH1_DIGITAL_VOLUME,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = TAA5412_REG_CH2_DIGITAL_VOLUME,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = TAA5412_REG_CH3_DIGITAL_VOLUME,
+		.max = 0xff,
+		.invert = 0,
+	},
+	{
+		.shift = 0,
+		.reg = TAA5412_REG_CH4_DIGITAL_VOLUME,
+		.max = 0xff,
+		.invert = 0,
+	},
+};
+
+static const struct pcmdevice_mixer_control taa5412_fine_gain_ctl[] = {
+	{
+		.shift = 4,
+		.reg = TAA5412_REG_CH1_FINE_GAIN,
+		.max = 0xf,
+		.invert = 0,
+	},
+	{
+		.shift = 4,
+		.reg = TAA5412_REG_CH2_FINE_GAIN,
+		.max = 0xf,
+		.invert = 0,
+	},
+	{
+		.shift = 4,
+		.reg = TAA5412_REG_CH3_FINE_GAIN,
+		.max = 0xf,
+		.invert = 4,
+	},
+	{
+		.shift = 0,
+		.reg = TAA5412_REG_CH4_FINE_GAIN,
+		.max = 0xf,
+		.invert = 4,
+	},
+};
+
+static const DECLARE_TLV_DB_MINMAX_MUTE(pcmd3140_dig_gain_tlv,
+	-10000, 2700);
+static const DECLARE_TLV_DB_MINMAX_MUTE(pcm1690_fine_dig_gain_tlv,
+	-12750, 0);
+static const DECLARE_TLV_DB_MINMAX_MUTE(pcm1690_dig_gain_tlv,
+	-25500, 0);
+static const DECLARE_TLV_DB_MINMAX_MUTE(pcm9211_dig_gain_tlv,
+	-11450, 2000);
+static const DECLARE_TLV_DB_MINMAX_MUTE(adc5120_fgain_tlv,
+	-10050, 2700);
+static const DECLARE_TLV_DB_LINEAR(adc5120_chgain_tlv, 0, 4200);
+static const DECLARE_TLV_DB_MINMAX_MUTE(pcm6260_fgain_tlv,
+	-10000, 2700);
+static const DECLARE_TLV_DB_LINEAR(pcm6260_chgain_tlv, 0, 4200);
+static const DECLARE_TLV_DB_MINMAX_MUTE(taa5412_dig_vol_tlv,
+	-8050, 4700);
+static const DECLARE_TLV_DB_LINEAR(taa5412_fine_gain_tlv,
+	-80, 70);
+
+static int pcmdev_change_dev(struct pcmdevice_priv *pcm_priv,
+	unsigned short dev_no)
+{
+	struct i2c_client *client = (struct i2c_client *)pcm_priv->client;
+	struct regmap *map = pcm_priv->regmap;
+	int ret;
+
+	if (client->addr == pcm_priv->addr[dev_no])
+		return 0;
+
+	client->addr = pcm_priv->addr[dev_no];
+	/* All pcmdevices share the same regmap, clear the page
+	 * inside regmap once switching to another pcmdevice.
+	 * Register 0 at any pages inside pcmdevice is the same
+	 * one for page-switching.
+	 */
+	ret = regmap_write(map, PCMDEVICE_PAGE_SELECT, 0);
+	if (ret < 0)
+		dev_err(pcm_priv->dev, "%s: err = %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int pcmdev_dev_read(struct pcmdevice_priv *pcm_dev,
+	unsigned int dev_no, unsigned int reg, unsigned int *val)
+{
+	struct regmap *map = pcm_dev->regmap;
+	int ret;
+
+	if (dev_no >= pcm_dev->ndev) {
+		dev_err(pcm_dev->dev, "%s: no such channel(%d)\n", __func__,
+			dev_no);
+		return -EINVAL;
+	}
+
+	ret = pcmdev_change_dev(pcm_dev, dev_no);
+	if (ret < 0) {
+		dev_err(pcm_dev->dev, "%s: chg dev err = %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = regmap_read(map, reg, val);
+	if (ret < 0)
+		dev_err(pcm_dev->dev, "%s: err = %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int pcmdev_dev_update_bits(struct pcmdevice_priv *pcm_dev,
+	unsigned int dev_no, unsigned int reg, unsigned int mask,
+	unsigned int value)
+{
+	struct regmap *map = pcm_dev->regmap;
+	int ret;
+
+	if (dev_no >= pcm_dev->ndev) {
+		dev_err(pcm_dev->dev, "%s: no such channel(%d)\n", __func__,
+			dev_no);
+		return -EINVAL;
+	}
+
+	ret = pcmdev_change_dev(pcm_dev, dev_no);
+	if (ret < 0) {
+		dev_err(pcm_dev->dev, "%s: chg dev err = %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(map, reg, mask, value);
+	if (ret < 0)
+		dev_err(pcm_dev->dev, "%s: update_bits err=%d\n",
+			__func__, ret);
+
+	return ret;
+}
+
+static int pcmdev_get_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol, int vol_ctrl_type)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct pcmdevice_priv *pcm_dev =
+		snd_soc_component_get_drvdata(component);
+	struct pcmdevice_mixer_control *mc =
+		(struct pcmdevice_mixer_control *)kcontrol->private_value;
+	int max = mc->max, ret;
+	unsigned int mask = BIT(fls(max)) - 1;
+	unsigned int dev_no = mc->dev_no;
+	unsigned int shift = mc->shift;
+	unsigned int reg = mc->reg;
+	unsigned int val;
+
+	mutex_lock(&pcm_dev->codec_lock);
+
+	if (pcm_dev->chip_id == PCM1690) {
+		ret = pcmdev_dev_read(pcm_dev, dev_no, PCM1690_REG_MODE_CTRL,
+			&val);
+		if (ret) {
+			dev_err(pcm_dev->dev, "%s: read mode err=%d\n",
+				__func__, ret);
+			goto out;
+		}
+		val &= PCM1690_REG_MODE_CTRL_DAMS_MSK;
+		/* Set to wide-range mode, before using vol ctrl. */
+		if (!val && vol_ctrl_type == PCMDEV_PCM1690_VOL_CTRL) {
+			ucontrol->value.integer.value[0] = -25500;
+			goto out;
+		}
+		/* Set to fine mode, before using fine vol ctrl. */
+		if (val && vol_ctrl_type == PCMDEV_PCM1690_FINE_VOL_CTRL) {
+			ucontrol->value.integer.value[0] = -12750;
+			goto out;
+		}
+	}
+
+	ret = pcmdev_dev_read(pcm_dev, dev_no, reg, &val);
+	if (ret) {
+		dev_err(pcm_dev->dev, "%s: read err=%d\n",
+			__func__, ret);
+		goto out;
+	}
+
+	val = (val >> shift) & mask;
+	val = (val > max) ? max : val;
+	val = mc->invert ? max - val : val;
+	ucontrol->value.integer.value[0] = val;
+out:
+	mutex_unlock(&pcm_dev->codec_lock);
+	return ret;
+}
+
+static int pcmdevice_get_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	return pcmdev_get_volsw(kcontrol, ucontrol, PCMDEV_GENERIC_VOL_CTRL);
+}
+
+static int pcm1690_get_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	return pcmdev_get_volsw(kcontrol, ucontrol, PCMDEV_PCM1690_VOL_CTRL);
+}
+
+static int pcm1690_get_finevolsw(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	return pcmdev_get_volsw(kcontrol, ucontrol,
+		PCMDEV_PCM1690_FINE_VOL_CTRL);
+}
+
+static int pcmdev_put_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol, int vol_ctrl_type)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct pcmdevice_priv *pcm_dev =
+		snd_soc_component_get_drvdata(component);
+	struct pcmdevice_mixer_control *mc =
+		(struct pcmdevice_mixer_control *)kcontrol->private_value;
+	int max = mc->max, err = 1;
+	unsigned int mask = BIT(fls(max)) - 1;
+	unsigned int dev_no = mc->dev_no;
+	unsigned int shift = mc->shift;
+	unsigned int val, val_mask;
+	unsigned int reg = mc->reg;
+
+	mutex_lock(&pcm_dev->codec_lock);
+	val = ucontrol->value.integer.value[0] & mask;
+	val = (val > max) ? max : val;
+	val = mc->invert ? max - val : val;
+	val_mask = mask << shift;
+	val = val << shift;
+
+	switch (vol_ctrl_type) {
+	case PCMDEV_PCM1690_VOL_CTRL:
+		val_mask |= PCM1690_REG_MODE_CTRL_DAMS_MSK;
+		val |= PCM1690_REG_MODE_CTRL_DAMS_WIDE_RANGE;
+		break;
+	case PCMDEV_PCM1690_FINE_VOL_CTRL:
+		val_mask |= PCM1690_REG_MODE_CTRL_DAMS_MSK;
+		val |= PCM1690_REG_MODE_CTRL_DAMS_FINE_STEP;
+		break;
+	}
+
+	err = pcmdev_dev_update_bits(pcm_dev, dev_no, reg, val_mask, val);
+	if (err) {
+		dev_err(pcm_dev->dev, "%s: update_bits err = %d\n",
+			__func__, err);
+		err = 0;
+	}
+
+	mutex_unlock(&pcm_dev->codec_lock);
+	return err;
+}
+
+static int pcmdevice_put_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	return pcmdev_put_volsw(kcontrol, ucontrol, PCMDEV_GENERIC_VOL_CTRL);
+}
+
+static int pcm1690_put_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	return pcmdev_put_volsw(kcontrol, ucontrol, PCMDEV_PCM1690_VOL_CTRL);
+}
+
+static int pcm1690_put_finevolsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	return pcmdev_put_volsw(kcontrol, ucontrol,
+		PCMDEV_PCM1690_FINE_VOL_CTRL);
+}
+
+static const struct pcmdev_ctrl_info pcmdev_gain_ctl_info[][2] = {
+	// ADC3120
+	{
+		{
+			.gain = adc5120_chgain_tlv,
+			.pcmdev_ctrl = adc5120_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = adc5120_fgain_tlv,
+			.pcmdev_ctrl = adc5120_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// ADC5120
+	{
+		{
+			.gain = adc5120_chgain_tlv,
+			.pcmdev_ctrl = adc5120_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = adc5120_fgain_tlv,
+			.pcmdev_ctrl = adc5120_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// ADC6120
+	{
+		{
+			.gain = adc5120_chgain_tlv,
+			.pcmdev_ctrl = adc5120_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = adc5120_fgain_tlv,
+			.pcmdev_ctrl = adc5120_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// DIX4192
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.ctrl_array_size = 0,
+		},
+	},
+	// PCM1690
+	{
+		{
+			.gain = pcm1690_fine_dig_gain_tlv,
+			.pcmdev_ctrl = pcm1690_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm1690_digi_gain_ctl),
+			.get = pcm1690_get_volsw,
+			.put = pcm1690_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+		{
+			.gain = pcm1690_dig_gain_tlv,
+			.pcmdev_ctrl = pcm1690_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm1690_digi_gain_ctl),
+			.get = pcm1690_get_finevolsw,
+			.put = pcm1690_put_finevolsw,
+			.pcmdev_ctrl_name_id = 2,
+		},
+	},
+	// PCM3120
+	{
+		{
+			.gain = adc5120_chgain_tlv,
+			.pcmdev_ctrl = adc5120_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = adc5120_fgain_tlv,
+			.pcmdev_ctrl = adc5120_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM3140
+	{
+		{
+			.gain = pcm6260_chgain_tlv,
+			.pcmdev_ctrl = pcm6240_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = pcm6260_fgain_tlv,
+			.pcmdev_ctrl = pcm6240_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM5120
+	{
+		{
+			.gain = adc5120_chgain_tlv,
+			.pcmdev_ctrl = adc5120_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = adc5120_fgain_tlv,
+			.pcmdev_ctrl = adc5120_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM5140
+	{
+		{
+			.gain = pcm6260_chgain_tlv,
+			.pcmdev_ctrl = pcm6240_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = pcm6260_fgain_tlv,
+			.pcmdev_ctrl = pcm6240_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM6120
+	{
+		{
+			.gain = adc5120_chgain_tlv,
+			.pcmdev_ctrl = adc5120_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = adc5120_fgain_tlv,
+			.pcmdev_ctrl = adc5120_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(adc5120_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM6140
+	{
+		{
+			.gain = pcm6260_chgain_tlv,
+			.pcmdev_ctrl = pcm6240_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = pcm6260_fgain_tlv,
+			.pcmdev_ctrl = pcm6240_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM6240
+	{
+		{
+			.gain = pcm6260_chgain_tlv,
+			.pcmdev_ctrl = pcm6240_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = pcm6260_fgain_tlv,
+			.pcmdev_ctrl = pcm6240_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6240_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM6260
+	{
+		{
+			.gain = pcm6260_chgain_tlv,
+			.pcmdev_ctrl = pcm6260_analog_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6260_analog_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 0,
+		},
+		{
+			.gain = pcm6260_fgain_tlv,
+			.pcmdev_ctrl = pcm6260_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm6260_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCM9211
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.gain = pcm9211_dig_gain_tlv,
+			.pcmdev_ctrl = pcm9211_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcm9211_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+
+	},
+	// PCMD3140
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.gain = pcmd3140_dig_gain_tlv,
+			.pcmdev_ctrl = pcmd3140_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcmd3140_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCMD3180
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.gain = pcmd3140_dig_gain_tlv,
+			.pcmdev_ctrl = pcmd3180_digi_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(pcmd3180_digi_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// PCMD512X
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.ctrl_array_size = 0,
+		},
+	},
+	// TAA5212
+	{
+		{
+			.gain = taa5412_fine_gain_tlv,
+			.pcmdev_ctrl = taa5412_fine_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(taa5412_fine_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 2,
+		},
+		{
+			.gain = taa5412_dig_vol_tlv,
+			.pcmdev_ctrl = taa5412_digi_vol_ctl,
+			.ctrl_array_size = ARRAY_SIZE(taa5412_digi_vol_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// TAA5412
+	{
+		{
+			.gain = taa5412_fine_gain_tlv,
+			.pcmdev_ctrl = taa5412_fine_gain_ctl,
+			.ctrl_array_size = ARRAY_SIZE(taa5412_fine_gain_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 2,
+		},
+		{
+			.gain = taa5412_dig_vol_tlv,
+			.pcmdev_ctrl = taa5412_digi_vol_ctl,
+			.ctrl_array_size = ARRAY_SIZE(taa5412_digi_vol_ctl),
+			.get = pcmdevice_get_volsw,
+			.put = pcmdevice_put_volsw,
+			.pcmdev_ctrl_name_id = 1,
+		},
+	},
+	// TAD5212
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.ctrl_array_size = 0,
+		},
+	},
+	// TAD5412
+	{
+		{
+			.ctrl_array_size = 0,
+		},
+		{
+			.ctrl_array_size = 0,
+		},
+	},
+};
+
+static int pcmdev_dev_bulk_write(struct pcmdevice_priv *pcm_dev,
+	unsigned int dev_no, unsigned int reg, unsigned char *data,
+	unsigned int len)
+{
+	struct regmap *map = pcm_dev->regmap;
+	int ret;
+
+	if (dev_no >= pcm_dev->ndev) {
+		dev_err(pcm_dev->dev, "%s: no such channel(%d)\n", __func__,
+			dev_no);
+		return -EINVAL;
+	}
+
+	ret = pcmdev_change_dev(pcm_dev, dev_no);
+	if (ret < 0) {
+		dev_err(pcm_dev->dev, "%s: chg dev err = %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = regmap_bulk_write(map, reg, data, len);
+	if (ret < 0)
+		dev_err(pcm_dev->dev, "%s: bulk_write err = %d\n", __func__,
+			ret);
+
+	return ret;
+}
+
+static int pcmdev_dev_write(struct pcmdevice_priv *pcm_dev,
+	unsigned int dev_no, unsigned int reg, unsigned int value)
+{
+	struct regmap *map = pcm_dev->regmap;
+	int ret;
+
+	if (dev_no >= pcm_dev->ndev) {
+		dev_err(pcm_dev->dev, "%s: no such channel(%d)\n", __func__,
+			dev_no);
+		return -EINVAL;
+	}
+
+	ret = pcmdev_change_dev(pcm_dev, dev_no);
+	if (ret < 0) {
+		dev_err(pcm_dev->dev, "%s: chg dev err = %d\n", __func__, ret);
+		return ret;
+	}
+
+	ret = regmap_write(map, reg, value);
+	if (ret < 0)
+		dev_err(pcm_dev->dev, "%s: err = %d\n", __func__, ret);
+
+	return ret;
+}
+
+static int pcmdevice_info_profile(
+	struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_info *uinfo)
+{
+	struct snd_soc_component *codec
+		= snd_soc_kcontrol_component(kcontrol);
+	struct pcmdevice_priv *pcm_dev =
+		snd_soc_component_get_drvdata(codec);
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = max(0, pcm_dev->regbin.ncfgs - 1);
+
+	return 0;
+}
+
+static int pcmdevice_get_profile_id(
+	struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *codec
+		= snd_soc_kcontrol_component(kcontrol);
+	struct pcmdevice_priv *pcm_dev =
+		snd_soc_component_get_drvdata(codec);
+
+	ucontrol->value.integer.value[0] = pcm_dev->cur_conf;
+
+	return 0;
+}
+
+static int pcmdevice_set_profile_id(
+	struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *codec
+		= snd_soc_kcontrol_component(kcontrol);
+	struct pcmdevice_priv *pcm_dev =
+		snd_soc_component_get_drvdata(codec);
+	int nr_profile = ucontrol->value.integer.value[0];
+	int max = pcm_dev->regbin.ncfgs - 1;
+	int ret = 0;
+
+	nr_profile = clamp(nr_profile, 0, max);
+
+	if (pcm_dev->cur_conf != nr_profile) {
+		pcm_dev->cur_conf = nr_profile;
+		ret = 1;
+	}
+
+	return ret;
+}
+
+static int pcmdevice_info_volsw(struct snd_kcontrol *kcontrol,
+	struct snd_ctl_elem_info *uinfo)
+{
+	struct pcmdevice_mixer_control *mc =
+		(struct pcmdevice_mixer_control *)kcontrol->private_value;
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = mc->max;
+	return 0;
+}
+
+static void pcm9211_sw_rst(struct pcmdevice_priv *pcm_dev)
+{
+	int ret, i;
+
+	for (i = 0; i < pcm_dev->ndev; i++) {
+		ret = pcmdev_dev_update_bits(pcm_dev, i,
+			PCM9211_REG_SW_CTRL, PCM9211_REG_SW_CTRL_MRST_MSK,
+			PCM9211_REG_SW_CTRL_MRST);
+		if (ret < 0)
+			dev_err(pcm_dev->dev, "%s: dev %d swreset fail %d\n",
+				__func__, i, ret);
+	}
+}
+
+static void pcmdevice_sw_rst(struct pcmdevice_priv *pcm_dev)
+{
+	int ret, i;
+
+	for (i = 0; i < pcm_dev->ndev; i++) {
+		ret = pcmdev_dev_write(pcm_dev, i, PCMDEVICE_REG_SWRESET,
+			PCMDEVICE_REG_SWRESET_RESET);
+		if (ret < 0)
+			dev_err(pcm_dev->dev, "%s: dev %d swreset fail %d\n",
+				__func__, i, ret);
+	}
+}
+
+static struct pcmdevice_config_info *pcmdevice_add_config(void *ctxt,
+	const unsigned char *config_data, unsigned int config_size,
+	int *status)
+{
+	struct pcmdevice_priv *pcm_dev = (struct pcmdevice_priv *)ctxt;
+	struct pcmdevice_config_info *cfg_info;
+	struct pcmdevice_block_data **bk_da;
+	unsigned int config_offset = 0, i;
+
+	cfg_info = kzalloc(sizeof(struct pcmdevice_config_info), GFP_KERNEL);
+	if (!cfg_info) {
+		*status = -ENOMEM;
+		goto out;
+	}
+
+	if (pcm_dev->regbin.fw_hdr.binary_version_num >= 0x105) {
+		if (config_offset + 64 > (int)config_size) {
+			*status = -EINVAL;
+			dev_err(pcm_dev->dev,
+				"%s: cfg_name out of boundary\n", __func__);
+			goto out;
+		}
+		memcpy(cfg_info->cfg_name, &config_data[config_offset], 64);
+		config_offset += 64;
+	}
+
+	if (config_offset + 4 > config_size) {
+		*status = -EINVAL;
+		dev_err(pcm_dev->dev, "%s: nblocks out of boundary\n",
+			__func__);
+		goto out;
+	}
+	cfg_info->nblocks =
+		get_unaligned_be32(&config_data[config_offset]);
+	config_offset += 4;
+
+	bk_da = cfg_info->blk_data = kcalloc(cfg_info->nblocks,
+		sizeof(struct pcmdevice_block_data *), GFP_KERNEL);
+	if (!bk_da) {
+		*status = -ENOMEM;
+		goto out;
+	}
+	cfg_info->real_nblocks = 0;
+	for (i = 0; i < cfg_info->nblocks; i++) {
+		if (config_offset + 12 > config_size) {
+			*status = -EINVAL;
+			dev_err(pcm_dev->dev,
+				"%s: out of boundary i = %d nblocks = %u\n",
+				__func__, i, cfg_info->nblocks);
+			break;
+		}
+		bk_da[i] = kzalloc(sizeof(struct pcmdevice_block_data),
+			GFP_KERNEL);
+		if (!bk_da[i]) {
+			*status = -ENOMEM;
+			break;
+		}
+		bk_da[i]->dev_idx = config_data[config_offset];
+		config_offset++;
+
+		bk_da[i]->block_type = config_data[config_offset];
+		config_offset++;
+
+		if (bk_da[i]->block_type == PCMDEVICE_BIN_BLK_PRE_POWER_UP) {
+			if (bk_da[i]->dev_idx == 0)
+				cfg_info->active_dev =
+					(1 << pcm_dev->ndev) - 1;
+			else
+				cfg_info->active_dev =
+					1 << (bk_da[i]->dev_idx - 1);
+		}
+
+		bk_da[i]->yram_checksum =
+			get_unaligned_be16(&config_data[config_offset]);
+		config_offset += 2;
+		bk_da[i]->block_size =
+			get_unaligned_be32(&config_data[config_offset]);
+		config_offset += 4;
+
+		bk_da[i]->n_subblks =
+			get_unaligned_be32(&config_data[config_offset]);
+
+		config_offset += 4;
+
+		if (config_offset + bk_da[i]->block_size > config_size) {
+			*status = -EINVAL;
+			dev_err(pcm_dev->dev,
+				"%s: out of boundary: i = %d blks = %u\n",
+				__func__, i, cfg_info->nblocks);
+			break;
+		}
+
+		bk_da[i]->regdata = kmemdup(&config_data[config_offset],
+			bk_da[i]->block_size, GFP_KERNEL);
+		if (!bk_da[i]->regdata) {
+			*status = -ENOMEM;
+			goto out;
+		}
+		config_offset += bk_da[i]->block_size;
+		cfg_info->real_nblocks += 1;
+	}
+out:
+	return cfg_info;
+}
+
+static int pcmdev_gain_ctrl_add(struct pcmdevice_priv *pcm_dev,
+	int dev_no, int ctl_id)
+{
+	struct i2c_adapter *adap = pcm_dev->client->adapter;
+	struct snd_soc_component *comp = pcm_dev->component;
+	struct pcmdevice_mixer_control *pcmdev_ctrl;
+	struct snd_kcontrol_new *pcmdev_controls;
+	int ret, mix_index = 0, name_id, chn;
+	unsigned int id = pcm_dev->chip_id;
+	const int nr_chn =
+		pcmdev_gain_ctl_info[id][ctl_id].ctrl_array_size;
+	const char *ctrl_name;
+	char *name;
+
+	if (!nr_chn) {
+		dev_dbg(pcm_dev->dev, "%s: no gain ctrl for %s\n", __func__,
+			pcm_dev->dev_name);
+		return 0;
+	}
+
+	pcmdev_controls = devm_kzalloc(pcm_dev->dev,
+		nr_chn * sizeof(struct snd_kcontrol_new), GFP_KERNEL);
+	if (!pcmdev_controls)
+		return -ENOMEM;
+
+	name_id = pcmdev_gain_ctl_info[id][ctl_id].pcmdev_ctrl_name_id;
+
+	if (comp->name_prefix)
+		ctrl_name = pcmdev_ctrl_name_with_prefix[name_id];
+	else
+		ctrl_name = pcmdev_ctrl_name[name_id];
+
+	for (chn = 1; chn <= nr_chn; chn++) {
+		name = devm_kzalloc(pcm_dev->dev,
+			SNDRV_CTL_ELEM_ID_NAME_MAXLEN, GFP_KERNEL);
+		if (!name) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		if (comp->name_prefix)
+			scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+				ctrl_name, comp->name_prefix, dev_no, chn);
+		else
+			scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+				ctrl_name, pcm_dev->dev_name, adap->nr,
+				dev_no, chn);
+		pcmdev_controls[mix_index].tlv.p =
+			pcmdev_gain_ctl_info[id][ctl_id].gain;
+		pcmdev_ctrl = devm_kmemdup(pcm_dev->dev,
+			&pcmdev_gain_ctl_info[id][ctl_id].pcmdev_ctrl[chn - 1],
+			sizeof(*pcmdev_ctrl), GFP_KERNEL);
+		if (!pcmdev_ctrl) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		pcmdev_ctrl->dev_no = dev_no;
+		pcmdev_controls[mix_index].private_value =
+			(unsigned long)pcmdev_ctrl;
+		pcmdev_controls[mix_index].name = name;
+		pcmdev_controls[mix_index].access =
+			SNDRV_CTL_ELEM_ACCESS_TLV_READ |
+			SNDRV_CTL_ELEM_ACCESS_READWRITE;
+		pcmdev_controls[mix_index].iface =
+			SNDRV_CTL_ELEM_IFACE_MIXER;
+		pcmdev_controls[mix_index].info = pcmdevice_info_volsw;
+		pcmdev_controls[mix_index].get =
+			pcmdev_gain_ctl_info[id][ctl_id].get;
+		pcmdev_controls[mix_index].put =
+			pcmdev_gain_ctl_info[id][ctl_id].put;
+		mix_index++;
+	}
+
+	ret = snd_soc_add_component_controls(comp, pcmdev_controls, mix_index);
+	if (ret)
+		dev_err(pcm_dev->dev, "%s: add_controls err = %d\n",
+			__func__, ret);
+out:
+	return ret;
+}
+
+static int pcmdev_profile_ctrl_add(struct pcmdevice_priv *pcm_dev)
+{
+	struct snd_soc_component *comp = pcm_dev->component;
+	struct i2c_adapter *adap = pcm_dev->client->adapter;
+	struct snd_kcontrol_new *pcmdev_ctrl;
+	char *name;
+	int ret;
+
+	pcmdev_ctrl = devm_kzalloc(pcm_dev->dev,
+		sizeof(struct snd_kcontrol_new), GFP_KERNEL);
+	if (!pcmdev_ctrl)
+		return -ENOMEM;
+
+	/* Create a mixer item for selecting the active profile */
+	name = devm_kzalloc(pcm_dev->dev, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+		GFP_KERNEL);
+	if (!name)
+		return -ENOMEM;
+
+	if (comp->name_prefix)
+		scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+			"%s Profile id", comp->name_prefix);
+	else
+		scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+			"%s-i2c-%d Profile id", pcm_dev->dev_name, adap->nr);
+	pcmdev_ctrl->name = name;
+	pcmdev_ctrl->iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	pcmdev_ctrl->info = pcmdevice_info_profile;
+	pcmdev_ctrl->get = pcmdevice_get_profile_id;
+	pcmdev_ctrl->put = pcmdevice_set_profile_id;
+
+	ret = snd_soc_add_component_controls(comp, pcmdev_ctrl, 1);
+	if (ret)
+		dev_err(pcm_dev->dev, "%s: add_controls err = %d\n",
+			__func__, ret);
+
+	return ret;
+}
+
+static void pcmdevice_config_info_remove(void *ctxt)
+{
+	struct pcmdevice_priv *pcm_dev = (struct pcmdevice_priv *) ctxt;
+	struct pcmdevice_regbin *regbin = &(pcm_dev->regbin);
+	struct pcmdevice_config_info **cfg_info = regbin->cfg_info;
+	int i, j;
+
+	if (!cfg_info)
+		return;
+	for (i = 0; i < regbin->ncfgs; i++) {
+		if (!cfg_info[i])
+			continue;
+		if (cfg_info[i]->blk_data) {
+			for (j = 0; j < (int)cfg_info[i]->real_nblocks; j++) {
+				if (!cfg_info[i]->blk_data[j])
+					continue;
+				kfree(cfg_info[i]->blk_data[j]->regdata);
+				kfree(cfg_info[i]->blk_data[j]);
+			}
+			kfree(cfg_info[i]->blk_data);
+		}
+		kfree(cfg_info[i]);
+	}
+	kfree(cfg_info);
+}
+
+static int pcmdev_regbin_ready(const struct firmware *fmw, void *ctxt)
+{
+	struct pcmdevice_config_info **cfg_info;
+	struct pcmdevice_priv *pcm_dev = ctxt;
+	struct pcmdevice_regbin_hdr *fw_hdr;
+	struct pcmdevice_regbin *regbin;
+	unsigned int total_config_sz = 0;
+	int offset = 0, ret = 0, i;
+	unsigned char *buf;
+
+	regbin = &(pcm_dev->regbin);
+	fw_hdr = &(regbin->fw_hdr);
+	if (!fmw || !fmw->data) {
+		dev_err(pcm_dev->dev, "%s: failed to read %s\n",
+			__func__, pcm_dev->bin_name);
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -EINVAL;
+		goto out;
+	}
+	buf = (unsigned char *)fmw->data;
+
+	fw_hdr->img_sz = get_unaligned_be32(&buf[offset]);
+	offset += 4;
+	if (fw_hdr->img_sz != fmw->size) {
+		dev_err(pcm_dev->dev, "%s: file size(%d) not match %u",
+			__func__, (int)fmw->size, fw_hdr->img_sz);
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -EINVAL;
+		goto out;
+	}
+
+	fw_hdr->checksum = get_unaligned_be32(&buf[offset]);
+	offset += 4;
+	fw_hdr->binary_version_num = get_unaligned_be32(&buf[offset]);
+	if (fw_hdr->binary_version_num < 0x103) {
+		dev_err(pcm_dev->dev, "%s: bin version 0x%04x is out of date",
+			__func__, fw_hdr->binary_version_num);
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -EINVAL;
+		goto out;
+	}
+	offset += 4;
+	fw_hdr->drv_fw_version = get_unaligned_be32(&buf[offset]);
+	offset += 8;
+	fw_hdr->plat_type = buf[offset];
+	offset += 1;
+	fw_hdr->dev_family = buf[offset];
+	offset += 1;
+	fw_hdr->reserve = buf[offset];
+	offset += 1;
+	fw_hdr->ndev = buf[offset];
+	offset += 1;
+	if (fw_hdr->ndev != pcm_dev->ndev) {
+		dev_err(pcm_dev->dev, "%s: invalid ndev(%u)\n", __func__,
+			fw_hdr->ndev);
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (offset + PCMDEVICE_MAX_REGBIN_DEVICES > fw_hdr->img_sz) {
+		dev_err(pcm_dev->dev, "%s: devs out of boundary!\n", __func__);
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -EINVAL;
+		goto out;
+	}
+
+	for (i = 0; i < PCMDEVICE_MAX_REGBIN_DEVICES; i++, offset++)
+		fw_hdr->devs[i] = buf[offset];
+
+	fw_hdr->nconfig = get_unaligned_be32(&buf[offset]);
+	offset += 4;
+
+	for (i = 0; i < PCMDEVICE_CONFIG_SUM; i++) {
+		fw_hdr->config_size[i] = get_unaligned_be32(&buf[offset]);
+		offset += 4;
+		total_config_sz += fw_hdr->config_size[i];
+	}
+
+	if (fw_hdr->img_sz - total_config_sz != (unsigned int)offset) {
+		dev_err(pcm_dev->dev, "%s: bin file error!\n", __func__);
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -EINVAL;
+		goto out;
+	}
+	cfg_info = kcalloc(fw_hdr->nconfig, sizeof(*cfg_info), GFP_KERNEL);
+	if (!cfg_info) {
+		pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+		ret = -ENOMEM;
+		goto out;
+	}
+	regbin->cfg_info = cfg_info;
+	regbin->ncfgs = 0;
+	for (i = 0; i < (int)fw_hdr->nconfig; i++) {
+		cfg_info[i] = pcmdevice_add_config(ctxt, &buf[offset],
+				fw_hdr->config_size[i], &ret);
+		if (ret) {
+			/* In case the bin file is partially destroyed. */
+			if (regbin->ncfgs == 0)
+				pcm_dev->fw_state = PCMDEVICE_FW_LOAD_FAILED;
+			break;
+		}
+		offset += (int)fw_hdr->config_size[i];
+		regbin->ncfgs += 1;
+	}
+
+out:
+	if (pcm_dev->fw_state == PCMDEVICE_FW_LOAD_FAILED) {
+		dev_err(pcm_dev->dev,
+			"%s: remove config due to fw load error!\n", __func__);
+		pcmdevice_config_info_remove(pcm_dev);
+	}
+
+	return ret;
+}
+
+static int pcmdevice_comp_probe(struct snd_soc_component *comp)
+{
+	struct pcmdevice_priv *pcm_dev = snd_soc_component_get_drvdata(comp);
+	struct i2c_adapter *adap = pcm_dev->client->adapter;
+	const struct firmware *fw_entry = NULL;
+	int ret, i, j;
+
+	mutex_lock(&pcm_dev->codec_lock);
+
+	pcm_dev->component = comp;
+
+	for (i = 0; i < pcm_dev->ndev; i++) {
+		for (j = 0; j < 2; j++) {
+			ret = pcmdev_gain_ctrl_add(pcm_dev, i, j);
+			if (ret < 0)
+				goto out;
+		}
+	}
+
+	if (comp->name_prefix) {
+	/* There's name_prefix defined in DTS. Bin file name will be
+	 * name_prefix.bin stores the firmware including register setting and
+	 * params for different filters inside chips, it must be copied into
+	 * firmware folder. The same types of pcmdevices sitting on the same
+	 * i2c bus will be aggregated as one single codec, all of them share
+	 * the same bin file.
+	 */
+		scnprintf(pcm_dev->bin_name, PCMDEVICE_BIN_FILENAME_LEN,
+			"%s.bin", comp->name_prefix);
+	} else {
+	/* There's NO name_prefix defined in DTS. Bin file name will be
+	 * device-name[defined in pcmdevice_i2c_id]-i2c-bus_id[0,1,...,N]-
+	 * sum[1,...,4]dev.bin stores the firmware including register setting
+	 * and params for different filters inside chips, it must be copied
+	 * into firmware folder. The same types of pcmdevices sitting on the
+	 * same i2c bus will be aggregated as one single codec, all of them
+	 * share the same bin file.
+	 */
+		scnprintf(pcm_dev->bin_name, PCMDEVICE_BIN_FILENAME_LEN,
+			"%s-i2c-%d-%udev.bin", pcm_dev->dev_name, adap->nr,
+			pcm_dev->ndev);
+	}
+
+	ret = request_firmware(&fw_entry, pcm_dev->bin_name, pcm_dev->dev);
+	if (ret) {
+		dev_err(pcm_dev->dev, "%s: request %s err = %d\n", __func__,
+			pcm_dev->bin_name, ret);
+		goto out;
+	}
+
+	ret = pcmdev_regbin_ready(fw_entry, pcm_dev);
+	if (ret) {
+		dev_err(pcm_dev->dev, "%s: %s parse err = %d\n", __func__,
+			pcm_dev->bin_name, ret);
+		goto out;
+	}
+	ret = pcmdev_profile_ctrl_add(pcm_dev);
+out:
+	if (fw_entry)
+		release_firmware(fw_entry);
+
+	mutex_unlock(&pcm_dev->codec_lock);
+	return ret;
+}
+
+
+static void pcmdevice_comp_remove(struct snd_soc_component *codec)
+{
+	struct pcmdevice_priv *pcm_dev = snd_soc_component_get_drvdata(codec);
+
+	if (!pcm_dev)
+		return;
+	mutex_lock(&pcm_dev->codec_lock);
+	pcmdevice_config_info_remove(pcm_dev);
+	mutex_unlock(&pcm_dev->codec_lock);
+}
+
+static const struct snd_soc_dapm_widget pcmdevice_dapm_widgets[] = {
+	SND_SOC_DAPM_AIF_IN("ASI", "ASI Playback", 0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("ASI1 OUT", "ASI1 Capture",
+		0, SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_OUTPUT("OUT"),
+	SND_SOC_DAPM_INPUT("MIC"),
+};
+
+static const struct snd_soc_dapm_route pcmdevice_audio_map[] = {
+	{"OUT", NULL, "ASI"},
+	{"ASI1 OUT", NULL, "MIC"},
+};
+
+static const struct snd_soc_component_driver
+	soc_codec_driver_pcmdevice = {
+	.probe			= pcmdevice_comp_probe,
+	.remove			= pcmdevice_comp_remove,
+	.dapm_widgets		= pcmdevice_dapm_widgets,
+	.num_dapm_widgets	= ARRAY_SIZE(pcmdevice_dapm_widgets),
+	.dapm_routes		= pcmdevice_audio_map,
+	.num_dapm_routes	= ARRAY_SIZE(pcmdevice_audio_map),
+	.suspend_bias_off	= 1,
+	.idle_bias_on		= 0,
+	.use_pmdown_time	= 1,
+	.endianness		= 1,
+};
+
+static int pcmdevice_process_block(void *ctxt, unsigned char *data,
+	unsigned char dev_idx, int sublocksize)
+{
+	struct pcmdevice_priv *pcm_dev = (struct pcmdevice_priv *)ctxt;
+	int subblk_offset = 2, chn, chnend, ret;
+	unsigned char subblk_typ = data[1];
+
+	if (dev_idx) {
+		chn = dev_idx - 1;
+		chnend = dev_idx;
+	} else {
+		chn = 0;
+		chnend = pcm_dev->ndev;
+	}
+
+	for (; chn < chnend; chn++) {
+		switch (subblk_typ) {
+		case PCMDEVICE_CMD_SING_W: {
+			unsigned short len = get_unaligned_be16(&data[2]);
+			int i = 0;
+
+			subblk_offset += 2;
+			if (subblk_offset + 4 * len > sublocksize) {
+				dev_err(pcm_dev->dev,
+					"%s: byt wr out of boundary\n",
+					__func__);
+				break;
+			}
+
+			for (i = 0; i < len; i++) {
+				ret = pcmdev_dev_write(pcm_dev, chn,
+					PCMDEVICE_REG(data[subblk_offset + 1],
+						data[subblk_offset + 2]),
+					data[subblk_offset + 3]);
+				if (ret < 0)
+					dev_err(pcm_dev->dev,
+						"%s: single write error\n",
+						__func__);
+
+				subblk_offset += 4;
+			}
+		}
+		break;
+		case PCMDEVICE_CMD_BURST: {
+			unsigned short len = get_unaligned_be16(&data[2]);
+
+			subblk_offset += 2;
+			if (subblk_offset + 4 + len > sublocksize) {
+				dev_err(pcm_dev->dev,
+					"%s: burst Out of boundary\n",
+					__func__);
+				break;
+			}
+			if (len % 4) {
+				dev_err(pcm_dev->dev,
+					"%s: burst-len(%u) not div by 4\n",
+					__func__, len);
+				break;
+			}
+			ret = pcmdev_dev_bulk_write(pcm_dev, chn,
+				PCMDEVICE_REG(data[subblk_offset + 1],
+				data[subblk_offset + 2]),
+				&(data[subblk_offset + 4]), len);
+			if (ret < 0)
+				dev_err(pcm_dev->dev,
+					"%s: bulk_write err = %d\n",
+					__func__, ret);
+
+			subblk_offset += (len + 4);
+		}
+			break;
+		case PCMDEVICE_CMD_DELAY: {
+			unsigned int delay_time = 0;
+
+			if (subblk_offset + 2 > sublocksize) {
+				dev_err(pcm_dev->dev,
+					"%s: deley out of boundary\n",
+					__func__);
+				break;
+			}
+			delay_time = get_unaligned_be16(&data[2]) * 1000;
+			usleep_range(delay_time, delay_time + 50);
+			subblk_offset += 2;
+		}
+			break;
+		case PCMDEVICE_CMD_FIELD_W:
+		if (subblk_offset + 6 > sublocksize) {
+			dev_err(pcm_dev->dev,
+				"%s: bit write out of memory\n", __func__);
+			break;
+		}
+			ret = pcmdev_dev_update_bits(pcm_dev, chn,
+				PCMDEVICE_REG(data[subblk_offset + 3],
+				data[subblk_offset + 4]),
+				data[subblk_offset + 1],
+				data[subblk_offset + 5]);
+		if (ret < 0)
+			dev_err(pcm_dev->dev, "%s: update_bits err = %d\n",
+				__func__, ret);
+
+		subblk_offset += 6;
+			break;
+		default:
+			break;
+		}
+	}
+
+	return subblk_offset;
+}
+
+static void pcmdevice_select_cfg_blk(void *ctxt, int conf_no,
+	unsigned char block_type)
+{
+	struct pcmdevice_priv *pcm_dev = (struct pcmdevice_priv *)ctxt;
+	struct pcmdevice_regbin *regbin = &(pcm_dev->regbin);
+	struct pcmdevice_config_info **cfg_info = regbin->cfg_info;
+	struct pcmdevice_block_data **blk_data;
+	int j, k;
+
+	if (conf_no >= regbin->ncfgs || conf_no < 0 || NULL == cfg_info) {
+		dev_err(pcm_dev->dev, "%s: conf_no should be less than %u\n",
+			__func__, regbin->ncfgs);
+		goto out;
+	}
+	blk_data = cfg_info[conf_no]->blk_data;
+
+	for (j = 0; j < (int)cfg_info[conf_no]->real_nblocks; j++) {
+		unsigned int length = 0, ret;
+
+		if (block_type > 5 || block_type < 2) {
+			dev_err(pcm_dev->dev,
+				"%s: block_type should be out of range\n",
+				__func__);
+			goto out;
+		}
+		if (block_type != blk_data[j]->block_type)
+			continue;
+
+		for (k = 0; k < (int)blk_data[j]->n_subblks; k++) {
+			ret = pcmdevice_process_block(pcm_dev,
+				blk_data[j]->regdata + length,
+				blk_data[j]->dev_idx,
+				blk_data[j]->block_size - length);
+			length += ret;
+			if (blk_data[j]->block_size < length) {
+				dev_err(pcm_dev->dev,
+					"%s: %u %u out of boundary\n",
+					__func__, length,
+					blk_data[j]->block_size);
+				break;
+			}
+		}
+		if (length != blk_data[j]->block_size)
+			dev_err(pcm_dev->dev, "%s: %u %u size is not same\n",
+				__func__, length, blk_data[j]->block_size);
+	}
+
+out:
+	return;
+}
+
+static int pcmdevice_mute(struct snd_soc_dai *dai, int mute, int stream)
+{
+	struct snd_soc_component *codec = dai->component;
+	struct pcmdevice_priv *pcm_dev = snd_soc_component_get_drvdata(codec);
+	unsigned char block_type;
+
+	if (pcm_dev->fw_state == PCMDEVICE_FW_LOAD_FAILED) {
+		dev_err(pcm_dev->dev, "%s: bin file not loaded\n", __func__);
+		return -EINVAL;
+	}
+
+	if (mute)
+		block_type = PCMDEVICE_BIN_BLK_PRE_SHUTDOWN;
+	else
+		block_type = PCMDEVICE_BIN_BLK_PRE_POWER_UP;
+
+	mutex_lock(&pcm_dev->codec_lock);
+	pcmdevice_select_cfg_blk(pcm_dev, pcm_dev->cur_conf, block_type);
+	mutex_unlock(&pcm_dev->codec_lock);
+	return 0;
+}
+
+static int pcmdevice_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *params, struct snd_soc_dai *dai)
+{
+	struct pcmdevice_priv *pcm_dev = snd_soc_dai_get_drvdata(dai);
+	unsigned int fsrate;
+	unsigned int slot_width;
+	int bclk_rate;
+	int ret = 0;
+
+	fsrate = params_rate(params);
+	switch (fsrate) {
+	case 48000:
+		break;
+	case 44100:
+		break;
+	default:
+		dev_err(pcm_dev->dev, "%s: incorrect sample rate = %u\n",
+			__func__, fsrate);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	slot_width = params_width(params);
+	switch (slot_width) {
+	case 16:
+		break;
+	case 20:
+		break;
+	case 24:
+		break;
+	case 32:
+		break;
+	default:
+		dev_err(pcm_dev->dev, "%s: incorrect slot width = %u\n",
+			__func__, slot_width);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	bclk_rate = snd_soc_params_to_bclk(params);
+	if (bclk_rate < 0) {
+		dev_err(pcm_dev->dev, "%s: incorrect bclk rate = %d\n",
+			__func__, bclk_rate);
+		ret = bclk_rate;
+	}
+
+out:
+	return ret;
+}
+
+static const struct snd_soc_dai_ops pcmdevice_dai_ops = {
+	.mute_stream = pcmdevice_mute,
+	.hw_params = pcmdevice_hw_params,
+};
+
+static struct snd_soc_dai_driver pcmdevice_dai_driver[] = {
+	{
+		.name = "pcmdevice-codec",
+		.capture = {
+			.stream_name	 = "Capture",
+			.channels_min	 = 2,
+			.channels_max	 = PCMDEVICE_MAX_CHANNELS,
+			.rates		 = PCMDEVICE_RATES,
+			.formats	 = PCMDEVICE_FORMATS,
+		},
+		.playback = {
+			.stream_name	 = "Playback",
+			.channels_min	 = 2,
+			.channels_max	 = PCMDEVICE_MAX_CHANNELS,
+			.rates		 = PCMDEVICE_RATES,
+			.formats	 = PCMDEVICE_FORMATS,
+		},
+		.ops = &pcmdevice_dai_ops,
+		.symmetric_rate = 1,
+	}
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id pcmdevice_of_match[] = {
+	{ .compatible = "ti,adc3120"  },
+	{ .compatible = "ti,adc5120"  },
+	{ .compatible = "ti,adc6120"  },
+	{ .compatible = "ti,dix4192"  },
+	{ .compatible = "ti,pcm1690"  },
+	{ .compatible = "ti,pcm3120"  },
+	{ .compatible = "ti,pcm3140"  },
+	{ .compatible = "ti,pcm5120"  },
+	{ .compatible = "ti,pcm5140"  },
+	{ .compatible = "ti,pcm6120"  },
+	{ .compatible = "ti,pcm6140"  },
+	{ .compatible = "ti,pcm6240"  },
+	{ .compatible = "ti,pcm6260"  },
+	{ .compatible = "ti,pcm9211"  },
+	{ .compatible = "ti,pcmd3140" },
+	{ .compatible = "ti,pcmd3180" },
+	{ .compatible = "ti,pcmd512x" },
+	{ .compatible = "ti,taa5212"  },
+	{ .compatible = "ti,taa5412"  },
+	{ .compatible = "ti,tad5212"  },
+	{ .compatible = "ti,tad5412"  },
+	{},
+};
+MODULE_DEVICE_TABLE(of, pcmdevice_of_match);
+#endif
+
+static const struct regmap_range_cfg pcmdevice_ranges[] = {
+	{
+		.range_min = 0,
+		.range_max = 256 * 128,
+		.selector_reg = PCMDEVICE_PAGE_SELECT,
+		.selector_mask = 0xff,
+		.selector_shift = 0,
+		.window_start = 0,
+		.window_len = 128,
+	},
+};
+
+static const struct regmap_config pcmdevice_i2c_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.cache_type = REGCACHE_MAPLE,
+	.ranges = pcmdevice_ranges,
+	.num_ranges = ARRAY_SIZE(pcmdevice_ranges),
+	.max_register = 256 * 128,
+};
+
+static void pcmdevice_remove(struct pcmdevice_priv *pcm_dev)
+{
+	if (gpio_is_valid(pcm_dev->irq_info.gpio)) {
+		gpio_free(pcm_dev->irq_info.gpio);
+		free_irq(pcm_dev->irq_info.nmb, pcm_dev);
+	}
+	mutex_destroy(&pcm_dev->codec_lock);
+}
+
+static int pcmdevice_i2c_probe(struct i2c_client *i2c)
+{
+	const struct i2c_device_id *id = i2c_match_id(pcmdevice_i2c_id, i2c);
+	struct pcmdevice_priv *pcm_dev;
+	struct device_node *np;
+	unsigned int dev_addrs[PCMDEVICE_MAX_I2C_DEVICES];
+	int ret = 0, i = 0, ndev = 0;
+#ifdef CONFIG_OF
+	const __be32 *reg, *reg_end;
+	int len, sw, aw;
+#endif
+
+	pcm_dev = devm_kzalloc(&i2c->dev, sizeof(*pcm_dev), GFP_KERNEL);
+	if (!pcm_dev) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	pcm_dev->chip_id = (id != NULL) ? id->driver_data : 0;
+
+	pcm_dev->dev = &i2c->dev;
+	pcm_dev->client = i2c;
+
+	if (pcm_dev->chip_id >= MAX_DEVICE)
+		pcm_dev->chip_id = 0;
+
+	strscpy(pcm_dev->dev_name, pcmdevice_i2c_id[pcm_dev->chip_id].name,
+		sizeof(pcm_dev->dev_name));
+
+	pcm_dev->regmap = devm_regmap_init_i2c(i2c, &pcmdevice_i2c_regmap);
+	if (IS_ERR(pcm_dev->regmap)) {
+		ret = PTR_ERR(pcm_dev->regmap);
+		dev_err(&i2c->dev, "%s: failed to allocate register map: %d\n",
+			__func__, ret);
+		goto out;
+	}
+
+	i2c_set_clientdata(i2c, pcm_dev);
+	mutex_init(&pcm_dev->codec_lock);
+	np = pcm_dev->dev->of_node;
+#ifdef CONFIG_OF
+	aw = of_n_addr_cells(np);
+	sw = of_n_size_cells(np);
+	if (sw == 0) {
+		reg = (const __be32 *)of_get_property(np,
+			"reg", &len);
+		reg_end = reg + len/sizeof(*reg);
+		ndev = 0;
+		do {
+			dev_addrs[ndev] = of_read_number(reg, aw);
+			reg += aw;
+			ndev++;
+		} while (reg < reg_end);
+	} else {
+		ndev = 1;
+		dev_addrs[0] = i2c->addr;
+	}
+#else
+	ndev = 1;
+	dev_addrs[0] = i2c->addr;
+#endif
+	pcm_dev->irq_info.gpio = of_irq_get(np, 0);
+
+	for (i = 0; i < ndev; i++)
+		pcm_dev->addr[i] = dev_addrs[i];
+
+	pcm_dev->ndev = ndev;
+
+	pcm_dev->hw_rst = devm_gpiod_get_optional(&i2c->dev,
+			"reset-gpios", GPIOD_OUT_HIGH);
+	/* No reset GPIO, no side-effect */
+	if (IS_ERR(pcm_dev->hw_rst)) {
+		if (pcm_dev->chip_id == PCM9211 || pcm_dev->chip_id == PCM1690)
+			pcm9211_sw_rst(pcm_dev);
+		else
+			pcmdevice_sw_rst(pcm_dev);
+	} else {
+		gpiod_set_value_cansleep(pcm_dev->hw_rst, 0);
+		usleep_range(500, 1000);
+		gpiod_set_value_cansleep(pcm_dev->hw_rst, 1);
+	}
+
+	if (pcm_dev->chip_id == PCM1690)
+		goto skip_interrupt;
+	if (gpio_is_valid(pcm_dev->irq_info.gpio)) {
+		dev_dbg(pcm_dev->dev, "irq-gpio = %d", pcm_dev->irq_info.gpio);
+
+		ret = gpio_request(pcm_dev->irq_info.gpio, "PCMDEV-IRQ");
+		if (!ret) {
+			int gpio = pcm_dev->irq_info.gpio;
+
+			gpio_direction_input(gpio);
+			pcm_dev->irq_info.nmb = gpio_to_irq(gpio);
+
+		} else
+			dev_err(pcm_dev->dev, "%s: GPIO %d request error\n",
+				__func__, pcm_dev->irq_info.gpio);
+	} else
+		dev_err(pcm_dev->dev, "Looking up irq-gpio failed %d\n",
+			pcm_dev->irq_info.gpio);
+
+skip_interrupt:
+	ret = devm_snd_soc_register_component(&i2c->dev,
+		&soc_codec_driver_pcmdevice, pcmdevice_dai_driver,
+		ARRAY_SIZE(pcmdevice_dai_driver));
+	if (ret < 0)
+		dev_err(&i2c->dev, "probe register comp failed %d\n", ret);
+
+out:
+	if (ret < 0)
+		pcmdevice_remove(pcm_dev);
+	return ret;
+}
+
+static void pcmdevice_i2c_remove(struct i2c_client *i2c)
+{
+	struct pcmdevice_priv *pcm_dev = i2c_get_clientdata(i2c);
+
+	pcmdevice_remove(pcm_dev);
+}
+
+static struct i2c_driver pcmdevice_i2c_driver = {
+	.driver = {
+		.name = "pcmdevice-codec",
+		.of_match_table = of_match_ptr(pcmdevice_of_match),
+	},
+	.probe	= pcmdevice_i2c_probe,
+	.remove = pcmdevice_i2c_remove,
+	.id_table = pcmdevice_i2c_id,
+};
+module_i2c_driver(pcmdevice_i2c_driver);
+
+MODULE_AUTHOR("Shenghao Ding <shenghao-ding@ti.com>");
+MODULE_DESCRIPTION("ASoC PCM6240 Family Audio ADC/DAC Driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1


^ permalink raw reply related

* [PATCH v2] dt-bindings: usb: qcom,pmic-typec: update example to follow connector schema
From: Dmitry Baryshkov @ 2024-03-30 22:21 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Bryan O'Donoghue,
	Greg Kroah-Hartman, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-msm, linux-usb, devicetree, linux-kernel, Luca Weiss,
	Dmitry Baryshkov

Update Qualcomm PMIC Type-C examples to follow the USB-C connector
schema. The USB-C connector should have three ports (USB HS @0,
SSTX/RX @1 and SBU @2 lanes). Reorder ports accordingly and add SBU port
connected to the SBU mux (e.g. FSA4480).

Fixes: 00bb478b829e ("dt-bindings: usb: Add Qualcomm PMIC Type-C")
Reported-by: Luca Weiss <luca.weiss@fairphone.com>
Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Update examples to follow usb-c-connector schema wrt. ports definitions.
---
Changes in v2:
- Dropped patch removing the role-switching port. We do not use it, but
  it can become useful by other designs.
- Link to v1: https://lore.kernel.org/r/20240322-typec-fix-example-v1-0-6b01c347419e@linaro.org
---
 .../devicetree/bindings/usb/qcom,pmic-typec.yaml   | 34 +++++++++++++++++-----
 1 file changed, 26 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
index d9694570c419..fe757619e535 100644
--- a/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
+++ b/Documentation/devicetree/bindings/usb/qcom,pmic-typec.yaml
@@ -192,15 +192,22 @@ examples:
 
                     port@0 {
                         reg = <0>;
-                        pmic_typec_mux_out: endpoint {
-                            remote-endpoint = <&usb_phy_typec_mux_in>;
+                        pmic_typec_hs_in: endpoint {
+                            remote-endpoint = <&usb_hs_out>;
                         };
                     };
 
                     port@1 {
                         reg = <1>;
-                        pmic_typec_role_switch_out: endpoint {
-                            remote-endpoint = <&usb_role_switch_in>;
+                        pmic_typec_ss_in: endpoint {
+                            remote-endpoint = <&usb_phy_typec_ss_out>;
+                        };
+                    };
+
+                    port@2 {
+                        reg = <2>;
+                        pmic_typec_sbu: endpoint {
+                            remote-endpoint = <&usb_mux_sbu>;
                         };
                     };
                 };
@@ -212,8 +219,8 @@ examples:
         dr_mode = "otg";
         usb-role-switch;
         port {
-            usb_role_switch_in: endpoint {
-                remote-endpoint = <&pmic_typec_role_switch_out>;
+            usb_hs_out: endpoint {
+                remote-endpoint = <&pmic_typec_hs_in>;
             };
         };
     };
@@ -221,8 +228,19 @@ examples:
     usb-phy {
         orientation-switch;
         port {
-            usb_phy_typec_mux_in: endpoint {
-                remote-endpoint = <&pmic_typec_mux_out>;
+            usb_phy_typec_ss_out: endpoint {
+                remote-endpoint = <&pmic_typec_ss_in>;
+            };
+        };
+    };
+
+    usb-mux {
+        orientation-switch;
+        mode-switch;
+
+        port {
+            usb_mux_sbu: endpoint {
+                remote-endpoint = <&pmic_typec_sbu>;
             };
         };
     };

---
base-commit: 13ee4a7161b6fd938aef6688ff43b163f6d83e37
change-id: 20240322-typec-fix-example-3d9b1eca853d

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


^ permalink raw reply related

* Re: [PATCH v2 1/3] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
From: Marc Gonzalez @ 2024-03-30 22:04 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Kalle Valo, Jeff Johnson, ath10k
  Cc: wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Konrad Dybcio,
	Jami Kettunen, Jeffrey Hugo
In-Reply-To: <6bd3db0f-2e18-4ad1-abc2-f59c6acc8037@linaro.org>

On 30/03/2024 19:23, Krzysztof Kozlowski wrote:

> On 30/03/2024 19:20, Krzysztof Kozlowski wrote:
> 
>> On 28/03/2024 18:36, Marc Gonzalez wrote:
>> 
>>> The ath10k driver waits for an "MSA_READY" indicator
>>> to complete initialization. If the indicator is not
>>> received, then the device remains unusable.
>>>
>>> cf. ath10k_qmi_driver_event_work()
>>>
>>> Several msm8998-based devices are affected by this issue.
>>> Oddly, it seems safe to NOT wait for the indicator, and
>>> proceed immediately when QMI_EVENT_SERVER_ARRIVE.
>>
>> This is v2, so where is the changelog?
> 
> Expecting reviewer to dig previous discussions will not help your case.
> It helps reviewers if you provide necessary information, like resolution
> of previous discussion in the changelog.
> 
> I dig the previous discussion, since you did not mention it here, and it
> seems you entirely ignored its outcome. That's not a DT property.
> 
> NAK, sorry. Please go back to v1 and read the comments you got there.

My apologies for omitting the changelog.

And I don't blame you for missing the thread's resolution,
since I made a bit of a mess of it with my various messages.

The firmware-5.bin approach was deemed DOA since these files
are parsed too late with respect to the required work-around.
Thus, we went back to either DT or a to-be-written system used
in the vendor driver.

Jeff Johnson (one of the maintainers) concluded with:
"But I'm OK with the DT option as well. Kalle?"

Thus, I spun v2 to get Kalle's Ack, and more crucially to give
a heads-up to the msm8998 users my patch would impact.

Regards


^ permalink raw reply

* Re: [PATCH 2/4] dt-bindings: mfd: x-powers,axp152: add boost regulator
From: Andre Przywara @ 2024-03-30 21:40 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Chen-Yu Tsai, Lee Jones, Liam Girdwood, Mark Brown, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, devicetree, linux-kernel,
	linux-sunxi, Jernej Skrabec, Samuel Holland, Ryan Walklin,
	Chris Morgan
In-Reply-To: <0b53cf44-b5ca-4ccc-9912-777b85f1bcbd@linaro.org>

On Sat, 30 Mar 2024 10:30:05 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

Hi,

> On 30/03/2024 00:50, Andre Przywara wrote:
> > The X-Powers AXP717 contains a boost regulator, that it meant to provide
> > the 5V USB VBUS voltage when the devices operates on battery.
> > 
> > Add the name "boost" to the regexp describing the allowed node names,
> > to allow the regulator to be described in the devicetree.
> > 
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> >  Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> > index b8e8db0d58e9c..14ab367fc8871 100644
> > --- a/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> > +++ b/Documentation/devicetree/bindings/mfd/x-powers,axp152.yaml
> > @@ -274,7 +274,7 @@ properties:
> >            Defines the work frequency of DC-DC in kHz.
> >  
> >      patternProperties:
> > -      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo)$":
> > +      "^(([a-f])?ldo[0-9]|dcdc[0-7a-e]|ldo(_|-)io(0|1)|(dc1)?sw|rtc(_|-)ldo|cpusldo|drivevbus|dc5ldo|boost)$":  
> 
> That's not an easy to read regex...

TBH regexps are the least of my problems when reading bindings ;-)

> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Thanks!

> If driver does not depend on _, please consider dropping (_|-).

The drivers (checked both Linux and FreeBSD) do look for a specific
string, but it's the real old regulators that used ldo_io[01] and
rtc_ldo, all the "newer" ones use a dash. Since this binding covers all
of them, we can't drop this from this regexp, but rest assured we only
go with dashes for new and upcoming devices.

Thanks,
Andre

^ permalink raw reply

* Re: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings
From: kernel test robot @ 2024-03-30 20:59 UTC (permalink / raw)
  To: git, linux-media
  Cc: oe-kbuild-all, dave.stevenson, jacopo.mondi, mchehab, robh,
	krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
	festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
	linux-kernel, Luigi311
In-Reply-To: <20240327231710.53188-19-git@luigi311.com>

Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on media-tree/master]
[also build test WARNING on linuxtv-media-stage/master linus/master v6.9-rc1 next-20240328]
[cannot apply to sailus-media-tree/streams]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/git-luigi311-com/media-i2c-imx258-Remove-unused-defines/20240328-072629
base:   git://linuxtv.org/media_tree.git master
patch link:    https://lore.kernel.org/r/20240327231710.53188-19-git%40luigi311.com
patch subject: [PATCH 18/23] dt-bindings: media: imx258: Add alternate compatible strings
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240331/202403310442.vdZu7Yh6-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403310442.vdZu7Yh6-lkp@intel.com/

dtcheck warnings: (new ones prefixed by >>)
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
   	from schema $id: http://json-schema.org/draft-07/schema#
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: properties:compatible: [{'enum': ['sony,imx258', 'sony,imx258-pdaf']}] is not of type 'object', 'boolean'
   	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
--
>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml: ignoring, error in schema: properties: compatible

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [RFC PATCH v2 3/5] dt-bindings: clock: meson: document A1 SoC audio clock controller driver
From: Jan Dakinevich @ 2024-03-30 19:42 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: Neil Armstrong, Michael Turquette, Stephen Boyd, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Kevin Hilman,
	Martin Blumenstingl, Philipp Zabel, linux-amlogic, linux-clk,
	devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <1j4jcp9qdl.fsf@starbuckisacylon.baylibre.com>



On 3/29/24 15:24, Jerome Brunet wrote:
> 
> On Thu 28 Mar 2024 at 04:08, Jan Dakinevich <jan.dakinevich@salutedevices.com> wrote:
> 
>> Add device tree bindings for A1 SoC audio clock and reset controllers.
>>
>> Signed-off-by: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> ---
>>  .../bindings/clock/amlogic,a1-audio-clkc.yaml | 141 ++++++++++++++++++
>>  .../dt-bindings/clock/amlogic,a1-audio-clkc.h | 122 +++++++++++++++
>>  .../reset/amlogic,meson-a1-audio-reset.h      |  29 ++++
>>  3 files changed, 292 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/amlogic,a1-audio-clkc.yaml
>>  create mode 100644 include/dt-bindings/clock/amlogic,a1-audio-clkc.h
>>  create mode 100644 include/dt-bindings/reset/amlogic,meson-a1-audio-reset.h
>>
>> diff --git a/Documentation/devicetree/bindings/clock/amlogic,a1-audio-clkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a1-audio-clkc.yaml
>> new file mode 100644
>> index 000000000000..1c9ef3292f3c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/amlogic,a1-audio-clkc.yaml
>> @@ -0,0 +1,141 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/clock/amlogic,a1-audio-clkc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic A1 Audio Clock Control Unit and Reset Controller
>> +
>> +maintainers:
>> +  - Neil Armstrong <neil.armstrong@linaro.org>
>> +  - Jerome Brunet <jbrunet@baylibre.com>
>> +  - Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - amlogic,a1-audio-clkc
>> +      - amlogic,a1-audio2-clkc
>> +
>> +  '#clock-cells':
>> +    const: 1
>> +
>> +  '#reset-cells':
>> +    const: 1
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    minItems: 6
>> +    maxItems: 7
>> +
>> +  clock-names:
>> +    minItems: 6
>> +    maxItems: 7
>> +
>> +required:
>> +  - compatible
>> +  - '#clock-cells'
>> +  - reg
>> +  - clocks
>> +  - clock-names
>> +
>> +allOf:
>> +  - if:
>> +      properties:
>> +        compatible:
>> +          contains:
>> +            enum:
>> +              - amlogic,a1-audio-clkc
>> +    then:
>> +      properties:
>> +        clocks:
>> +          items:
>> +            - description: input core clock
>> +            - description: input main peripheral bus clock
>> +            - description: input dds_in
>> +            - description: input fixed pll div2
>> +            - description: input fixed pll div3
>> +            - description: input hifi_pll
>> +            - description: input oscillator (usually at 24MHz)
>> +        clocks-names:
>> +          items:
>> +            - const: core
>> +            - const: pclk
>> +            - const: dds_in
>> +            - const: fclk_div2
>> +            - const: fclk_div3
>> +            - const: hifi_pll
>> +            - const: xtal
>> +      required:
>> +        - '#reset-cells'
>> +    else:
>> +      properties:
>> +        clocks:
>> +          items:
>> +            - description: input main peripheral bus clock
>> +            - description: input dds_in
>> +            - description: input fixed pll div2
>> +            - description: input fixed pll div3
>> +            - description: input hifi_pll
>> +            - description: input oscillator (usually at 24MHz)
>> +        clock-names:
>> +          items:
>> +            - const: pclk
>> +            - const: dds_in
>> +            - const: fclk_div2
>> +            - const: fclk_div3
>> +            - const: hifi_pll
>> +            - const: xtal
> 
> All the optional slave clock inputs are missing from this dcoumentation..
> They need to be documentated as well
> 
Hmmm... here should be a list of 26 entries. First 6 of them are
mandatory, other are optional.

I have no ideas left on how to describe it in yaml, also I can't find
something similar beside existing schemas... Maybe you can give some advice.

>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/clock/amlogic,a1-pll-clkc.h>
>> +    #include <dt-bindings/clock/amlogic,a1-peripherals-clkc.h>
>> +    #include <dt-bindings/clock/amlogic,a1-audio-clkc.h>
>> +    audio {
>> +        #address-cells = <2>;
>> +        #size-cells = <2>;
>> +
>> +        clkc_audio: audio-clock-controller@fe050000 {
>> +                compatible = "amlogic,a1-audio-clkc";
>> +                reg = <0x0 0xfe050000 0x0 0xb0>;
>> +                #clock-cells = <1>;
>> +                #reset-cells = <1>;
>> +                clocks = <&clkc_audio2 AUD2_CLKID_AUDIOTOP>,
>> +                         <&clkc_periphs CLKID_AUDIO>,
>> +                         <&clkc_periphs CLKID_DDS_IN>,
>> +                         <&clkc_pll CLKID_FCLK_DIV2>,
>> +                         <&clkc_pll CLKID_FCLK_DIV3>,
>> +                         <&clkc_pll CLKID_HIFI_PLL>,
>> +                         <&xtal>;
>> +                clock-names = "core",
>> +                              "pclk",
>> +                              "dds_in",
>> +                              "fclk_div2",
>> +                              "fclk_div3",
>> +                              "hifi_pll",
>> +                              "xtal";
>> +        };
>> +
>> +        clkc_audio2: audio-clock-controller@fe054800 {
>> +                compatible = "amlogic,a1-audio2-clkc";
>> +                reg = <0x0 0xfe054800 0x0 0x20>;
>> +                #clock-cells = <1>;
>> +                clocks = <&clkc_periphs CLKID_AUDIO>,
>> +                         <&clkc_periphs CLKID_DDS_IN>,
>> +                         <&clkc_pll CLKID_FCLK_DIV2>,
>> +                         <&clkc_pll CLKID_FCLK_DIV3>,
>> +                         <&clkc_pll CLKID_HIFI_PLL>,
>> +                         <&xtal>;
>> +                clock-names = "pclk",
>> +                              "dds_in",
>> +                              "fclk_div2",
>> +                              "fclk_div3",
>> +                              "hifi_pll",
>> +                              "xtal";
>> +        };
>> +    };
>> diff --git a/include/dt-bindings/clock/amlogic,a1-audio-clkc.h b/include/dt-bindings/clock/amlogic,a1-audio-clkc.h
>> new file mode 100644
>> index 000000000000..b30df3b1ae08
>> --- /dev/null
>> +++ b/include/dt-bindings/clock/amlogic,a1-audio-clkc.h
>> @@ -0,0 +1,122 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>> +/*
>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>> + *
>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> + */
>> +
>> +#ifndef __A1_AUDIO_CLKC_BINDINGS_H
>> +#define __A1_AUDIO_CLKC_BINDINGS_H
>> +
>> +#define AUD_CLKID_DDR_ARB		1
>> +#define AUD_CLKID_TDMIN_A		2
>> +#define AUD_CLKID_TDMIN_B		3
>> +#define AUD_CLKID_TDMIN_LB		4
>> +#define AUD_CLKID_LOOPBACK		5
>> +#define AUD_CLKID_TDMOUT_A		6
>> +#define AUD_CLKID_TDMOUT_B		7
>> +#define AUD_CLKID_FRDDR_A		8
>> +#define AUD_CLKID_FRDDR_B		9
>> +#define AUD_CLKID_TODDR_A		10
>> +#define AUD_CLKID_TODDR_B		11
>> +#define AUD_CLKID_SPDIFIN		12
>> +#define AUD_CLKID_RESAMPLE		13
>> +#define AUD_CLKID_EQDRC			14
>> +#define AUD_CLKID_LOCKER		15
>> +#define AUD_CLKID_MST_A_MCLK_SEL	16
>> +#define AUD_CLKID_MST_A_MCLK_DIV	17
>> +#define AUD_CLKID_MST_A_MCLK		18
>> +#define AUD_CLKID_MST_B_MCLK_SEL	19
>> +#define AUD_CLKID_MST_B_MCLK_DIV	20
>> +#define AUD_CLKID_MST_B_MCLK		21
>> +#define AUD_CLKID_MST_C_MCLK_SEL	22
>> +#define AUD_CLKID_MST_C_MCLK_DIV	23
>> +#define AUD_CLKID_MST_C_MCLK		24
>> +#define AUD_CLKID_MST_D_MCLK_SEL	25
>> +#define AUD_CLKID_MST_D_MCLK_DIV	26
>> +#define AUD_CLKID_MST_D_MCLK		27
>> +#define AUD_CLKID_SPDIFIN_CLK_SEL	28
>> +#define AUD_CLKID_SPDIFIN_CLK_DIV	29
>> +#define AUD_CLKID_SPDIFIN_CLK		30
>> +#define AUD_CLKID_RESAMPLE_CLK_SEL	31
>> +#define AUD_CLKID_RESAMPLE_CLK_DIV	32
>> +#define AUD_CLKID_RESAMPLE_CLK		33
>> +#define AUD_CLKID_LOCKER_IN_CLK_SEL	34
>> +#define AUD_CLKID_LOCKER_IN_CLK_DIV	35
>> +#define AUD_CLKID_LOCKER_IN_CLK		36
>> +#define AUD_CLKID_LOCKER_OUT_CLK_SEL	37
>> +#define AUD_CLKID_LOCKER_OUT_CLK_DIV	38
>> +#define AUD_CLKID_LOCKER_OUT_CLK	39
>> +#define AUD_CLKID_EQDRC_CLK_SEL		40
>> +#define AUD_CLKID_EQDRC_CLK_DIV		41
>> +#define AUD_CLKID_EQDRC_CLK		42
>> +#define AUD_CLKID_MST_A_SCLK_PRE_EN	43
>> +#define AUD_CLKID_MST_A_SCLK_DIV	44
>> +#define AUD_CLKID_MST_A_SCLK_POST_EN	45
>> +#define AUD_CLKID_MST_A_SCLK		46
>> +#define AUD_CLKID_MST_B_SCLK_PRE_EN	47
>> +#define AUD_CLKID_MST_B_SCLK_DIV	48
>> +#define AUD_CLKID_MST_B_SCLK_POST_EN	49
>> +#define AUD_CLKID_MST_B_SCLK		50
>> +#define AUD_CLKID_MST_C_SCLK_PRE_EN	51
>> +#define AUD_CLKID_MST_C_SCLK_DIV	52
>> +#define AUD_CLKID_MST_C_SCLK_POST_EN	53
>> +#define AUD_CLKID_MST_C_SCLK		54
>> +#define AUD_CLKID_MST_D_SCLK_PRE_EN	55
>> +#define AUD_CLKID_MST_D_SCLK_DIV	56
>> +#define AUD_CLKID_MST_D_SCLK_POST_EN	57
>> +#define AUD_CLKID_MST_D_SCLK		58
>> +#define AUD_CLKID_MST_A_LRCLK_DIV	59
>> +#define AUD_CLKID_MST_A_LRCLK		60
>> +#define AUD_CLKID_MST_B_LRCLK_DIV	61
>> +#define AUD_CLKID_MST_B_LRCLK		62
>> +#define AUD_CLKID_MST_C_LRCLK_DIV	63
>> +#define AUD_CLKID_MST_C_LRCLK		64
>> +#define AUD_CLKID_MST_D_LRCLK_DIV	65
>> +#define AUD_CLKID_MST_D_LRCLK		66
>> +#define AUD_CLKID_TDMIN_A_SCLK_SEL	67
>> +#define AUD_CLKID_TDMIN_A_SCLK_PRE_EN	68
>> +#define AUD_CLKID_TDMIN_A_SCLK_POST_EN	69
>> +#define AUD_CLKID_TDMIN_A_SCLK		70
>> +#define AUD_CLKID_TDMIN_A_LRCLK		71
>> +#define AUD_CLKID_TDMIN_B_SCLK_SEL	72
>> +#define AUD_CLKID_TDMIN_B_SCLK_PRE_EN	73
>> +#define AUD_CLKID_TDMIN_B_SCLK_POST_EN	74
>> +#define AUD_CLKID_TDMIN_B_SCLK		75
>> +#define AUD_CLKID_TDMIN_B_LRCLK		76
>> +#define AUD_CLKID_TDMIN_LB_SCLK_SEL	77
>> +#define AUD_CLKID_TDMIN_LB_SCLK_PRE_EN	78
>> +#define AUD_CLKID_TDMIN_LB_SCLK_POST_EN	79
>> +#define AUD_CLKID_TDMIN_LB_SCLK		80
>> +#define AUD_CLKID_TDMIN_LB_LRCLK	81
>> +#define AUD_CLKID_TDMOUT_A_SCLK_SEL	82
>> +#define AUD_CLKID_TDMOUT_A_SCLK_PRE_EN	83
>> +#define AUD_CLKID_TDMOUT_A_SCLK_POST_EN	84
>> +#define AUD_CLKID_TDMOUT_A_SCLK		85
>> +#define AUD_CLKID_TDMOUT_A_LRCLK	86
>> +#define AUD_CLKID_TDMOUT_B_SCLK_SEL	87
>> +#define AUD_CLKID_TDMOUT_B_SCLK_PRE_EN	88
>> +#define AUD_CLKID_TDMOUT_B_SCLK_POST_EN	89
>> +#define AUD_CLKID_TDMOUT_B_SCLK		90
>> +#define AUD_CLKID_TDMOUT_B_LRCLK	91
>> +
>> +#define AUD2_CLKID_DDR_ARB		1
>> +#define AUD2_CLKID_PDM			2
>> +#define AUD2_CLKID_TDMIN_VAD		3
>> +#define AUD2_CLKID_TODDR_VAD		4
>> +#define AUD2_CLKID_VAD			5
>> +#define AUD2_CLKID_AUDIOTOP		6
>> +#define AUD2_CLKID_VAD_MCLK_SEL		7
>> +#define AUD2_CLKID_VAD_MCLK_DIV		8
>> +#define AUD2_CLKID_VAD_MCLK		9
>> +#define AUD2_CLKID_VAD_CLK_SEL		10
>> +#define AUD2_CLKID_VAD_CLK_DIV		11
>> +#define AUD2_CLKID_VAD_CLK		12
>> +#define AUD2_CLKID_PDM_DCLK_SEL		13
>> +#define AUD2_CLKID_PDM_DCLK_DIV		14
>> +#define AUD2_CLKID_PDM_DCLK		15
>> +#define AUD2_CLKID_PDM_SYSCLK_SEL	16
>> +#define AUD2_CLKID_PDM_SYSCLK_DIV	17
>> +#define AUD2_CLKID_PDM_SYSCLK		18
>> +
>> +#endif /* __A1_AUDIO_CLKC_BINDINGS_H */
>> diff --git a/include/dt-bindings/reset/amlogic,meson-a1-audio-reset.h b/include/dt-bindings/reset/amlogic,meson-a1-audio-reset.h
>> new file mode 100644
>> index 000000000000..653fddba1d8f
>> --- /dev/null
>> +++ b/include/dt-bindings/reset/amlogic,meson-a1-audio-reset.h
>> @@ -0,0 +1,29 @@
>> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
>> +/*
>> + * Copyright (c) 2024, SaluteDevices. All Rights Reserved.
>> + *
>> + * Author: Jan Dakinevich <jan.dakinevich@salutedevices.com>
>> + */
>> +
>> +#ifndef _DT_BINDINGS_AMLOGIC_MESON_A1_AUDIO_RESET_H
>> +#define _DT_BINDINGS_AMLOGIC_MESON_A1_AUDIO_RESET_H
>> +
>> +#define AUD_RESET_DDRARB	0
>> +#define AUD_RESET_TDMIN_A	1
>> +#define AUD_RESET_TDMIN_B	2
>> +#define AUD_RESET_TDMIN_LB	3
>> +#define AUD_RESET_LOOPBACK	4
>> +#define AUD_RESET_TDMOUT_A	5
>> +#define AUD_RESET_TDMOUT_B	6
>> +#define AUD_RESET_FRDDR_A	7
>> +#define AUD_RESET_FRDDR_B	8
>> +#define AUD_RESET_TODDR_A	9
>> +#define AUD_RESET_TODDR_B	10
>> +#define AUD_RESET_SPDIFIN	11
>> +#define AUD_RESET_RESAMPLE	12
>> +#define AUD_RESET_EQDRC		13
>> +#define AUD_RESET_LOCKER	14
>> +#define AUD_RESET_TOACODEC	30
>> +#define AUD_RESET_CLKTREE	31
>> +
>> +#endif /* _DT_BINDINGS_AMLOGIC_MESON_A1_AUDIO_RESET_H */
> 
> 

-- 
Best regards
Jan Dakinevich

^ permalink raw reply

* Re: [PATCH 1/2] arm64: dts: qcom: msm8916/8216-samsung-fortuna: Add samsung,fortuna binding
From: Krzysztof Kozlowski @ 2024-03-30 18:42 UTC (permalink / raw)
  To: Raymond Hackley, linux-kernel
  Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
	devicetree, ~postmarketos/upstreaming
In-Reply-To: <20240326141222.200452-2-raymondhackley@protonmail.com>

On 26/03/2024 15:12, Raymond Hackley wrote:
> Add common binding samsung,fortuna used by the following
> Samsung Galaxy Grand Prime devices:
> - SM-G530FZ (msm8916-samsung-grandprimelte)
> - SM-G530H  (msm8216-samsung-fortuna3g)
> - SM-G530W  (msm8916-gprimeltecan)
> 
> Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
> ---
>  arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts     | 2 +-
>  arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts  | 2 +-
>  arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts b/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
> index 366914be7d53..877fd2d7bb57 100644
> --- a/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
> +++ b/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
> @@ -6,6 +6,6 @@
>  
>  / {
>  	model = "Samsung Galaxy Grand Prime (SM-G530H)";
> -	compatible = "samsung,fortuna3g", "qcom,msm8916";
> +	compatible = "samsung,fortuna3g", "samsung,fortuna", "qcom,msm8916";

It does not look like you tested the DTS against bindings. Please run
`make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v5 1/1] dt-bindings: net: dwmac: Document STM32 property st,ext-phyclk
From: Krzysztof Kozlowski @ 2024-03-30 18:40 UTC (permalink / raw)
  To: Christophe Roullier, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Maxime Coquelin, Alexandre Torgue, Richard Cochran,
	Jose Abreu, Liam Girdwood, Mark Brown, Marek Vasut
  Cc: netdev, devicetree, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <20240328140803.324141-2-christophe.roullier@foss.st.com>

On 28/03/2024 15:08, Christophe Roullier wrote:
> The Linux kernel dwmac-stm32 driver currently supports three DT
> properties used to configure whether PHY clock are generated by
> the MAC or supplied to the MAC from the PHY.
> 
> Originally there were two properties, st,eth-clk-sel and
> st,eth-ref-clk-sel, each used to configure MAC clocking in
> different bus mode and for different MAC clock frequency.
> Since it is possible to determine the MAC 'eth-ck' clock
> frequency from the clock subsystem and PHY bus mode from
> the 'phy-mode' property, two disparate DT properties are
> no longer required to configure MAC clocking.
> 
> Linux kernel commit 1bb694e20839 ("net: ethernet: stmmac: simplify phy modes management for stm32")
> introduced a third, unified, property st,ext-phyclk. This property
> covers both use cases of st,eth-clk-sel and st,eth-ref-clk-sel DT
> properties, as well as a new use case for 25 MHz clock generated
> by the MAC.
> 
> The third property st,ext-phyclk is so far undocumented,
> document it.
> 
> Below table summarizes the clock requirement and clock sources for
> supported PHY interface modes.
>  __________________________________________________________________________
> |PHY_MODE | Normal | PHY wo crystal|   PHY wo crystal   |No 125Mhz from PHY|
> |         |        |      25MHz    |        50MHz       |                  |
> 
> ---------------------------------------------------------------------------
> |  MII    |    -   |     eth-ck    |        n/a         |       n/a        |
> |         |        | st,ext-phyclk |                    |                  |
> 
> ---------------------------------------------------------------------------
> |  GMII   |    -   |     eth-ck    |        n/a         |       n/a        |
> |         |        | st,ext-phyclk |                    |                  |
> 
> ---------------------------------------------------------------------------
> | RGMII   |    -   |     eth-ck    |        n/a         |      eth-ck      |
> |         |        | st,ext-phyclk |                    | st,eth-clk-sel or|
> |         |        |               |                    | st,ext-phyclk    |
> 
> ---------------------------------------------------------------------------
> | RMII    |    -   |     eth-ck    |      eth-ck        |       n/a        |
> |         |        | st,ext-phyclk | st,eth-ref-clk-sel |                  |
> |         |        |               | or st,ext-phyclk   |                  |
> 
> ---------------------------------------------------------------------------
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>

Can you please start testing patches *before* sending them?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 4/4] dt-bindings: aspeed: Add eSPI controller
From: Krzysztof Kozlowski @ 2024-03-30 18:36 UTC (permalink / raw)
  To: Manojkiran Eda, patrick.rudolph, chiawei_wang, ryan_chen,
	devicetree, linux-arm-kernel, linux-aspeed, linux-kernel,
	linux-mtd
  Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, joel, andrew,
	miquel.raynal, richard, vigneshr, jk, openbmc, broonie, linux-spi
In-Reply-To: <582c276c-ea2f-456c-9bf9-47e39289a0b6@gmail.com>

On 28/03/2024 12:33, Manojkiran Eda wrote:
>>>>> +    description: Controls the flash channel of eSPI hardware
>>>> That explains nothing. Unless you wanted to use here MTD bindings.
>>>>
>>>> This binding did not improve much. I don't understand why this is not
>>>> SPI (nothing in commit msg, nothing in description), what is eSPI,
>>>
>>> eSPI uses Peripheral, Virtual Wire, Out of Band, and Flash Access
>>> channels to communicate different sets of data.
>>
>> And what are these channels? What does it mean a "channel"? Is it just
>> how you organize transfers and classes of devices? Or some sort of
>> addressable instance on the bus?
>>
> 
> Yes, an espi channel provides a means to allow multiple independent 
> flows of traffic to share the same physical bus. Each of the channels 
> has its own dedicated resources such as queue and flow control.

Resources as queue and flow-control? Where are they defined in
Devicetree? Which binding?

> 
>> The channels feel like some sort of software or logical concept, not
>> physical. Physical would be endpoint with peripheral. Or flash memory.
> 
> A channel is a logical communication pathway or interface between the 
> chipset and peripheral devices. The concept of channels in the ESPI 
> protocol helps organize and manage different types of communication 
> between the chipset and peripherals. Each channel may have its own set 
> of protocols, data transfer rates, and supported features, tailored to 
> the requirements of the devices it serves.
> 
>> How do they fit here?
> 
> I am not sure I understand, can you please elaborate ?

All this suggests channel is programming aspect of your device thus does
not have to be represented in DT. I don't know, come with any DT
property to back up your case...

So far I see only interrupts and clocks, but then I would claim these
could be part of parent node.

Rob said it last time: your split of nodes looks artificial and it all
looks like one node.

Your DTS reg like:
	reg = <0x0 0x800>,<0x0 0x4000000>;
proves it. I don't know if this is just bug in your code or some silly
DTS just to create fake children. :/

> 
>>>
>>>    * The *Peripheral* Channel is used for communication between eSPI host
>>>      bridge located on the master side and eSPI endpoints located on the
>>>      slave side. LPC Host and LPC Peripherals are an example of eSPI host
>>>      bridge and eSPI endpoints respectively.
>>>    * *Virtual Wire* Channel: The Virtual Wire channel is used to
>>>      communicate the state of sideband pins or GPIO tunneled through eSPI
>>>      as in-band messages. Serial IRQ interrupts are communicated through
>>>      this channel as in-band messages.
>>>    * *OOB* Channel: The SMBus packets are tunneled through eSPI as
>>>      Out-Of-Band (OOB) messages. The whole SMBus packet is embedded
>>>      inside the eSPI OOB message as data.
>>>    * *Flash Access* Channel: The Flash Access channel provides a path
>>>      allowing the flash components to be shared run-time between chipset
>>>      and the eSPI slaves that require flash accesses such as EC (Embedded
>>>      Controller) and BMC.
>>
>> Please make binding complete, so define all of the channels.
> 
> 
> I would like to inquire about the rationale behind this request. Based 

Rationale - writing bindings document.
https://elixir.bootlin.com/linux/v6.9-rc1/source/Documentation/devicetree/bindings/writing-bindings.rst#L17


> on previous feedback received from the upstream efforts 
> [https://lore.kernel.org/openbmc/HK0PR06MB37798462D17443C697433D7191D09@HK0PR06MB3779.apcprd06.prod.outlook.com/], 
> suggestions were made to model the flash channel by utilizing the mtd 
> subsystem, the virtual wire channel by utilizing the GPIO subsystem, and 
> to consider the OOB channel as a type of i2c device, thereby allowing it 
> to be utilized by the existing in-kernel MCTP subsystem, among others. 
> My intention was to prioritize upstreaming the flash channel binding, 
> along with its driver code, before proceeding to address other channels. 

Just to clarify: I don't care about drivers and we do not talk about
them here.

> I am curious to understand if it is a strict requirement to have the 
> complete binding upstreamed before addressing the device drivers code.

What if your other "devices" or "channels" are entirely different and
binding would just not work? Or how can we understand the design if you
upstream only part of it?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 07/10] dt-bindings: iio: dac: add bindings doc for AD9739A
From: Krzysztof Kozlowski @ 2024-03-30 18:27 UTC (permalink / raw)
  To: David Lechner, nuno.sa
  Cc: linux-iio, devicetree, Dragos Bogdan, Jonathan Cameron,
	Lars-Peter Clausen, Michael Hennerich, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Olivier Moysan
In-Reply-To: <CAMknhBHA3LOhZicUCG32RyiV4+OROJymzm0Qc=wqhfo3u=8vnQ@mail.gmail.com>

On 29/03/2024 20:06, David Lechner wrote:
> 
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  vdd_3_3-supply:
>> +    description: 3.3V Digital input supply.
> 
> vdd33-supply would better match the datasheet name

And would conform to coding style. No underscores in names.

Nuno,
To avoid comments like this one, please read and follow DTS coding style
when you work with DTS or Devicetree bindings.


Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
From: Krzysztof Kozlowski @ 2024-03-30 18:25 UTC (permalink / raw)
  To: Marc Gonzalez, Kalle Valo, Jeff Johnson, ath10k
  Cc: wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Konrad Dybcio,
	Jami Kettunen, Jeffrey Hugo
In-Reply-To: <5cdad89c-282a-4df5-a286-b8404bc4dd81@freebox.fr>

On 28/03/2024 18:39, Marc Gonzalez wrote:
> The ath10k driver waits for an "MSA_READY" indicator
> to complete initialization. If the indicator is not
> received, then the device remains unusable.
> 
> cf. ath10k_qmi_driver_event_work()
> 
> Several msm8998-based devices are affected by this issue.
> Oddly, it seems safe to NOT wait for the indicator, and
> proceed immediately when QMI_EVENT_SERVER_ARRIVE.
> 
> Jeff Johnson wrote:
> 
>   The feedback I received was "it might be ok to change all ath10k qmi
>   to skip waiting for msa_ready", and it was pointed out that ath11k
>   (and ath12k) do not wait for it.
> 
>   However with so many deployed devices, "might be ok" isn't a strong
>   argument for changing the default behavior.
> 

I think you got pretty clear comments:

"This sounds more like a firmware feature, not a hardware feature."

"This is why having this property in DT does not look right
place for this."

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
From: Krzysztof Kozlowski @ 2024-03-30 18:23 UTC (permalink / raw)
  To: Marc Gonzalez, Kalle Valo, Jeff Johnson, ath10k
  Cc: wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Konrad Dybcio,
	Jami Kettunen, Jeffrey Hugo
In-Reply-To: <72c162cc-45e0-48b6-8d90-d59fac299375@linaro.org>

On 30/03/2024 19:20, Krzysztof Kozlowski wrote:
> On 28/03/2024 18:36, Marc Gonzalez wrote:
>> The ath10k driver waits for an "MSA_READY" indicator
>> to complete initialization. If the indicator is not
>> received, then the device remains unusable.
>>
>> cf. ath10k_qmi_driver_event_work()
>>
>> Several msm8998-based devices are affected by this issue.
>> Oddly, it seems safe to NOT wait for the indicator, and
>> proceed immediately when QMI_EVENT_SERVER_ARRIVE.
>>
> 
> This is v2, so where is the changelog?

Expecting reviewer to dig previous discussions will not help your case.
It helps reviewers if you provide necessary information, like resolution
of previous discussion in the changelog.

I dig the previous discussion, since you did not mention it here, and it
seems you entirely ignored its outcome. That's not a DT property.

NAK, sorry. Please go back to v1 and read the comments you got there.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
From: Krzysztof Kozlowski @ 2024-03-30 18:20 UTC (permalink / raw)
  To: Marc Gonzalez, Kalle Valo, Jeff Johnson, ath10k
  Cc: wireless, DT, MSM, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Pierre-Hugues Husson, Arnaud Vrac, Bjorn Andersson, Konrad Dybcio,
	Jami Kettunen, Jeffrey Hugo
In-Reply-To: <84f20fb5-5d48-419c-8eff-d7044afb81c0@freebox.fr>

On 28/03/2024 18:36, Marc Gonzalez wrote:
> The ath10k driver waits for an "MSA_READY" indicator
> to complete initialization. If the indicator is not
> received, then the device remains unusable.
> 
> cf. ath10k_qmi_driver_event_work()
> 
> Several msm8998-based devices are affected by this issue.
> Oddly, it seems safe to NOT wait for the indicator, and
> proceed immediately when QMI_EVENT_SERVER_ARRIVE.
> 

This is v2, so where is the changelog?

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 1/3] drivers/iio/adc: Add XuanTie TH1520 ADC driver
From: Jonathan Cameron @ 2024-03-30 18:17 UTC (permalink / raw)
  To: wefu
  Cc: jszhang, guoren, conor, robh, krzysztof.kozlowski+dt,
	paul.walmsley, palmer, aou, lars, andriy.shevchenko, nuno.sa,
	marcelo.schmitt, bigunclemax, marius.cristea, fr0st61te,
	okan.sahin, marcus.folkesson, schnelle, lee, mike.looijmans,
	linux-riscv, devicetree, linux-kernel, linux-iio
In-Reply-To: <20240329200241.4122000-2-wefu@redhat.com>

On Sat, 30 Mar 2024 04:01:24 +0800
wefu@redhat.com wrote:

> From: Wei Fu <wefu@redhat.com>
> 
> Signed-off-by: Wei Fu <wefu@redhat.com>

Hi Wei Fu,

Comments inline.

Main thing is that there is quite a bit of code here to support a more
feature rich driver, but with parts missing.  Rip all that out until those
features are added in future patches.

Jonathan

> ---
>  drivers/iio/adc/Kconfig              |  13 +
>  drivers/iio/adc/Makefile             |   1 +
>  drivers/iio/adc/xuantie-th1520-adc.c | 572 +++++++++++++++++++++++++++
>  drivers/iio/adc/xuantie-th1520-adc.h | 193 +++++++++
>  4 files changed, 779 insertions(+)
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 0d9282fa67f5..9e37ba2a877a 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -1515,4 +1515,17 @@ config XILINX_AMS
>  	  The driver can also be built as a module. If so, the module will be called
>  	  xilinx-ams.
>  
> +config XUANTIE_TH1520_ADC
> +	tristate "XuanTie TH1520 ADC driver"
> +	depends on OF

No need for that dependency.  In theory you could use this with ACPI PRP0001
for example.

> +	depends on HAS_IOMEM
> +	depends on ARCH_THEAD 
|| COMPILE_TEST 
to get us build coverage as I don't immediately see anything arch specific.

> +	default y
Don't set the default (i.e. rely on default default of no.)

> +	help
> +	  Say yes here to support for XuanTie TH1520 MPW analog-to-digital
> +	  converter.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called xuantie-th1520-adc.
> +
>  endmenu
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index b3c434722364..820e2a136b33 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -135,4 +135,5 @@ obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
>  xilinx-xadc-y := xilinx-xadc-core.o xilinx-xadc-events.o
>  obj-$(CONFIG_XILINX_XADC) += xilinx-xadc.o
>  obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
> +obj-$(CONFIG_XUANTIE_TH1520_ADC) += xuantie-th1520-adc.o
>  obj-$(CONFIG_SD_ADC_MODULATOR) += sd_adc_modulator.o
> diff --git a/drivers/iio/adc/xuantie-th1520-adc.c b/drivers/iio/adc/xuantie-th1520-adc.c
> new file mode 100644
> index 000000000000..df0452c2abe7
> --- /dev/null
> +++ b/drivers/iio/adc/xuantie-th1520-adc.c
> @@ -0,0 +1,572 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * XuanTie TH1520 ADC driver
> + *
> + * Copyright (C) 2021-2024 Alibaba Group Holding Limited.
> + * Fugang Duan <duanfugang.dfg@linux.alibaba.com>
> + *
Blank line here not useful so drop it.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
As below. Unlikely these are needed in a driver.

> +#include <linux/regulator/consumer.h>
> +#include <linux/of_platform.h>

Doubt you need that.  mod_devicetable.h perhaps?

> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
Why?

> +#include <linux/iio/sysfs.h>
> +
> +#include "xuantie-th1520-adc.h"
> +
> +static inline void th1520_adc_cfg_init(struct th1520_adc *info)
> +{
> +	struct th1520_adc_feature *adc_feature = &info->adc_feature;
> +
> +	/* set default Configuration for ADC controller */
> +	adc_feature->selres_sel = TH1520_ADC_SELRES_12BIT;
> +	adc_feature->input_mode = TH1520_ADC_SINGLE_ENDED_INPUTS;
> +	adc_feature->vol_ref = TH1520_ADC_VOL_VREF;
> +	adc_feature->offset_mode = TH1520_ADC_OFFSET_DIS;
> +	adc_feature->conv_mode = TH1520_ADC_MODE_SINGLE;
> +	adc_feature->clk_sel = TH1520_ADC_FCLK_TYP_1M;
> +
> +	adc_feature->int_actual = TH1520_ADC_ACTUAL_ALL;
> +	adc_feature->int_detal = TH1520_ADC_DETAL_ALL;
> +
> +	info->ch0_offmeas = 0;
> +	info->ch1_offmeas = 0;
Bring these features in when you add support to the driver. Until then
stick just using the default values inline as it simplifies the code.

> +}
> +
> +static void th1520_adc_reg_set(struct th1520_adc *info)
> +{
> +	u32 phy_cfg = 0;
> +	u32 op_ctrl = 0;
> +	struct th1520_adc_feature *adc_feature = &info->adc_feature;
> +
> +	/* phy_cfg */

That's obvious from the code, so I'd drop the comment.

> +	switch (adc_feature->selres_sel) {
> +	case TH1520_ADC_SELRES_6BIT:
Use numbers here.  You could just use maths for the conversion as well
to simplify things further.  However I've suggested elsewhere that you just
always use 12bit in which case this complexity isn't needed.

> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELRES_6BIT;
> +		break;
> +	case TH1520_ADC_SELRES_8BIT:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELRES_8BIT;
> +		break;
> +	case TH1520_ADC_SELRES_10BIT:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELRES_10BIT;
> +		break;
> +	case TH1520_ADC_SELRES_12BIT:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELRES_12BIT;
> +		break;
> +	default:
It's an enum, so we shouldn't get here with anything else and can drop the default.
> +		break;
> +	}
> +
> +	switch (adc_feature->input_mode) {
> +	case TH1520_ADC_SINGLE_ENDED_INPUTS:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELDIFF_SINGLE_ENDED_INPUTS;
> +		break;
> +	case TH1520_ADC_DIFFERENTIAL_INPUTS:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELDIFF_DIFFERENTIAL_INPUTS;
> +		break;
> +	default:

Not need as all values covered.

> +		break;
> +	}
> +
> +	switch (adc_feature->vol_ref) {
> +	case TH1520_ADC_VOL_VREF:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELBG_EXTERNAL |
> +			   TH1520_ADC_PHY_CFG_SELREF_EXT;
> +		break;
> +	case TH1520_ADC_VOL_INTE:
> +		phy_cfg |= TH1520_ADC_PHY_CFG_SELBG_INTERNAL |
> +			   TH1520_ADC_PHY_CFG_SELREF_INTERNAL;
> +		break;
> +	default:
Probably also not needed.
> +		break;
> +	}
> +
> +	/* op_ctrl */
> +	switch (adc_feature->conv_mode) {
> +	case TH1520_ADC_MODE_SINGLE:
> +		op_ctrl |= TH1520_ADC_OP_CTRL_MODE_SINGLE;
> +		break;
> +	case TH1520_ADC_MODE_CONTINOUS:
> +		op_ctrl |= TH1520_ADC_OP_CTRL_MODE_CONTINOUS;
> +		break;
> +	default:
As above.
> +		break;
> +	}
> +
> +	writel(phy_cfg, info->regs + TH1520_ADC_PHY_CFG);

I'd keep the writes with the register setup, so move this up above
filling in op_ctrl value.

> +	writel(op_ctrl, info->regs + TH1520_ADC_OP_CTRL);
> +
> +	writel(TH1520_ADC_PHY_ENCTR, info->regs + TH1520_ADC_PHY_TEST);
> +
> +	/* disable the irq */
> +	writel(0xff, info->regs + TH1520_ADC_INT_CTRL1);
> +	writel(0xff, info->regs + TH1520_ADC_INT_CTRL2);
> +
> +	if (adc_feature->conv_mode == TH1520_ADC_MODE_CONTINOUS)
> +		writel(TH1520_ADC_PHY_CTRL_ENADC_EN,
> +		       info->regs + TH1520_ADC_PHY_CTRL);
> +}
> +
> +static void th1520_adc_fclk_set(struct th1520_adc *info)
> +{
> +	int fclk_ctrl = 0;
> +	int start_time = 0;
> +	int sample_time = 0;
> +	struct th1520_adc_feature *adc_feature = &info->adc_feature;
> +
> +	switch (adc_feature->clk_sel) {
> +	case TH1520_ADC_FCLK_TYP_1M:
> +		fclk_ctrl = TH1520_ADC_FCLK_CTRL_TYP_1M;
> +		start_time = TH1520_ADC_START_TIME_TYP_1M;
> +		if (adc_feature->selres_sel == TH1520_ADC_SELRES_6BIT)
Use numbers. Also maybe just convert from resolution to register value
using maths rather than an if/else stack.  Otherwise, you could just have
an array mapping from register value to real value and search it.

> +			sample_time = TH1520_ADC_SAMPLE_TIME_TYP_6BIT;
> +		else if (adc_feature->selres_sel == TH1520_ADC_SELRES_8BIT)
> +			sample_time = TH1520_ADC_SAMPLE_TIME_TYP_8BIT;
> +		else if (adc_feature->selres_sel == TH1520_ADC_SELRES_10BIT)
> +			sample_time = TH1520_ADC_SAMPLE_TIME_TYP_10BIT;
> +		else if (adc_feature->selres_sel == TH1520_ADC_SELRES_12BIT)
> +			sample_time = TH1520_ADC_SAMPLE_TIME_TYP_12BIT;
> +		else {
> +			pr_err("[%s,%d]invalid selres select\n",
> +			       __func__, __LINE__);
> +			return;
> +		}
> +		break;
> +	default:
Comment on why nothing to do here.

> +		break;
> +	}
> +	writel(fclk_ctrl, info->regs + TH1520_ADC_FCLK_CTRL);
> +	writel(start_time, info->regs + TH1520_ADC_START_TIME);
> +	writel(sample_time, info->regs + TH1520_ADC_SAMPLE_TIME);
> +}
> +
> +static void th1520_adc_hw_init(struct th1520_adc *info)
> +{
> +	th1520_adc_reg_set(info);
> +	th1520_adc_fclk_set(info);
> +}
> +
> +static const struct iio_chan_spec th1520_adc_iio_channels[] = {
> +	TH1520_ADC_CHAN(0, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(1, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(2, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(3, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(4, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(5, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(6, IIO_VOLTAGE),
> +	TH1520_ADC_CHAN(7, IIO_VOLTAGE),
> +	/* sentinel */
?  There isn't a sentinel so drop the comment.

> +};
> +
> +static irqreturn_t th1520_adc_isr(int irq, void *dev_id)
> +{
> +	struct iio_dev *indio_dev = dev_id;
> +	struct th1520_adc *info = iio_priv(indio_dev);
> +	/* TBD */

?  Shouldn't see a TBD in a submitted driver.

> +	complete(&info->completion);
> +	return IRQ_HANDLED;
> +}
> +
> +static int th1520_read_raw(struct iio_dev *indio_dev,
> +			   struct iio_chan_spec const *chan,
> +			   int *val, int *val2, long mask)
> +{
> +	int tmp;
> +	long ret;
> +	struct th1520_adc *info = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
Could use guard() here though the saving in this case is monor.

> +		mutex_lock(&info->mlock);
> +
> +		if (info->adc_feature.conv_mode == TH1520_ADC_MODE_SINGLE) {

At the moment there is no control over this mode, so I would drop
the alternative path until you add such control.  Also drop even tracking
what that mode is set to as it won't change.

> +			uint ievent;
> +			uint vld_flag;
> +			uint phy_ctrl;
> +			uint real_chan;
> +			uint op_ctrl = 0;
> +			uint single_retry = TH1520_ADC_FIFO_DATA_SIZE;
> +
> +			op_ctrl = readl(info->regs + TH1520_ADC_OP_CTRL);
> +			op_ctrl &= ~TH1520_ADC_OP_CTRL_CH_EN_ALL;
> +			op_ctrl |= (BIT(chan->channel +	TH1520_ADC_OP_CTRL_CH_EN_0) &
> +					TH1520_ADC_OP_CTRL_CH_EN_ALL);

FIELD_PREP()

> +			writel(op_ctrl, info->regs + TH1520_ADC_OP_CTRL);

This first block is common to both paths. I'd factor out as much as possible

> +
> +			writel(TH1520_ADC_PHY_CTRL_ENADC_EN,
> +			       info->regs + TH1520_ADC_PHY_CTRL);
> +
> +			vld_flag = TH1520_ADC_SAMPLE_DATA_CH0_VLD;
> +
> +			while (single_retry--) {

Add a common to justify the retries (particularly why this number of them).

This deep indentation suggests to me that some of this code should be factored
out into a separate function.


> +				writel(TH1520_ADC_OP_SINGLE_START_EN,
> +				       info->regs + TH1520_ADC_OP_SINGLE_START);
> +				/* wait the sampling result */
> +				ret = readl_poll_timeout(info->regs +
> +							 TH1520_ADC_SAMPLE_DATA,
> +							 ievent,
> +							 ievent & vld_flag, 100,
> +							 TH1520_ADC_TIMEOUT);
> +				if (ret)
> +					pr_err("wait the sampling timeout\n");

Given you are going to retry, shouldn't see an error message here.

> +
> +				real_chan =
> +				(ievent & TH1520_ADC_SAMPLE_DATA_CH0_NUMBER) >>
> +				TH1520_ADC_SAMPLE_DATA_CH0_NUMBER_OFF;

FIELD_GET()

> +				if (real_chan == chan->channel)
> +					break;
> +			}
> +
> +			phy_ctrl = readl(info->regs + TH1520_ADC_PHY_CTRL);
> +			phy_ctrl &= ~TH1520_ADC_PHY_CTRL_ENADC_EN;
> +			writel(phy_ctrl, info->regs + TH1520_ADC_PHY_CTRL);
> +
> +			/* read the sampling data */
> +			*val = (ievent & TH1520_ADC_SAMPLE_DATA_CH0) >>
> +			       TH1520_ADC_SAMPLE_DATA_CH0_OFF;
FIELD_GET()

> +		} else {
> +			uint ievent;
> +			uint vld_flag;
> +			uint op_single;
> +			uint op_ctrl = 0;
> +
> +			op_ctrl = readl(info->regs + TH1520_ADC_OP_CTRL);
> +			op_ctrl &= ~TH1520_ADC_OP_CTRL_CH_EN_ALL;
> +			op_ctrl |= (BIT(chan->channel + TH1520_ADC_OP_CTRL_CH_EN_0) &
> +				   TH1520_ADC_OP_CTRL_CH_EN_ALL);
> +			writel(op_ctrl, info->regs + TH1520_ADC_OP_CTRL);
> +
> +			op_single = readl(info->regs +
> +					  TH1520_ADC_OP_SINGLE_START);
> +			op_single &= ~TH1520_ADC_OP_SINGLE_START_EN;
> +			writel(op_single,
> +			       info->regs + TH1520_ADC_OP_SINGLE_START);
> +
> +			vld_flag = TH1520_ADC_SAMPLE_DATA_CH0_VLD |
> +				   TH1520_ADC_SAMPLE_DATA_CH1_VLD;
> +
> +			/* wait the sampling result */
> +			ret  = readl_poll_timeout(info->regs +
> +							TH1520_ADC_SAMPLE_DATA,
> +						  ievent, ievent & vld_flag, 10,
> +						  TH1520_ADC_TIMEOUT);
> +			if (ret)
> +				pr_err("wait the sampling timeout\n");
> +
> +			/* read the sampling data */
> +			tmp = readl(info->regs + TH1520_ADC_SAMPLE_DATA);
> +			if (tmp & TH1520_ADC_SAMPLE_DATA_CH0_VLD)
> +				*val = (tmp & TH1520_ADC_SAMPLE_DATA_CH0) >>
> +				       TH1520_ADC_SAMPLE_DATA_CH0_OFF;
> +			else
> +				*val = (tmp & TH1520_ADC_SAMPLE_DATA_CH1) >>
> +				       TH1520_ADC_SAMPLE_DATA_CH1_OFF;
> +		}
> +
> +		mutex_unlock(&info->mlock);
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = info->vref_uv / 1000;
> +		*val2 = info->adc_feature.selres_sel;
> +		return IIO_VAL_FRACTIONAL_LOG2;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = info->current_clk;
> +		*val2 = 0;
> +		return IIO_VAL_INT;
> +
> +	default:
> +		break;
return -EINVAL; then drop the one below.

> +	}
> +
> +	return -EINVAL;
> +}

> +
> +static ssize_t th1520_adc_res_show(struct device *dev,
> +				   struct device_attribute *attr, char *buf)
> +{
> +	size_t bufpos = 0, count = 5;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct th1520_adc *info = iio_priv(indio_dev);
> +
> +	snprintf(buf + bufpos, count - bufpos, "%.*x: ", 4,
> +		 info->adc_feature.selres_sel);
> +	bufpos += 4;
> +	buf[bufpos++] = '\n';
> +
> +	return bufpos;
> +}
> +
> +static ssize_t th1520_adc_res_store(struct device *dev,
> +				    struct device_attribute *attr,
> +				    const char *buf, size_t size)
> +{
> +	unsigned long res;
> +	char *start = (char *)buf;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct th1520_adc *info = iio_priv(indio_dev);
> +
> +	if (kstrtoul(start, 0, &res))
> +		return -EINVAL;
> +
> +	switch (res) {
> +	case TH1520_ADC_SELRES_6BIT:
> +	case TH1520_ADC_SELRES_8BIT:
> +	case TH1520_ADC_SELRES_10BIT:
> +	case TH1520_ADC_SELRES_12BIT:
> +		info->adc_feature.selres_sel = res;
> +		th1520_adc_reset(info);
> +		th1520_adc_hw_init(info);
> +		break;
> +	default:
> +		dev_err(dev, "not support res\n");
> +		return -EINVAL;
> +	}
> +
> +	return size;
> +}
> +
> +static DEVICE_ATTR_RW(th1520_adc_res);
Custom ABI needs documentation.  We very rarely allow direct control of resolution
as it normally also affects scale which is standard ABI. 

Also, for drivers only providing polled reads, the saving in time of using
low res readouts it negligible so just use 12bit ever time.

> +
> +static int th1520_adc_probe(struct platform_device *pdev)
> +{
> +	int irq;
> +	int ret;
> +	struct resource *mem;
> +	struct th1520_adc *info;
> +	struct iio_dev *indio_dev;
> +
> +	indio_dev = devm_iio_device_alloc(&pdev->dev,
> +					  sizeof(struct th1520_adc));
> +	if (!indio_dev) {
> +		dev_err(&pdev->dev, "Failed allocating iio device\n");
> +		return -ENOMEM;
dev_err_probe() here as well.

> +	}
> +
> +	info = iio_priv(indio_dev);
> +	info->dev = &pdev->dev;
> +
> +	irq = platform_get_irq(pdev, 0);
> +	if (irq < 0)
> +		return irq;
> +
> +	ret = devm_request_irq(info->dev, irq, th1520_adc_isr, 0,
> +			       dev_name(&pdev->dev), indio_dev);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed requesting irq, irq = %d\n", irq);
> +		return ret;

Use return dev_err_probe() for errors in probe() routines and functions that
are only called during probe.  It better handles deferred probing and also
gives shorter code in general.

> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	info->regs = devm_ioremap_resource(&pdev->dev, mem);
> +	if (IS_ERR(info->regs))
> +		return PTR_ERR(info->regs);
> +
> +	info->clk = devm_clk_get(&pdev->dev, "adc");
> +	if (IS_ERR(info->clk)) {
> +		dev_err(&pdev->dev, "failed getting clock, err = %ld\n",
> +			PTR_ERR(info->clk));
> +		return PTR_ERR(info->clk);
> +	}
> +
> +	info->vref = devm_regulator_get(&pdev->dev, "vref");
> +	if (IS_ERR(info->vref))
> +		return PTR_ERR(info->vref);
> +
> +	ret = regulator_enable(info->vref);
> +	if (ret)
> +		return ret;
> +
> +	info->vref_uv = regulator_get_voltage(info->vref);

There is a patch set on list that will replace this boiler plate with a single
call to enable the regulator and get it's voltage, but we can make use of that
once/if that series is accepted.

> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	mutex_init(&info->mlock);
> +	init_completion(&info->completion);
> +
> +	indio_dev->name = dev_name(&pdev->dev);
> +	indio_dev->dev.parent = &pdev->dev;
> +	indio_dev->dev.of_node = pdev->dev.of_node;
Both parent and of_node are set in the IIO core. I doubt you need to override
them.

> +	indio_dev->info = &th1520_adc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = th1520_adc_iio_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(th1520_adc_iio_channels);
> +
> +	ret = clk_prepare_enable(info->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev,
> +			"Could not prepare or enable the clock.\n");
> +		goto error_adc_clk_enable;
> +	}
> +
> +	th1520_adc_cfg_init(info);
> +	th1520_adc_reset(info);
> +	th1520_adc_hw_init(info);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Couldn't register the device.\n");
> +		goto error_iio_device_register;
> +	}
> +
> +	ret = sysfs_create_file(&pdev->dev.kobj, &dev_attr_th1520_adc_res.attr);

This is very much the wrong way to do sysfs attributes in general, but I doubt
this will end up in the final driver anyway so I won't go into how this should
be done.  Adding a file after registration is racey with userspace.

> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to create adc debug sysfs.\n");
> +		goto error_iio_device_register;
> +	}
> +
> +	dev_info(&pdev->dev, "XuanTie TH1520 adc registered.\n");

Too noisy.  There are loads of ways of finding that out that don't need a message
in the kernel log.  If you really want something then dev_dbg() at most.


> +	return 0;
> +
> +error_iio_device_register:
> +	clk_disable_unprepare(info->clk);
> +error_adc_clk_enable:
> +	regulator_disable(info->vref);
> +
> +	return ret;
> +}
> +
> +static int th1520_adc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct th1520_adc *info = iio_priv(indio_dev);
> +
> +	iio_device_unregister(indio_dev);
> +	regulator_disable(info->vref);
> +	clk_disable_unprepare(info->clk);
> +
> +	return 0;
> +}

> +static SIMPLE_DEV_PM_OPS(th1520_adc_pm_ops,
IIRC this is deprecated use.
DEFINE_SIMPLE_DEV_PM_OPS()
> +			 th1520_adc_suspend, th1520_adc_resume);
> +
> +static struct platform_driver th1520_adc_driver = {
> +	.probe          = th1520_adc_probe,
> +	.remove         = th1520_adc_remove,
> +	.driver         = {
> +		.name   = DRIVER_NAME,

Put the string here and don't use a define.

> +		.of_match_table = th1520_adc_match,
> +		.pm     = &th1520_adc_pm_ops,
with the above, pm_sleep_ptr() around this and then if you build
without PM support the unused code will be removed by the compiler.

> +	},
> +};
> +module_platform_driver(th1520_adc_driver);
> +
> +MODULE_AUTHOR("fugang.duan <duanfugang.dfg@linux.alibaba.com>");
> +MODULE_DESCRIPTION("XuanTie TH1520 ADC driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/iio/adc/xuantie-th1520-adc.h b/drivers/iio/adc/xuantie-th1520-adc.h
> new file mode 100644
> index 000000000000..c38d17fc6bbe
> --- /dev/null
> +++ b/drivers/iio/adc/xuantie-th1520-adc.h
> @@ -0,0 +1,193 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * XuanTie TH1520 ADC driver
> + *
> + * Copyright (C) 2021-2024 Alibaba Group Holding Limited.
> + * Fugang Duan <duanfugang.dfg@linux.alibaba.com>
> + *
> + */
> +
> +#include <linux/bitops.h>
> +
> +/* This will be the driver name the kernel reports */
> +#define DRIVER_NAME "xuantie-th1520-adc"

No obvious reason for a header - push this down into the C file.

> +
> +/* ADC registers */
> +#define TH1520_ADC_PHY_CFG			0x00
> +#define TH1520_ADC_PHY_CTRL			0x04
> +#define TH1520_ADC_PHY_TEST			0x08
> +#define TH1520_ADC_OP_CTRL			0x0C
> +#define TH1520_ADC_OP_SINGLE_START		0x10
> +#define TH1520_ADC_FCLK_CTRL			0x14
> +#define TH1520_ADC_START_TIME			0x18
> +#define TH1520_ADC_SAMPLE_TIME			0x1C
> +#define TH1520_ADC_SAMPLE_DATA			0x20
> +#define TH1520_ADC_INT_CTRL1			0x50
> +#define TH1520_ADC_INT_CTRL2			0x54
> +#define TH1520_ADC_INT_STATUS			0x58
> +#define TH1520_ADC_INT_ACTUAL_VALUE_CH0		0x60
> +#define TH1520_ADC_INT_ACTUAL_VALUE_CH1		0x64
> +#define TH1520_ADC_INT_DELTA_VALUE_CH0		0x90
> +#define TH1520_ADC_INT_DELTA_VALUE_CH1		0x94
> +
> +/* Configuration register field define */
> +#define TH1520_ADC_PHY_CFG_SELRES_6BIT			(0x0)
> +#define TH1520_ADC_PHY_CFG_SELRES_8BIT			(0x1)
> +#define TH1520_ADC_PHY_CFG_SELRES_10BIT			(0x2)
> +#define TH1520_ADC_PHY_CFG_SELRES_12BIT			(0x3)
> +#define TH1520_ADC_PHY_CFG_SELDIFF_SINGLE_ENDED_INPUTS	(0x0 << 4)
> +#define TH1520_ADC_PHY_CFG_SELDIFF_DIFFERENTIAL_INPUTS	(0x1 << 4)
> +#define TH1520_ADC_PHY_CFG_SELBG_INTERNAL		(0x1 << 8)
> +#define TH1520_ADC_PHY_CFG_SELBG_EXTERNAL		(0x0 << 8)
> +#define TH1520_ADC_PHY_CFG_SELREF_INTERNAL		(0x1 << 12)
> +#define TH1520_ADC_PHY_CFG_SELREF_EXT			(0x0 << 12)
> +
> +/* PHY CTRL register field define */
> +#define TH1520_ADC_PHY_CTRL_ENOFFSET_EN			(0x1 << 12)

BIT()

> +#define TH1520_ADC_PHY_CTRL_ENOFFMEAS_EN		(0x1 << 8)
> +#define TH1520_ADC_PHY_CTRL_RST_EN			(0x1 << 4)
> +#define TH1520_ADC_PHY_CTRL_ENADC_EN			(0x1 << 0)
> +
> +/* ADC OP ctrl field define  */
> +#define TH1520_ADC_OP_CTRL_CH_EN_ALL			GENMASK(19, 12)
> +#define TH1520_ADC_OP_CTRL_CH_EN_0			(12)
> +#define TH1520_ADC_OP_CTRL_MODE_SINGLE			(0x1 << 0)
> +#define TH1520_ADC_OP_CTRL_MODE_CONTINOUS		(0x0 << 0)
> +
> +/* ADC OP single start */
> +#define TH1520_ADC_OP_SINGLE_START_EN			BIT(0)
> +
> +/* ADC fclk ctrl */
> +#define TH1520_ADC_FCLK_CTRL_FCLLK_DIV			GENMASK(6, 0)
> +#define TH1520_ADC_FCLK_CTRL_TYP_1M			(0x10004)
> +#define TH1520_ADC_START_TIME_TYP_1M			(0x160)
> +#define TH1520_ADC_SAMPLE_TIME_TYP_1M			(0x10)
> +#define TH1520_ADC_SAMPLE_TIME_TYP_6BIT			(8)
> +#define TH1520_ADC_SAMPLE_TIME_TYP_8BIT			(10)
> +#define TH1520_ADC_SAMPLE_TIME_TYP_10BIT		(12)
> +#define TH1520_ADC_SAMPLE_TIME_TYP_12BIT		(14)
> +
> +/* ADC sample data */
> +#define TH1520_ADC_SAMPLE_DATA_CH1			GENMASK(27, 16)
> +#define TH1520_ADC_SAMPLE_DATA_CH1_OFF			(16)
> +#define TH1520_ADC_SAMPLE_DATA_CH1_VLD			BIT(31)
> +#define TH1520_ADC_SAMPLE_DATA_CH1_NUMBER		GENMASK(30, 28)
> +#define TH1520_ADC_SAMPLE_DATA_CH1_NUMBER_OFF		(28)
> +#define TH1520_ADC_SAMPLE_DATA_CH0			GENMASK(11, 0)
> +#define TH1520_ADC_SAMPLE_DATA_CH0_VLD			BIT(15)
> +#define TH1520_ADC_SAMPLE_DATA_CH0_OFF			(0)
> +#define TH1520_ADC_SAMPLE_DATA_CH0_NUMBER		GENMASK(14, 12)
> +#define TH1520_ADC_SAMPLE_DATA_CH0_NUMBER_OFF		(12)
> +
> +/* ADC INT Ctrl */
> +#define TH1520_ADC_INT_CTRL1_CH1_INT_MODE		BIT(1)
> +#define TH1520_ADC_INT_CTRL1_CH0_INT_MODE		BIT(0)
> +#define TH1520_ADC_INT_CTRL2_CH1_INT_MASK		BIT(1)
> +#define TH1520_ADC_INT_CTRL2_CH0_INT_MASK		BIT(0)
> +#define TH1520_ADC_INT_STS_CH1_INT_STS			BIT(1)
> +#define TH1520_ADC_INT_STS_CH0_INT_STS			BIT(0)
> +
> +#define TH1520_ADC_ACTUAL_VALUE_CH0_HVAL		GENMASK(27, 16)
> +#define TH1520_ADC_ACTUAL_VALUE_CH0_HVAL_OFF		(16)
> +#define TH1520_ADC_ACTUAL_VALUE_CH0_LVAL		GENMASK(11, 0)
> +#define TH1520_ADC_ACTUAL_VALUE_CH0_LVAL_OFF		(0)
> +#define TH1520_ADC_ACTUAL_VALUE_CH1_HVAL		GENMASK(27, 16)
> +#define TH1520_ADC_ACTUAL_VALUE_CH1_HVAL_OFF		(16)
> +#define TH1520_ADC_ACTUAL_VALUE_CH1_LVAL		GENMASK(11, 0)
> +#define TH1520_ADC_ACTUAL_VALUE_CH1_LVAL_OFF		(0)
> +
> +#define TH1520_ADC_DLT_VALUE_CH0_HVAL			GENMASK(27, 16)
> +#define TH1520_ADC_DLT_VALUE_CH0_HVAL_OFF		(16)
> +#define TH1520_ADC_DLT_VALUE_CH0_LVAL			GENMASK(11, 0)
> +#define TH1520_ADC_DLT_VALUE_CH0_LVAL_OFF		(0)
> +#define TH1520_ADC_DLT_VALUE_CH1_HVAL			GENMASK(27, 16)
> +#define TH1520_ADC_DLT_VALUE_CH1_HVAL_OFF		(16)
> +#define TH1520_ADC_DLT_VALUE_CH1_LVAL			GENMASK(11, 0)
> +#define TH1520_ADC_DLT_VALUE_CH1_LVAL_OFF		(0)
Don't specify offsets.  Use FIELD_GET() and FIELD_PREP()  for all accesses
as those work with the mask alone.  Also, make it clear what is a mask via
naming.  Typically _MSK or similar.


> +
> +#define TH1520_ADC_FIFO_DATA_SIZE			32
> +#define TH1520_ADC_PHY_ENCTR				0x8e0
> +#define TH1520_ADC_TIMEOUT				500000
> +
> +#define TH1520_ADC_CHAN(_idx, _chan_type) {			\
> +	.type = (_chan_type),					\
> +	.indexed = 1,						\
> +	.channel = (_idx),					\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),		\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +}
> +
> +enum vol_ref {
> +	TH1520_ADC_VOL_VREF,
> +	TH1520_ADC_VOL_INTE,
> +};
> +
> +enum input_mode_sel {
> +	TH1520_ADC_SINGLE_ENDED_INPUTS,
> +	TH1520_ADC_DIFFERENTIAL_INPUTS,
> +};
> +
> +enum selres_sel {
> +	TH1520_ADC_SELRES_6BIT = 6,
These seem unnecessary given the numeric values make sense without the
enum.
> +	TH1520_ADC_SELRES_8BIT = 8,
> +	TH1520_ADC_SELRES_10BIT = 10,
> +	TH1520_ADC_SELRES_12BIT = 12,
> +};
> +
> +enum offset_mode_sel {
> +	TH1520_ADC_OFFSET_DIS = 0,
> +	TH1520_ADC_OFFSET_EN,
> +};
> +
> +enum conversion_mode_sel {
> +	TH1520_ADC_MODE_SINGLE,
> +	TH1520_ADC_MODE_CONTINOUS,
> +};
> +
> +enum clk_sel {
> +	TH1520_ADC_FCLK_TYP_1M,
> +};
> +
> +enum int_actual_mask {
> +	TH1520_ADC_ACTUAL_CH0,
> +	TH1520_ADC_ACTUAL_CH1,
> +	TH1520_ADC_ACTUAL_ALL,
> +

Excess blank line.

> +};
> +
> +enum int_delta_mask {
> +	TH1520_ADC_DETAL_CH0,
> +	TH1520_ADC_DETAL_CH1,
> +	TH1520_ADC_DETAL_ALL,
> +};
> +
> +struct th1520_adc_feature {
> +	enum selres_sel			selres_sel;
> +	enum input_mode_sel		input_mode;
> +	enum vol_ref			vol_ref;
> +	enum offset_mode_sel		offset_mode;
> +	enum conversion_mode_sel	conv_mode;
> +	enum clk_sel			clk_sel;
> +	enum int_actual_mask		int_actual;
> +	enum int_delta_mask		int_detal;
> +};
> +
> +struct th1520_adc {
> +	struct device			*dev;
> +	void __iomem			*regs;
> +	struct clk			*clk;
> +
> +	u32				vref_uv;
> +	u32				value;
> +	struct regulator		*vref;
> +	struct th1520_adc_feature	adc_feature;
> +	u32				current_clk;
> +	u32				ch0_offmeas;
> +	u32				ch1_offmeas;
As above - don't introduce this stuff until it is used by features
in the driver. Just hard code the defaults when you write the registers.
Then when control is enabled in a future patch, it's much more obvious
what has actually changed.

> +
> +	struct completion		completion;
> +	/* lock to protect against multiple access to the device */

More info please on why this is a problem.  Do we have read modify write
cycles, or sequences where we need to not change the config whilst
an ADC read is happening?

> +	struct mutex			mlock;
> +};
> +


^ permalink raw reply

* Re: [PATCH v6 1/3] dt-bindings: dmaengine: Add dma multiplexer for CV18XX/SG200X series SoC
From: Krzysztof Kozlowski @ 2024-03-30 18:17 UTC (permalink / raw)
  To: Inochi Amaoto, Vinod Koul, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Chen Wang, Paul Walmsley, Palmer Dabbelt, Albert Ou
  Cc: Jisheng Zhang, Liu Gui, Jingbao Qiu, dlan, dmaengine, devicetree,
	linux-kernel, linux-riscv
In-Reply-To: <IA1PR20MB4953DDD8E89675650E0E2529BB3A2@IA1PR20MB4953.namprd20.prod.outlook.com>

On 29/03/2024 03:04, Inochi Amaoto wrote:
> The DMA IP of Sophgo CV18XX/SG200X is based on a DW AXI CORE, with
> an additional channel remap register located in the top system control
> area. The DMA channel is exclusive to each core.
> 
> In addition, the DMA multiplexer is a subdevice of system controller,
> so this binding only contains necessary properties for the multiplexer
> itself.
> 
> Add the dmamux binding for CV18XX/SG200X series SoC.
> 

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v3 net-next 1/2] dt-bindings: net: airoha,en8811h: Add en8811h
From: Krzysztof Kozlowski @ 2024-03-30 18:13 UTC (permalink / raw)
  To: Eric Woudstra, Rob Herring
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Krzysztof Kozlowski, Conor Dooley, Andrew Lunn, Heiner Kallweit,
	Russell King, Matthias Brugger, AngeloGioacchino Del Regno,
	Frank Wunderlich, Daniel Golle, Lucien Jheng, Zhi-Jun You, netdev,
	devicetree
In-Reply-To: <9ea90a1a-37b2-4baf-94af-2b89276a625d@gmail.com>

On 26/03/2024 21:41, Eric Woudstra wrote:
> Hi Rob,
> 
> On 3/26/24 20:29, Rob Herring wrote:
>> On Tue, Mar 26, 2024 at 05:23:04PM +0100, Eric Woudstra wrote:
>>> Add the Airoha EN8811H 2.5 Gigabit PHY.
>>>
>>> The en8811h phy can be set with serdes polarity reversed on rx and/or tx.
>>>
>>> Signed-off-by: Eric Woudstra <ericwouds@gmail.com>
>>
>> Did you change something or forget to add Krzysztof's Reviewed-by?
> 
> Nothing has changed in this commit. I was wondering if I should do this,
> so I should have added the Reviewed-by Krzysztof.
> 

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577

Please carefully read above guideline. Entire.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH 1/2] dt-bindings: arm: sunxi: document Tanix TX1 name
From: Krzysztof Kozlowski @ 2024-03-30 18:11 UTC (permalink / raw)
  To: Andre Przywara, Chen-Yu Tsai, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Jernej Skrabec, Samuel Holland
  Cc: devicetree, linux-sunxi
In-Reply-To: <20240330013243.17943-2-andre.przywara@arm.com>

On 30/03/2024 02:32, Andre Przywara wrote:
> The Tanix TX1 is a tiny TV box with the Allwinner H313 SoC, a lower bin
> version of the Allwinner H616. It comes with no SD card slot or Ethernet
> port.
> 
> Add the board/SoC compatible string pair to the list of known boards.
> Since the H313 does not look different from a software point of view,
> we keep the H616 compatible string.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: panel: Add LG SW43408 MIPI-DSI panel
From: Krzysztof Kozlowski @ 2024-03-30 18:10 UTC (permalink / raw)
  To: Dmitry Baryshkov, Sumit Semwal, Caleb Connolly, Neil Armstrong,
	Jessica Zhang, Sam Ravnborg, David Airlie, Daniel Vetter,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: dri-devel, devicetree, linux-kernel, linux-arm-msm, Vinod Koul,
	Caleb Connolly
In-Reply-To: <20240330-lg-sw43408-panel-v2-1-293a58717b38@linaro.org>

On 30/03/2024 16:00, Dmitry Baryshkov wrote:
> From: Sumit Semwal <sumit.semwal@linaro.org>
> 
> LG SW43408 is 1080x2160, 4-lane MIPI-DSI panel present on Google Pixel 3
> phones.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Sumit Semwal <sumit.semwal@linaro.org>
> [caleb: convert to yaml]
> Signed-off-by: Caleb Connolly <caleb@connolly.tech>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof


^ permalink raw reply

* Re: [RFC PATCH v2 1/2] spi: dt-bindings: add Siflower Quad SPI controller
From: Krzysztof Kozlowski @ 2024-03-30 17:43 UTC (permalink / raw)
  To: Qingfang Deng, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Qingfang Deng, linux-spi, devicetree, linux-kernel
In-Reply-To: <20240329054657.1602450-1-dqfext@gmail.com>

On 29/03/2024 06:46, Qingfang Deng wrote:
> From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> 
> Add YAML devicetree bindings for Siflower Quad SPI controller.

Not much improved.

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

Allow people to actually review your code - give them some time, like
24h between postings.

> 
> Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> ---
> v2: fix dt_binding_check reported errors
> 
>  .../bindings/spi/siflower,qspi.yaml           | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/siflower,qspi.yaml b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> new file mode 100644
> index 000000000000..15ce25a2176a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/siflower,qspi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Siflower Quad Serial Peripheral Interface (QSPI)
> +
> +maintainers:
> +  - Qingfang Deng <qingfang.deng@siflower.com.cn>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: siflower,qspi
> +

My previous comments stay valid. Respond to them.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [RFC PATCH 1/2] spi: dt-bindings: add Siflower Quad SPI controller
From: Krzysztof Kozlowski @ 2024-03-30 17:42 UTC (permalink / raw)
  To: Qingfang Deng, Mark Brown, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Qingfang Deng, linux-spi, devicetree, linux-kernel
In-Reply-To: <20240329015147.1481349-1-dqfext@gmail.com>

On 29/03/2024 02:51, Qingfang Deng wrote:
> From: Qingfang Deng <qingfang.deng@siflower.com.cn>
> 
> Add YAML devicetree bindings for Siflower Quad SPI controller.

Describe the hardware. What is this Siflower?
> 
> Signed-off-by: Qingfang Deng <qingfang.deng@siflower.com.cn>
> ---
>  .../bindings/spi/siflower,qspi.yaml           | 54 +++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/siflower,qspi.yaml b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> new file mode 100644
> index 000000000000..c2dbe82affc2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/siflower,qspi.yaml
> @@ -0,0 +1,54 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/siflower,qspi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Siflower Quad Serial Peripheral Interface (QSPI)
> +
> +maintainers:
> +  - Qingfang Deng <qingfang.deng@siflower.com.cn>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    const: siflower,qspi

Except that this was not tested, aren't you adding it for some SoC? If
so, then you miss here SoC part.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit
From: Fabio Estevam @ 2024-03-30 17:03 UTC (permalink / raw)
  To: Gilles Talis
  Cc: devicetree, imx, linux-arm-kernel, conor+dt,
	krzysztof.kozlowski+dt, robh, shawnguo, alex, andrew
In-Reply-To: <20240330133410.41408-4-gilles.talis@gmail.com>

Hi Gilles,

On Sat, Mar 30, 2024 at 10:34 AM Gilles Talis <gilles.talis@gmail.com> wrote:
>
> The Emcraft Systems NavQ+ kit is a mobile robotics platform
> based on NXP i.MX8 MPlus SoC.
>
> The following interfaces and devices are enabled:
> - eMMC
> - Gigabit Ethernet
> - RTC
> - SD-Card
> - UART console
>
> Signed-off-by: Gilles Talis <gilles.talis@gmail.com>

Reviewed-by: Fabio Estevam <festevam@gmail.com>

^ permalink raw reply

* Re: [PATCH v2 3/3] arm64: dts: freescale: Add device tree for Emcraft Systems NavQ+ Kit
From: Andrew Lunn @ 2024-03-30 16:46 UTC (permalink / raw)
  To: Gilles Talis
  Cc: devicetree, imx, linux-arm-kernel, conor+dt,
	krzysztof.kozlowski+dt, robh, shawnguo, festevam, alex
In-Reply-To: <20240330133410.41408-4-gilles.talis@gmail.com>

> +&eqos {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&pinctrl_eqos>;
> +	phy-mode = "rgmii-id";
> +	phy-handle = <&ethphy0>;
> +	status = "okay";
> +
> +	mdio {
> +		compatible = "snps,dwmac-mdio";
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		ethphy0: ethernet-phy@0 {
> +			compatible = "ethernet-phy-ieee802.3-c22";
> +			reg = <0>;
> +			reset-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;
> +			reset-assert-us = <1000>;
> +			reset-deassert-us = <10000>;
> +			qca,disable-smarteee;
> +			qca,disable-hibernation-mode;
> +		};
> +	};
> +};

Thanks for removing the regulator.

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox