[PATCH V1 0/3] ASoC: codecs: Add aw87390 amplifier driver

[PATCH V1 0/3] ASoC: codecs: Add aw87390 amplifier driver

[wangweidong.a]

From: Weidong Wang <wangweidong.a@awinic.com>

The awinic aw87390 is a new high efficiency, low noise,
constant large volume, 6th Smart K audio amplifier.

Add a DT schema for describing awinic aw87390 audio amplifiers.
They are controlled using I2C.

Weidong Wang (3):
  ASoC: dt-bindings: Add schema for "awinic,aw87390"
  ASoC: codecs: Add code for bin parsing compatible with aw87390
  ASoC: codecs: Add aw87390 amplifier driver

 .../bindings/sound/awinic,aw87390.yaml        |  43 ++
 sound/soc/codecs/Kconfig                      |  15 +-
 sound/soc/codecs/Makefile                     |   2 +
 sound/soc/codecs/aw87390.c                    | 463 ++++++++++++++++++
 sound/soc/codecs/aw87390.h                    |  85 ++++
 sound/soc/codecs/aw88395/aw88395_lib.c        |  23 +-
 sound/soc/codecs/aw88395/aw88395_reg.h        |   1 +
 7 files changed, 621 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/sound/awinic,aw87390.yaml
 create mode 100644 sound/soc/codecs/aw87390.c
 create mode 100644 sound/soc/codecs/aw87390.h


base-commit: 708283abf896dd4853e673cc8cba70acaf9bf4ea
-- 
2.41.0

[PATCH V1 1/3] ASoC: dt-bindings: Add schema for "awinic,aw87390"

[wangweidong.a]

From: Weidong Wang <wangweidong.a@awinic.com>

Add a DT schema for describing awinic aw87390 audio amplifiers.
They are controlled using I2C.

Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
---
 .../bindings/sound/awinic,aw87390.yaml        | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/awinic,aw87390.yaml

diff --git a/Documentation/devicetree/bindings/sound/awinic,aw87390.yaml b/Documentation/devicetree/bindings/sound/awinic,aw87390.yaml
new file mode 100644
index 000000000000..b4de99c9830e
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/awinic,aw87390.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/awinic,aw87390.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Awinic Aw87390 Audio Amplifier
+
+maintainers:
+  - Weidong Wang <wangweidong.a@awinic.com>
+
+description:
+  The awinic aw87390 is specifically designed to improve
+  the musical output dynamic range, enhance the overall
+  sound quallity, which is a new high efficiency, low
+  noise, constant large volume, 6th Smart K audio amplifier.
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    const: awinic,aw87390
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        audio-codec@58 {
+            compatible = "awinic,aw87390";
+            reg = <0x58>;
+        };
+    };
-- 
2.41.0

Re: [PATCH V1 1/3] ASoC: dt-bindings: Add schema for "awinic,aw87390"

[Krzysztof Kozlowski]

On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:
> From: Weidong Wang <wangweidong.a@awinic.com>
> 
> Add a DT schema for describing awinic aw87390 audio amplifiers.
> They are controlled using I2C.

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


> +
> +allOf:
> +  - $ref: dai-common.yaml#
> +
> +properties:
> +  compatible:
> +    const: awinic,aw87390
> +
> +  reg:
> +    maxItems: 1

No reset-gpios? Shouldn't this be just merged with awinic,aw88395 bindings?

Missing sound-dai-cells (const: 0 or 1)

> +
> +required:
> +  - compatible
> +  - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        audio-codec@58 {
> +            compatible = "awinic,aw87390";
> +            reg = <0x58>;

Please add sound-dai-cells for the example to be complete.

Best regards,
Krzysztof

Re: [PATCH V1 1/3] ASoC: dt-bindings: Add schema for "awinic,aw87390"

[wangweidong.a]

Thank you very much for your review. 
I would like to discuss something with you

On 04/09/2023 12:14, krzysztof.kozlowski@linaro.org wrote:
> On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:
>> From: Weidong Wang <wangweidong.a@awinic.com>
>> 
>> Add a DT schema for describing awinic aw87390 audio amplifiers.
>> They are controlled using I2C.

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

>> +
>> +allOf:
>> +  - $ref: dai-common.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: awinic,aw87390
>> +
>> +  reg:
>> +    maxItems: 1

> No reset-gpios? Shouldn't this be just merged with awinic,aw88395 bindings?

Yes, this chip does not have reset-gpios, and the i2c address of 
this chip is different from the aw88395 chip.So I didn't 
merge it with awinic, aw88395.

> Missing sound-dai-cells (const: 0 or 1)

Thank you very much. I'll add #sound-dai-cells

>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +        audio-codec@58 {
>> +            compatible = "awinic,aw87390";
>> +            reg = <0x58>;

> Please add sound-dai-cells for the example to be complete.

Thank you very much, I will modify it in patch v2

Best regards,
Weidong Wang

Re: [PATCH V1 1/3] ASoC: dt-bindings: Add schema for "awinic,aw87390"

[Krzysztof Kozlowski]

On 05/09/2023 05:31, wangweidong.a@awinic.com wrote:
>>> +examples:
>>> +  - |
>>> +    i2c {
>>> +        #address-cells = <1>;
>>> +        #size-cells = <0>;
>>> +        audio-codec@58 {
>>> +            compatible = "awinic,aw87390";
>>> +            reg = <0x58>;
> 
>> Please add sound-dai-cells for the example to be complete.
> 
> Thank you very much, I will modify it in patch v2

I expect in example all properties your device can use. Not only some
subset you want to add just to satisfy my comment above.

Best regards,
Krzysztof

[PATCH V1 2/3] ASoC: codecs: Add code for bin parsing compatible with aw87390

[wangweidong.a]

From: Weidong Wang <wangweidong.a@awinic.com>

Add aw87390 compatible code to the aw88395_lib.c file
so that it can parse aw87390's bin file.

Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
---
 sound/soc/codecs/aw88395/aw88395_lib.c | 23 ++++++++++++++---------
 sound/soc/codecs/aw88395/aw88395_reg.h |  1 +
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/sound/soc/codecs/aw88395/aw88395_lib.c b/sound/soc/codecs/aw88395/aw88395_lib.c
index 8ee1baa03269..9d561ae27feb 100644
--- a/sound/soc/codecs/aw88395/aw88395_lib.c
+++ b/sound/soc/codecs/aw88395/aw88395_lib.c
@@ -455,9 +455,11 @@ static int aw_dev_parse_reg_bin_with_hdr(struct aw_device *aw_dev,
 		goto parse_bin_failed;
 	}
 
-	if (aw_bin->header_info[0].valid_data_len % 4) {
-		dev_err(aw_dev->dev, "bin data len get error!");
-		goto parse_bin_failed;
+	if (aw_dev->chip_id == AW88261_CHIP_ID) {
+		if (aw_bin->header_info[0].valid_data_len % 4) {
+			dev_err(aw_dev->dev, "bin data len get error!");
+			goto parse_bin_failed;
+		}
 	}
 
 	prof_desc->sec_desc[AW88395_DATA_TYPE_REG].data =
@@ -579,9 +581,9 @@ static int aw_dev_parse_dev_default_type(struct aw_device *aw_dev,
 }
 
 static int aw88261_dev_cfg_get_valid_prof(struct aw_device *aw_dev,
-				struct aw_all_prof_info all_prof_info)
+				struct aw_all_prof_info *all_prof_info)
 {
-	struct aw_prof_desc *prof_desc = all_prof_info.prof_desc;
+	struct aw_prof_desc *prof_desc = all_prof_info->prof_desc;
 	struct aw_prof_info *prof_info = &aw_dev->prof_info;
 	int num = 0;
 	int i;
@@ -621,9 +623,9 @@ static int aw88261_dev_cfg_get_valid_prof(struct aw_device *aw_dev,
 }
 
 static int aw88395_dev_cfg_get_valid_prof(struct aw_device *aw_dev,
-				struct aw_all_prof_info all_prof_info)
+				struct aw_all_prof_info *all_prof_info)
 {
-	struct aw_prof_desc *prof_desc = all_prof_info.prof_desc;
+	struct aw_prof_desc *prof_desc = all_prof_info->prof_desc;
 	struct aw_prof_info *prof_info = &aw_dev->prof_info;
 	struct aw_sec_data_desc *sec_desc;
 	int num = 0;
@@ -701,12 +703,13 @@ static int aw_dev_load_cfg_by_hdr(struct aw_device *aw_dev,
 
 	switch (aw_dev->chip_id) {
 	case AW88395_CHIP_ID:
-		ret = aw88395_dev_cfg_get_valid_prof(aw_dev, *all_prof_info);
+		ret = aw88395_dev_cfg_get_valid_prof(aw_dev, all_prof_info);
 		if (ret < 0)
 			goto exit;
 		break;
 	case AW88261_CHIP_ID:
-		ret = aw88261_dev_cfg_get_valid_prof(aw_dev, *all_prof_info);
+	case AW87390_CHIP_ID:
+		ret = aw88261_dev_cfg_get_valid_prof(aw_dev, all_prof_info);
 		if (ret < 0)
 			goto exit;
 		break;
@@ -799,6 +802,7 @@ static int aw_get_dev_scene_count_v1(struct aw_device *aw_dev, struct aw_contain
 		ret = 0;
 		break;
 	case AW88261_CHIP_ID:
+	case AW87390_CHIP_ID:
 		for (i = 0; i < cfg_hdr->ddt_num; ++i) {
 			if (((cfg_dde[i].data_type == ACF_SEC_TYPE_REG) ||
 			     (cfg_dde[i].data_type == ACF_SEC_TYPE_HDR_REG)) &&
@@ -839,6 +843,7 @@ static int aw_get_default_scene_count_v1(struct aw_device *aw_dev,
 		ret = 0;
 		break;
 	case AW88261_CHIP_ID:
+	case AW87390_CHIP_ID:
 		for (i = 0; i < cfg_hdr->ddt_num; ++i) {
 			if (((cfg_dde[i].data_type == ACF_SEC_TYPE_REG) ||
 			     (cfg_dde[i].data_type == ACF_SEC_TYPE_HDR_REG)) &&
diff --git a/sound/soc/codecs/aw88395/aw88395_reg.h b/sound/soc/codecs/aw88395/aw88395_reg.h
index e7a7c02efaf3..d0a273387313 100644
--- a/sound/soc/codecs/aw88395/aw88395_reg.h
+++ b/sound/soc/codecs/aw88395/aw88395_reg.h
@@ -97,6 +97,7 @@
 enum aw88395_id {
 	AW88395_CHIP_ID = 0x2049,
 	AW88261_CHIP_ID = 0x2113,
+	AW87390_CHIP_ID = 0x76,
 };
 
 #define AW88395_REG_MAX		(0x7D)
-- 
2.41.0

[PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[wangweidong.a]

From: Weidong Wang <wangweidong.a@awinic.com>

Add i2c and amplifier registration for aw87390 and
their associated operation functions.

Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
---
 sound/soc/codecs/Kconfig   |  15 +-
 sound/soc/codecs/Makefile  |   2 +
 sound/soc/codecs/aw87390.c | 463 +++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/aw87390.h |  85 +++++++
 4 files changed, 563 insertions(+), 2 deletions(-)
 create mode 100644 sound/soc/codecs/aw87390.c
 create mode 100644 sound/soc/codecs/aw87390.h

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 95b5bd883215..9f4311b13d6e 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -54,6 +54,7 @@ config SND_SOC_ALL_CODECS
 	imply SND_SOC_ALC5632
 	imply SND_SOC_AUDIO_IIO_AUX
 	imply SND_SOC_AW8738
+	imply SND_SOC_AW87390
 	imply SND_SOC_AW88395
 	imply SND_SOC_AW88261
 	imply SND_SOC_BT_SCO
@@ -638,12 +639,12 @@ config SND_SOC_AW8738
 	  operation mode using the Awinic-specific one-wire pulse control.
 
 config SND_SOC_AW88395_LIB
+	select CRC8
 	tristate
 
 config SND_SOC_AW88395
 	tristate "Soc Audio for awinic aw88395"
 	depends on I2C
-	select CRC8
 	select CRC32
 	select REGMAP_I2C
 	select GPIOLIB
@@ -657,7 +658,6 @@ config SND_SOC_AW88395
 config SND_SOC_AW88261
 	tristate "Soc Audio for awinic aw88261"
 	depends on I2C
-	select CRC8
 	select REGMAP_I2C
 	select GPIOLIB
 	select SND_SOC_AW88395_LIB
@@ -668,6 +668,17 @@ config SND_SOC_AW88261
 	  boost converter can be adjusted smartly according to
 	  the input amplitude.
 
+config SND_SOC_AW87390
+	tristate "Soc Audio for awinic aw87390"
+	depends on I2C
+	select REGMAP_I2C
+	select SND_SOC_AW88395_LIB
+	help
+	  The awinic aw87390 is specifically designed to improve
+	  the musical output dynamic range, enhance the overall
+	  sound quallity, which is a new high efficiency, low
+	  noise, constant large volume, 6th Smart K audio amplifier.
+
 config SND_SOC_BD28623
 	tristate "ROHM BD28623 CODEC"
 	help
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index c8502a49b40a..12bf52e23d65 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -47,6 +47,7 @@ snd-soc-ak5558-objs := ak5558.o
 snd-soc-arizona-objs := arizona.o arizona-jack.o
 snd-soc-audio-iio-aux-objs := audio-iio-aux.o
 snd-soc-aw8738-objs := aw8738.o
+snd-soc-aw87390-objs := aw87390.o
 snd-soc-aw88395-lib-objs := aw88395/aw88395_lib.o
 snd-soc-aw88395-objs := aw88395/aw88395.o \
 			aw88395/aw88395_device.o
@@ -433,6 +434,7 @@ obj-$(CONFIG_SND_SOC_ALC5632)	+= snd-soc-alc5632.o
 obj-$(CONFIG_SND_SOC_ARIZONA)	+= snd-soc-arizona.o
 obj-$(CONFIG_SND_SOC_AUDIO_IIO_AUX)	+= snd-soc-audio-iio-aux.o
 obj-$(CONFIG_SND_SOC_AW8738)	+= snd-soc-aw8738.o
+obj-$(CONFIG_SND_SOC_AW87390)	+= snd-soc-aw87390.o
 obj-$(CONFIG_SND_SOC_AW88395_LIB) += snd-soc-aw88395-lib.o
 obj-$(CONFIG_SND_SOC_AW88395)	+=snd-soc-aw88395.o
 obj-$(CONFIG_SND_SOC_AW88261)	+=snd-soc-aw88261.o
diff --git a/sound/soc/codecs/aw87390.c b/sound/soc/codecs/aw87390.c
new file mode 100644
index 000000000000..cf3065b0a73e
--- /dev/null
+++ b/sound/soc/codecs/aw87390.c
@@ -0,0 +1,463 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// aw87390.c  --  AW87390 ALSA SoC Audio driver
+//
+// Copyright (c) 2023 awinic Technology CO., LTD
+//
+// Author: Weidong Wang <wangweidong.a@awinic.com>
+//
+
+#include <linux/i2c.h>
+#include <linux/firmware.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+#include <sound/soc.h>
+#include "aw87390.h"
+#include "aw88395/aw88395_data_type.h"
+#include "aw88395/aw88395_device.h"
+
+static const struct regmap_config aw87390_remap_config = {
+	.val_bits = 8,
+	.reg_bits = 8,
+	.max_register = AW87390_REG_MAX,
+	.reg_format_endian = REGMAP_ENDIAN_LITTLE,
+	.val_format_endian = REGMAP_ENDIAN_BIG,
+};
+
+static int aw87390_dev_reg_update(struct aw_device *aw_dev,
+					unsigned char *data, unsigned int len)
+{
+	int i, ret;
+
+	if (!data) {
+		dev_err(aw_dev->dev, "data is NULL\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < len; i = i + 2) {
+		if (data[i] == AW87390_DELAY_REG_ADDR) {
+			usleep_range(data[i + 1] * AW87390_REG_DELAY_TIME,
+					data[i + 1] * AW87390_REG_DELAY_TIME + 10);
+			continue;
+		}
+		ret = regmap_write(aw_dev->regmap, data[i], data[i + 1]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static char *aw87390_dev_get_prof_name(struct aw_device *aw_dev, int index)
+{
+	struct aw_prof_info *prof_info = &aw_dev->prof_info;
+	struct aw_prof_desc *prof_desc;
+
+	if ((index >= aw_dev->prof_info.count) || (index < 0)) {
+		dev_err(aw_dev->dev, "index[%d] overflow count[%d]\n",
+			index, aw_dev->prof_info.count);
+		return NULL;
+	}
+
+	prof_desc = &aw_dev->prof_info.prof_desc[index];
+
+	return prof_info->prof_name_list[prof_desc->id];
+}
+
+static int aw87390_dev_get_prof_data(struct aw_device *aw_dev, int index,
+			struct aw_prof_desc **prof_desc)
+{
+	if ((index >= aw_dev->prof_info.count) || (index < 0)) {
+		dev_err(aw_dev->dev, "%s: index[%d] overflow count[%d]\n",
+				__func__, index, aw_dev->prof_info.count);
+		return -EINVAL;
+	}
+
+	*prof_desc = &aw_dev->prof_info.prof_desc[index];
+
+	return 0;
+}
+
+static int aw87390_dev_fw_update(struct aw_device *aw_dev)
+{
+	struct aw_prof_desc *prof_index_desc;
+	struct aw_sec_data_desc *sec_desc;
+	char *prof_name;
+	int ret;
+
+	prof_name = aw87390_dev_get_prof_name(aw_dev, aw_dev->prof_index);
+	if (!prof_name) {
+		dev_err(aw_dev->dev, "get prof name failed\n");
+		return -EINVAL;
+	}
+
+	dev_dbg(aw_dev->dev, "start update %s", prof_name);
+
+	ret = aw87390_dev_get_prof_data(aw_dev, aw_dev->prof_index, &prof_index_desc);
+	if (ret) {
+		dev_err(aw_dev->dev, "aw87390_dev_get_prof_data failed\n");
+		return ret;
+	}
+
+	/* update reg */
+	sec_desc = prof_index_desc->sec_desc;
+	ret = aw87390_dev_reg_update(aw_dev, sec_desc[AW88395_DATA_TYPE_REG].data,
+					sec_desc[AW88395_DATA_TYPE_REG].len);
+	if (ret) {
+		dev_err(aw_dev->dev, "update reg failed\n");
+		return ret;
+	}
+
+	aw_dev->prof_cur = aw_dev->prof_index;
+
+	return ret;
+}
+
+static int aw87390_power_off(struct aw_device *aw_dev)
+{
+	int ret;
+
+	if (aw_dev->status == AW87390_DEV_PW_OFF) {
+		dev_info(aw_dev->dev, "already power off\n");
+		return 0;
+	}
+
+	ret = regmap_write(aw_dev->regmap, AW87390_SYSCTRL_REG, AW87390_POWER_DOWN_VALUE);
+	if (ret)
+		return ret;
+	aw_dev->status = AW87390_DEV_PW_OFF;
+
+	return ret;
+}
+
+static int aw87390_power_on(struct aw_device *aw_dev)
+{
+	int ret;
+
+	if (aw_dev->status == AW87390_DEV_PW_ON) {
+		dev_info(aw_dev->dev, "already power on\n");
+		return 0;
+	}
+
+	if (!aw_dev->fw_status) {
+		dev_err(aw_dev->dev, "fw not load\n");
+		return -EINVAL;
+	}
+
+	ret = regmap_write(aw_dev->regmap, AW87390_SYSCTRL_REG, AW87390_POWER_DOWN_VALUE);
+	if (ret)
+		return ret;
+
+	ret = aw87390_dev_fw_update(aw_dev);
+	if (ret) {
+		dev_err(aw_dev->dev, "%s load profile failed\n", __func__);
+		return ret;
+	}
+	aw_dev->status = AW87390_DEV_PW_ON;
+
+	return ret;
+}
+
+static int aw87390_dev_set_profile_index(struct aw_device *aw_dev, int index)
+{
+	if ((index >= aw_dev->prof_info.count) || (index < 0))
+		return -EINVAL;
+
+	if (aw_dev->prof_index == index)
+		return -EPERM;
+
+	aw_dev->prof_index = index;
+
+	return 0;
+}
+
+static int aw87390_profile_info(struct snd_kcontrol *kcontrol,
+			 struct snd_ctl_elem_info *uinfo)
+{
+	struct snd_soc_component *codec = snd_soc_kcontrol_component(kcontrol);
+	struct aw87390 *aw87390 = snd_soc_component_get_drvdata(codec);
+	const char *prof_name;
+	char *name;
+	int count;
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED;
+	uinfo->count = 1;
+
+	count = aw87390->aw_pa->prof_info.count;
+	if (count <= 0) {
+		uinfo->value.enumerated.items = 0;
+		return 0;
+	}
+
+	uinfo->value.enumerated.items = count;
+
+	if (uinfo->value.enumerated.item >= count)
+		uinfo->value.enumerated.item = count - 1;
+
+	name = uinfo->value.enumerated.name;
+	count = uinfo->value.enumerated.item;
+
+	prof_name = aw87390_dev_get_prof_name(aw87390->aw_pa, count);
+	if (!prof_name) {
+		strscpy(uinfo->value.enumerated.name, "null",
+						strlen("null") + 1);
+		return 0;
+	}
+
+	strscpy(name, prof_name, sizeof(uinfo->value.enumerated.name));
+
+	return 0;
+}
+
+static int aw87390_profile_get(struct snd_kcontrol *kcontrol,
+			struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *codec = snd_soc_kcontrol_component(kcontrol);
+	struct aw87390 *aw87390 = snd_soc_component_get_drvdata(codec);
+
+	ucontrol->value.integer.value[0] = aw87390->aw_pa->prof_index;
+
+	return 0;
+}
+
+static int aw87390_profile_set(struct snd_kcontrol *kcontrol,
+		struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *codec = snd_soc_kcontrol_component(kcontrol);
+	struct aw87390 *aw87390 = snd_soc_component_get_drvdata(codec);
+	int ret;
+
+	mutex_lock(&aw87390->lock);
+	ret = aw87390_dev_set_profile_index(aw87390->aw_pa, ucontrol->value.integer.value[0]);
+	if (ret) {
+		dev_dbg(codec->dev, "profile index does not change\n");
+		mutex_unlock(&aw87390->lock);
+		return 0;
+	}
+
+	if (aw87390->aw_pa->status == AW87390_DEV_PW_ON) {
+		aw87390_power_off(aw87390->aw_pa);
+		aw87390_power_on(aw87390->aw_pa);
+	}
+
+	mutex_unlock(&aw87390->lock);
+
+	return 1;
+}
+
+static const struct snd_kcontrol_new aw87390_controls[] = {
+	AW87390_PROFILE_EXT("AW87390 Profile Set", aw87390_profile_info,
+		aw87390_profile_get, aw87390_profile_set),
+};
+
+static int aw87390_request_firmware_file(struct aw87390 *aw87390)
+{
+	const struct firmware *cont = NULL;
+	int ret;
+
+	aw87390->aw_pa->fw_status = AW87390_DEV_FW_FAILED;
+
+	ret = request_firmware(&cont, AW87390_ACF_FILE, aw87390->aw_pa->dev);
+	if (ret)
+		return dev_err_probe(aw87390->aw_pa->dev, ret,
+					"load [%s] failed!\n", AW87390_ACF_FILE);
+
+	dev_dbg(aw87390->aw_pa->dev, "loaded %s - size: %zu\n",
+			AW87390_ACF_FILE, cont ? cont->size : 0);
+
+	aw87390->aw_cfg = devm_kzalloc(aw87390->aw_pa->dev, cont->size + sizeof(int), GFP_KERNEL);
+	if (!aw87390->aw_cfg) {
+		release_firmware(cont);
+		return -ENOMEM;
+	}
+
+	aw87390->aw_cfg->len = (int)cont->size;
+	memcpy(aw87390->aw_cfg->data, cont->data, cont->size);
+	release_firmware(cont);
+
+	ret = aw88395_dev_load_acf_check(aw87390->aw_pa, aw87390->aw_cfg);
+	if (ret) {
+		dev_err(aw87390->aw_pa->dev, "load [%s] failed !\n", AW87390_ACF_FILE);
+		return ret;
+	}
+
+	mutex_lock(&aw87390->lock);
+
+	ret = aw88395_dev_cfg_load(aw87390->aw_pa, aw87390->aw_cfg);
+	if (ret)
+		dev_err(aw87390->aw_pa->dev, "aw_dev acf parse failed\n");
+
+	mutex_unlock(&aw87390->lock);
+
+	return ret;
+}
+
+static int aw87390_drv_event(struct snd_soc_dapm_widget *w,
+				struct snd_kcontrol *kcontrol, int event)
+{
+	struct snd_soc_component *component = snd_soc_dapm_to_component(w->dapm);
+	struct aw87390 *aw87390 = snd_soc_component_get_drvdata(component);
+	struct aw_device *aw_dev = aw87390->aw_pa;
+	int ret;
+
+	switch (event) {
+	case SND_SOC_DAPM_PRE_PMU:
+		ret = aw87390_power_on(aw_dev);
+		break;
+	case SND_SOC_DAPM_POST_PMD:
+		ret = aw87390_power_off(aw_dev);
+		break;
+	default:
+		dev_err(aw_dev->dev, "%s: invalid event %d\n", __func__, event);
+		ret = -EINVAL;
+	}
+
+	return ret;
+}
+
+static const struct snd_soc_dapm_widget aw87390_dapm_widgets[] = {
+	SND_SOC_DAPM_INPUT("IN"),
+	SND_SOC_DAPM_PGA_E("SPK PA", SND_SOC_NOPM, 0, 0, NULL, 0, aw87390_drv_event,
+			       SND_SOC_DAPM_PRE_PMU | SND_SOC_DAPM_POST_PMD),
+	SND_SOC_DAPM_OUTPUT("OUT"),
+};
+
+static const struct snd_soc_dapm_route aw87390_dapm_routes[] = {
+	{ "SPK PA", NULL, "IN" },
+	{ "OUT", NULL, "SPK PA" },
+};
+
+static int aw87390_codec_probe(struct snd_soc_component *component)
+{
+	struct aw87390 *aw87390 = snd_soc_component_get_drvdata(component);
+	int ret;
+
+	ret = aw87390_request_firmware_file(aw87390);
+	if (ret)
+		return dev_err_probe(aw87390->aw_pa->dev, ret,
+				"aw87390_request_firmware_file failed\n");
+
+	return 0;
+}
+
+static const struct snd_soc_component_driver soc_codec_dev_aw87390 = {
+	.probe = aw87390_codec_probe,
+	.dapm_widgets = aw87390_dapm_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(aw87390_dapm_widgets),
+	.dapm_routes = aw87390_dapm_routes,
+	.num_dapm_routes = ARRAY_SIZE(aw87390_dapm_routes),
+	.controls = aw87390_controls,
+	.num_controls = ARRAY_SIZE(aw87390_controls),
+};
+
+static void aw87390_parse_channel_dt(struct aw87390 *aw87390)
+{
+	struct aw_device *aw_dev = aw87390->aw_pa;
+	struct device_node *np = aw_dev->dev->of_node;
+	u32 channel_value = AW87390_DEV_DEFAULT_CH;
+
+	of_property_read_u32(np, "sound-channel", &channel_value);
+
+	aw_dev->channel = channel_value;
+}
+
+static int aw87390_init(struct aw87390 **aw87390, struct i2c_client *i2c, struct regmap *regmap)
+{
+	struct aw_device *aw_dev;
+	unsigned int chip_id;
+	int ret;
+
+	/* read chip id */
+	ret = regmap_read(regmap, AW87390_ID_REG, &chip_id);
+	if (ret) {
+		dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret);
+		return ret;
+	}
+
+	if (chip_id != AW87390_CHIP_ID) {
+		dev_err(&i2c->dev, "unsupported device\n");
+		return -ENXIO;
+	}
+
+	dev_info(&i2c->dev, "chip id = 0x%x\n", chip_id);
+
+	aw_dev = devm_kzalloc(&i2c->dev, sizeof(*aw_dev), GFP_KERNEL);
+	if (!aw_dev)
+		return -ENOMEM;
+
+	(*aw87390)->aw_pa = aw_dev;
+	aw_dev->i2c = i2c;
+	aw_dev->regmap = regmap;
+	aw_dev->dev = &i2c->dev;
+	aw_dev->chip_id = AW87390_CHIP_ID;
+	aw_dev->acf = NULL;
+	aw_dev->prof_info.prof_desc = NULL;
+	aw_dev->prof_info.count = 0;
+	aw_dev->prof_info.prof_type = AW88395_DEV_NONE_TYPE_ID;
+	aw_dev->channel = AW87390_DEV_DEFAULT_CH;
+	aw_dev->fw_status = AW87390_DEV_FW_FAILED;
+	aw_dev->prof_index = AW87390_INIT_PROFILE;
+	aw_dev->status = AW87390_DEV_PW_OFF;
+
+	aw87390_parse_channel_dt(*aw87390);
+
+	return ret;
+}
+
+static int aw87390_i2c_probe(struct i2c_client *i2c)
+{
+	struct aw87390 *aw87390;
+	int ret;
+
+	ret = i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C);
+	if (!ret)
+		return dev_err_probe(&i2c->dev, -ENXIO, "check_functionality failed\n");
+
+	aw87390 = devm_kzalloc(&i2c->dev, sizeof(*aw87390), GFP_KERNEL);
+	if (!aw87390)
+		return -ENOMEM;
+
+	mutex_init(&aw87390->lock);
+
+	i2c_set_clientdata(i2c, aw87390);
+
+	aw87390->regmap = devm_regmap_init_i2c(i2c, &aw87390_remap_config);
+	if (IS_ERR(aw87390->regmap)) {
+		ret = PTR_ERR(aw87390->regmap);
+		return dev_err_probe(&i2c->dev, ret, "failed to init regmap: %d\n", ret);
+	}
+
+	/* aw pa init */
+	ret = aw87390_init(&aw87390, i2c, aw87390->regmap);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(aw87390->regmap, AW87390_ID_REG, AW87390_SOFT_RESET_VALUE);
+	if (ret)
+		return ret;
+
+	ret = devm_snd_soc_register_component(&i2c->dev,
+				&soc_codec_dev_aw87390, NULL, 0);
+	if (ret)
+		dev_err(&i2c->dev, "failed to register aw87390: %d\n", ret);
+
+	return ret;
+}
+
+static const struct i2c_device_id aw87390_i2c_id[] = {
+	{ AW87390_I2C_NAME, 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, aw87390_i2c_id);
+
+static struct i2c_driver aw87390_i2c_driver = {
+	.driver = {
+		.name = AW87390_I2C_NAME,
+	},
+	.probe = aw87390_i2c_probe,
+	.id_table = aw87390_i2c_id,
+};
+module_i2c_driver(aw87390_i2c_driver);
+
+MODULE_DESCRIPTION("ASoC AW87390 PA Driver");
+MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/codecs/aw87390.h b/sound/soc/codecs/aw87390.h
new file mode 100644
index 000000000000..468e6eb2e224
--- /dev/null
+++ b/sound/soc/codecs/aw87390.h
@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// aw87390.h  --  aw87390 ALSA SoC Audio driver
+//
+// Copyright (c) 2023 awinic Technology CO., LTD
+//
+// Author: Weidong Wang <wangweidong.a@awinic.com>
+//
+
+#ifndef __AW87390_H__
+#define __AW87390_H__
+
+#define AW87390_ID_REG			(0x00)
+#define AW87390_SYSCTRL_REG		(0x01)
+#define AW87390_MDCTRL_REG		(0x02)
+#define AW87390_CPOVP_REG		(0x03)
+#define AW87390_CPP_REG		(0x04)
+#define AW87390_PAG_REG		(0x05)
+#define AW87390_AGC3P_REG		(0x06)
+#define AW87390_AGC3PA_REG		(0x07)
+#define AW87390_AGC2P_REG		(0x08)
+#define AW87390_AGC2PA_REG		(0x09)
+#define AW87390_AGC1PA_REG		(0x0A)
+#define AW87390_SYSST_REG		(0x59)
+#define AW87390_SYSINT_REG		(0x60)
+#define AW87390_DFT_SYSCTRL_REG	(0x61)
+#define AW87390_DFT_MDCTRL_REG		(0x62)
+#define AW87390_DFT_CPADP_REG		(0x63)
+#define AW87390_DFT_AGCPA_REG		(0x64)
+#define AW87390_DFT_POFR_REG		(0x65)
+#define AW87390_DFT_OC_REG		(0x66)
+#define AW87390_DFT_ADP1_REG		(0x67)
+#define AW87390_DFT_REF_REG		(0x68)
+#define AW87390_DFT_LDO_REG		(0x69)
+#define AW87390_ADP1_REG		(0x70)
+#define AW87390_ADP2_REG		(0x71)
+#define AW87390_NG1_REG		(0x72)
+#define AW87390_NG2_REG		(0x73)
+#define AW87390_NG3_REG		(0x74)
+#define AW87390_CP_REG			(0x75)
+#define AW87390_AB_REG			(0x76)
+#define AW87390_TEST_REG		(0x77)
+#define AW87390_ENCR_REG		(0x78)
+#define AW87390_DELAY_REG_ADDR		(0xFE)
+
+#define AW87390_SOFT_RESET_VALUE	(0xAA)
+#define AW87390_POWER_DOWN_VALUE	(0x00)
+#define AW87390_REG_MAX		(0xFF)
+#define AW87390_DEV_DEFAULT_CH		(0)
+#define AW87390_INIT_PROFILE		(0)
+#define AW87390_REG_DELAY_TIME		(1000)
+#define AW87390_I2C_NAME		"aw87390_pa"
+#define AW87390_ACF_FILE		"aw87390_acf.bin"
+
+#define AW87390_PROFILE_EXT(xname, profile_info, profile_get, profile_set) \
+{ \
+	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, \
+	.name = xname, \
+	.info = profile_info, \
+	.get = profile_get, \
+	.put = profile_set, \
+}
+
+enum aw87390_id {
+	AW87390_CHIP_ID = 0x76,
+};
+
+enum {
+	AW87390_DEV_FW_FAILED = 0,
+	AW87390_DEV_FW_OK,
+};
+
+enum {
+	AW87390_DEV_PW_OFF = 0,
+	AW87390_DEV_PW_ON,
+};
+
+struct aw87390 {
+	struct aw_device *aw_pa;
+	struct mutex lock;
+	struct regmap *regmap;
+	struct aw_container *aw_cfg;
+};
+
+#endif
-- 
2.41.0

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[Krzysztof Kozlowski]

On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:
> From: Weidong Wang <wangweidong.a@awinic.com>
> 

...

> +static void aw87390_parse_channel_dt(struct aw87390 *aw87390)
> +{
> +	struct aw_device *aw_dev = aw87390->aw_pa;
> +	struct device_node *np = aw_dev->dev->of_node;
> +	u32 channel_value = AW87390_DEV_DEFAULT_CH;
> +
> +	of_property_read_u32(np, "sound-channel", &channel_value);

NAK, there is no such property. It seems you already sneaked in such for
other codecs. Please do not repeat such patterns of work.

That's also why I expect full DTS example, not some reduced pieces.

> +
> +	aw_dev->channel = channel_value;
> +}
> +
> +static int aw87390_init(struct aw87390 **aw87390, struct i2c_client *i2c, struct regmap *regmap)
> +{
> +	struct aw_device *aw_dev;
> +	unsigned int chip_id;
> +	int ret;
> +
> +	/* read chip id */
> +	ret = regmap_read(regmap, AW87390_ID_REG, &chip_id);
> +	if (ret) {
> +		dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret);
> +		return ret;
> +	}
> +
> +	if (chip_id != AW87390_CHIP_ID) {
> +		dev_err(&i2c->dev, "unsupported device\n");

Why? The compatible tells it cannot be anything else.

> +		return -ENXIO;
> +	}
> +
> +	dev_info(&i2c->dev, "chip id = 0x%x\n", chip_id);
> +
> +	aw_dev = devm_kzalloc(&i2c->dev, sizeof(*aw_dev), GFP_KERNEL);
> +	if (!aw_dev)
> +		return -ENOMEM;
> +
> +	(*aw87390)->aw_pa = aw_dev;
> +	aw_dev->i2c = i2c;
> +	aw_dev->regmap = regmap;
> +	aw_dev->dev = &i2c->dev;
> +	aw_dev->chip_id = AW87390_CHIP_ID;
> +	aw_dev->acf = NULL;
> +	aw_dev->prof_info.prof_desc = NULL;
> +	aw_dev->prof_info.count = 0;
> +	aw_dev->prof_info.prof_type = AW88395_DEV_NONE_TYPE_ID;
> +	aw_dev->channel = AW87390_DEV_DEFAULT_CH;
> +	aw_dev->fw_status = AW87390_DEV_FW_FAILED;
> +	aw_dev->prof_index = AW87390_INIT_PROFILE;
> +	aw_dev->status = AW87390_DEV_PW_OFF;
> +
> +	aw87390_parse_channel_dt(*aw87390);
> +
> +	return ret;
> +}
> +
> +static int aw87390_i2c_probe(struct i2c_client *i2c)
> +{
> +	struct aw87390 *aw87390;
> +	int ret;
> +
> +	ret = i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C);
> +	if (!ret)
> +		return dev_err_probe(&i2c->dev, -ENXIO, "check_functionality failed\n");
> +
> +	aw87390 = devm_kzalloc(&i2c->dev, sizeof(*aw87390), GFP_KERNEL);
> +	if (!aw87390)
> +		return -ENOMEM;
> +
> +	mutex_init(&aw87390->lock);
> +
> +	i2c_set_clientdata(i2c, aw87390);
> +
> +	aw87390->regmap = devm_regmap_init_i2c(i2c, &aw87390_remap_config);
> +	if (IS_ERR(aw87390->regmap)) {
> +		ret = PTR_ERR(aw87390->regmap);

ret is not needed here, so just:
return dev_err_probe()

> +		return dev_err_probe(&i2c->dev, ret, "failed to init regmap: %d\n", ret);
> +	}
> +
> +	/* aw pa init */
> +	ret = aw87390_init(&aw87390, i2c, aw87390->regmap);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(aw87390->regmap, AW87390_ID_REG, AW87390_SOFT_RESET_VALUE);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_snd_soc_register_component(&i2c->dev,
> +				&soc_codec_dev_aw87390, NULL, 0);
> +	if (ret)
> +		dev_err(&i2c->dev, "failed to register aw87390: %d\n", ret);
> +
> +	return ret;
> +}


Best regards,
Krzysztof

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

On Mon, Sep 04, 2023 at 02:17:43PM +0200, Krzysztof Kozlowski wrote:
> On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:

> > +	ret = regmap_read(regmap, AW87390_ID_REG, &chip_id);
> > +	if (ret) {
> > +		dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret);
> > +		return ret;
> > +	}

> > +	if (chip_id != AW87390_CHIP_ID) {
> > +		dev_err(&i2c->dev, "unsupported device\n");

> Why? The compatible tells it cannot be anything else.

This is very common good practice, as well as validating communication
with the device it verifies that the device descrbied in the DT is the
one that is actually present in the system.  This might create hassle
down the line if there is a backwards compatible upgrade but that's much
rarer for this class of hardware than cut'n'pasting of device trees.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[Krzysztof Kozlowski]

On 04/09/2023 14:30, Mark Brown wrote:
> On Mon, Sep 04, 2023 at 02:17:43PM +0200, Krzysztof Kozlowski wrote:
>> On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:
> 
>>> +	ret = regmap_read(regmap, AW87390_ID_REG, &chip_id);
>>> +	if (ret) {
>>> +		dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret);
>>> +		return ret;
>>> +	}
> 
>>> +	if (chip_id != AW87390_CHIP_ID) {
>>> +		dev_err(&i2c->dev, "unsupported device\n");
> 
>> Why? The compatible tells it cannot be anything else.
> 
> This is very common good practice, as well as validating communication

No, it is neither common nor good. The kernel's job is not to verify the
supplied DTS. Rob also made here a point:

https://lore.kernel.org/all/CAL_Jsq+wcrOjh7+0c=mrg+Qz6dbhOUE-VEeQ4FoWC3Y7ENoyfQ@mail.gmail.com/

> with the device it verifies that the device descrbied in the DT is the
> one that is actually present in the system.  This might create hassle
> down the line if there is a backwards compatible upgrade but that's much
> rarer for this class of hardware than cut'n'pasting of device trees.

Best regards,
Krzysztof

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

On Mon, Sep 04, 2023 at 03:02:31PM +0200, Krzysztof Kozlowski wrote:
> On 04/09/2023 14:30, Mark Brown wrote:

> > This is very common good practice, as well as validating communication

> No, it is neither common nor good. The kernel's job is not to verify the
> supplied DTS. Rob also made here a point:

It's certainly a common practice, even if you disagree that it's good.  

> https://lore.kernel.org/all/CAL_Jsq+wcrOjh7+0c=mrg+Qz6dbhOUE-VEeQ4FoWC3Y7ENoyfQ@mail.gmail.com/

That's a very different kind of error, I'm not clear how we expect
schema checks to identify a mismatch with schematics...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[Linus Walleij]

On Mon, Sep 4, 2023 at 3:02 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
> On 04/09/2023 14:30, Mark Brown wrote:
> > On Mon, Sep 04, 2023 at 02:17:43PM +0200, Krzysztof Kozlowski wrote:
> >> On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:
> >
> >>> +   ret = regmap_read(regmap, AW87390_ID_REG, &chip_id);
> >>> +   if (ret) {
> >>> +           dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret);
> >>> +           return ret;
> >>> +   }
> >
> >>> +   if (chip_id != AW87390_CHIP_ID) {
> >>> +           dev_err(&i2c->dev, "unsupported device\n");
> >
> >> Why? The compatible tells it cannot be anything else.
> >
> > This is very common good practice, as well as validating communication
>
> No, it is neither common nor good. The kernel's job is not to verify the
> supplied DTS. Rob also made here a point:
>
> https://lore.kernel.org/all/CAL_Jsq+wcrOjh7+0c=mrg+Qz6dbhOUE-VEeQ4FoWC3Y7ENoyfQ@mail.gmail.com/

I disagree, if a vendor one day decides to mount a new version of a
component without notifying the community or users this is really
helpful.

The function is named "probe()" for a reason, as in "inspect
the hardware and see what we find" this has always been the case
I think.

Yours,
Linus Walleij

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[wangweidong.a]

Thank you very much. Here are some things I'd like to discuss with you.

On 05/09/2023 11:50, krzysztof.kozlowski@linaro.org wrote:
> On 04/09/2023 13:46, wangweidong.a@awinic.com wrote:
>> From: Weidong Wang <wangweidong.a@awinic.com>
>> 

> ...

>> +static void aw87390_parse_channel_dt(struct aw87390 *aw87390)
>> +{
>> +	struct aw_device *aw_dev = aw87390->aw_pa;
>> +	struct device_node *np = aw_dev->dev->of_node;
>> +	u32 channel_value = AW87390_DEV_DEFAULT_CH;
>> +
>> +	of_property_read_u32(np, "sound-channel", &channel_value);

> NAK, there is no such property. It seems you already sneaked in such for
> other codecs. Please do not repeat such patterns of work.

> That's also why I expect full DTS example, not some reduced pieces.

Thank you very much. I would like to add a sound-channel property to awinic,aw87390.yaml
This property is used to distinguish between multiple PA's in order to 
load different configurations for different PA's

>> +
>> +	aw_dev->channel = channel_value;
>> +}
>> +
>> +static int aw87390_init(struct aw87390 **aw87390, struct i2c_client *i2c, struct regmap *regmap)
>> +{
>> +	struct aw_device *aw_dev;
>> +	unsigned int chip_id;
>> +	int ret;
>> +
>> +	/* read chip id */
>> +	ret = regmap_read(regmap, AW87390_ID_REG, &chip_id);
>> +	if (ret) {
>> +		dev_err(&i2c->dev, "%s read chipid error. ret = %d\n", __func__, ret);
>> +		return ret;
>> +	}
>> +
>> +	if (chip_id != AW87390_CHIP_ID) {
>> +		dev_err(&i2c->dev, "unsupported device\n");

> Why? The compatible tells it cannot be anything else.

>> +		return -ENXIO;
>> +	}
>> +
>> +	dev_info(&i2c->dev, "chip id = 0x%x\n", chip_id);
>> +
>> +	aw_dev = devm_kzalloc(&i2c->dev, sizeof(*aw_dev), GFP_KERNEL);
>> +	if (!aw_dev)
>> +		return -ENOMEM;
>> +
>> +	(*aw87390)->aw_pa = aw_dev;
>> +	aw_dev->i2c = i2c;
>> +	aw_dev->regmap = regmap;
>> +	aw_dev->dev = &i2c->dev;
>> +	aw_dev->chip_id = AW87390_CHIP_ID;
>> +	aw_dev->acf = NULL;
>> +	aw_dev->prof_info.prof_desc = NULL;
>> +	aw_dev->prof_info.count = 0;
>> +	aw_dev->prof_info.prof_type = AW88395_DEV_NONE_TYPE_ID;
>> +	aw_dev->channel = AW87390_DEV_DEFAULT_CH;
>> +	aw_dev->fw_status = AW87390_DEV_FW_FAILED;
>> +	aw_dev->prof_index = AW87390_INIT_PROFILE;
>> +	aw_dev->status = AW87390_DEV_PW_OFF;
>> +
>> +	aw87390_parse_channel_dt(*aw87390);
>> +
>> +	return ret;
>> +}
>> +
>> +static int aw87390_i2c_probe(struct i2c_client *i2c)
>> +{
>> +	struct aw87390 *aw87390;
>> +	int ret;
>> +
>> +	ret = i2c_check_functionality(i2c->adapter, I2C_FUNC_I2C);
>> +	if (!ret)
>> +		return dev_err_probe(&i2c->dev, -ENXIO, "check_functionality failed\n");
>> +
>> +	aw87390 = devm_kzalloc(&i2c->dev, sizeof(*aw87390), GFP_KERNEL);
>> +	if (!aw87390)
>> +		return -ENOMEM;
>> +
>> +	mutex_init(&aw87390->lock);
>> +
>> +	i2c_set_clientdata(i2c, aw87390);
>> +
>> +	aw87390->regmap = devm_regmap_init_i2c(i2c, &aw87390_remap_config);
>> +	if (IS_ERR(aw87390->regmap)) {
>> +		ret = PTR_ERR(aw87390->regmap);

> ret is not needed here, so just:
> return dev_err_probe()

Thank you very much. I will modify it to 
"return dev_err_probe(&i2c->dev, PTR_ERR(aw87390->regmap), "failed to init regmap: %d\n", ret);"

>> +		return dev_err_probe(&i2c->dev, ret, "failed to init regmap: %d\n", ret);
>> +	}
>> +
>> +	/* aw pa init */
>> +	ret = aw87390_init(&aw87390, i2c, aw87390->regmap);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = regmap_write(aw87390->regmap, AW87390_ID_REG, AW87390_SOFT_RESET_VALUE);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = devm_snd_soc_register_component(&i2c->dev,
>> +				&soc_codec_dev_aw87390, NULL, 0);
>> +	if (ret)
>> +		dev_err(&i2c->dev, "failed to register aw87390: %d\n", ret);
>> +
>> +	return ret;
>> +}


Best regards,
Weidong Wang

Re: [PATCH V1 3/3] ASoC: codecs: Add aw87390 amplifier driver

[Linus Walleij]

Hi Weidong,

thanks for your patch!

On Mon, Sep 4, 2023 at 1:47 PM <wangweidong.a@awinic.com> wrote:

> From: Weidong Wang <wangweidong.a@awinic.com>
>
> Add i2c and amplifier registration for aw87390 and
> their associated operation functions.
>
> Signed-off-by: Weidong Wang <wangweidong.a@awinic.com>
(...)
> +#include <linux/of_gpio.h>

Please do not include this deprecated header.

Also: it seems you do not even use it so it's unnecessary.

Yours,
Linus Walleij