[PATCH v1 0/5] ASoC: wm8904: Add DMIC and DRC support

[PATCH v1 0/5] ASoC: wm8904: Add DMIC and DRC support

[Francesco Dolcini]

From: Francesco Dolcini <francesco.dolcini@toradex.com>

This patch series adds DMIC and DRC support to the WM8904 driver, a new
of_ helper is added to simplify the driver code.

DRC functionality is added in the same patch series to provide the
necessary dynamic range control to make DMIC support useful.

The WM8904 supports digital microphones on two of its inputs:
IN1L/DMICDAT1 and IN1R/DMICDAT2. These two inputs can either be
connected to an ADC or to the DMIC system. There is an ADC for each
line, and only one DMIC block. This DMIC block is either connected to
DMICDAT1 or to DMICDAT2. One DMIC data line supports two digital
microphones via time multiplexing.

The pin's functionality is decided during hardware design (IN1L vs
DMICDAT1 and IN1R vs DMICDAT2). This is reflected in the Device Tree.

If one line is analog and one is DMIC, we need to be able to switch
between ADC and DMIC at runtime. The DMIC source is known from the
Device Tree. If both are DMIC inputs, we need to be able to switch the
DMIC source. There is no need to switch between ADC and DMIC at runtime.

Therefore, kcontrols are dynamically added by the driver depending on
its Device Tree configuration.

This is a heavy rework of a previous patch series provided by Alifer
Moraes and Pierluigi Passaro,
https://lore.kernel.org/lkml/20220307141041.27538-1-alifer.m@variscite.com.

Ernest Van Hoecke (5):
  of: Add of_property_read_u16_index
  ASoC: wm8904: Don't touch GPIO configs set to 0xFFFF
  ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC and EQ support
  ASoC: wm8904: get platform data from DT
  ASoC: wm8904: add DMIC support

 .../devicetree/bindings/sound/wlf,wm8904.yaml |  88 ++++++
 drivers/of/property.c                         |  33 +++
 include/linux/of.h                            |   9 +
 include/sound/wm8904.h                        |   3 +
 sound/soc/codecs/wm8904.c                     | 274 +++++++++++++++++-
 5 files changed, 403 insertions(+), 4 deletions(-)

-- 
2.39.5

[PATCH v1 1/5] of: Add of_property_read_u16_index

[Francesco Dolcini]

From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

There is an of_property_read_u32_index and of_property_read_u64_index.
This patch adds a similar helper for u16.

Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 drivers/of/property.c | 33 +++++++++++++++++++++++++++++++++
 include/linux/of.h    |  9 +++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/of/property.c b/drivers/of/property.c
index 208d922cc24c..c1feb631e383 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -147,6 +147,39 @@ static void *of_find_property_value_of_size(const struct device_node *np,
 	return prop->value;
 }
 
+/**
+ * of_property_read_u16_index - Find and read a u16 from a multi-value property.
+ *
+ * @np:		device node from which the property value is to be read.
+ * @propname:	name of the property to be searched.
+ * @index:	index of the u16 in the list of values
+ * @out_value:	pointer to return value, modified only if no error.
+ *
+ * Search for a property in a device node and read nth 16-bit value from
+ * it.
+ *
+ * Return: 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_value is modified only if a valid u16 value can be decoded.
+ */
+int of_property_read_u16_index(const struct device_node *np,
+				       const char *propname,
+				       u32 index, u16 *out_value)
+{
+	const u16 *val = of_find_property_value_of_size(np, propname,
+					((index + 1) * sizeof(*out_value)),
+					0, NULL);
+
+	if (IS_ERR(val))
+		return PTR_ERR(val);
+
+	*out_value = be16_to_cpup(((__be16 *)val) + index);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_property_read_u16_index);
+
 /**
  * of_property_read_u32_index - Find and read a u32 from a multi-value property.
  *
diff --git a/include/linux/of.h b/include/linux/of.h
index eaf0e2a2b75c..5e52d90f6408 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -314,6 +314,9 @@ extern struct property *of_find_property(const struct device_node *np,
 extern bool of_property_read_bool(const struct device_node *np, const char *propname);
 extern int of_property_count_elems_of_size(const struct device_node *np,
 				const char *propname, int elem_size);
+extern int of_property_read_u16_index(const struct device_node *np,
+				       const char *propname,
+				       u32 index, u16 *out_value);
 extern int of_property_read_u32_index(const struct device_node *np,
 				       const char *propname,
 				       u32 index, u32 *out_value);
@@ -627,6 +630,12 @@ static inline int of_property_count_elems_of_size(const struct device_node *np,
 	return -ENOSYS;
 }
 
+static inline int of_property_read_u16_index(const struct device_node *np,
+			const char *propname, u32 index, u16 *out_value)
+{
+	return -ENOSYS;
+}
+
 static inline int of_property_read_u32_index(const struct device_node *np,
 			const char *propname, u32 index, u32 *out_value)
 {
-- 
2.39.5

Re: [PATCH v1 1/5] of: Add of_property_read_u16_index

[Rob Herring (Arm)]

On Thu, 06 Feb 2025 17:31:48 +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> 
> There is an of_property_read_u32_index and of_property_read_u64_index.
> This patch adds a similar helper for u16.
> 
> Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  drivers/of/property.c | 33 +++++++++++++++++++++++++++++++++
>  include/linux/of.h    |  9 +++++++++
>  2 files changed, 42 insertions(+)
> 

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

Re: [PATCH v1 1/5] of: Add of_property_read_u16_index

[Charles Keepax]

On Thu, Feb 06, 2025 at 05:31:48PM +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> 
> There is an of_property_read_u32_index and of_property_read_u64_index.
> This patch adds a similar helper for u16.
> 
> Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH v1 2/5] ASoC: wm8904: Don't touch GPIO configs set to 0xFFFF

[Francesco Dolcini]

From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

When updating the GPIO registers, do nothing for all fields of gpio_cfg
that are "0xFFFF".

This "do nothing" flag used to be 0 to easily check whether the gpio_cfg
field was actually set inside pdata or left empty (default).

However, 0 is a valid configuration for these registers, while 0xFFFF is
not.

With this change, users can explicitly set them to 0. Not setting
gpio_cfg in the platform data will now lead to setting all GPIO
registers to 0 instead of leaving them unset.

No one is using this platform data with this codec.

The change gets the driver ready to properly set gpio_cfg from the DT.

Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 sound/soc/codecs/wm8904.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index aef82532f8cf..2082ff12d336 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -2270,7 +2270,8 @@ static int wm8904_i2c_probe(struct i2c_client *i2c)
 	/* Apply configuration from the platform data. */
 	if (wm8904->pdata) {
 		for (i = 0; i < WM8904_GPIO_REGS; i++) {
-			if (!wm8904->pdata->gpio_cfg[i])
+			/* 0xFFFF in this config means "don't touch" */
+			if (wm8904->pdata->gpio_cfg[i] == 0xffff)
 				continue;
 
 			regmap_update_bits(wm8904->regmap,
-- 
2.39.5

Re: [PATCH v1 2/5] ASoC: wm8904: Don't touch GPIO configs set to 0xFFFF

[Charles Keepax]

On Thu, Feb 06, 2025 at 05:31:49PM +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> 
> When updating the GPIO registers, do nothing for all fields of gpio_cfg
> that are "0xFFFF".
> 
> This "do nothing" flag used to be 0 to easily check whether the gpio_cfg
> field was actually set inside pdata or left empty (default).
> 
> However, 0 is a valid configuration for these registers, while 0xFFFF is
> not.
> 
> With this change, users can explicitly set them to 0. Not setting
> gpio_cfg in the platform data will now lead to setting all GPIO
> registers to 0 instead of leaving them unset.
> 
> No one is using this platform data with this codec.
> 
> The change gets the driver ready to properly set gpio_cfg from the DT.
> 
> Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
> Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH v1 3/5] ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC and EQ support

[Francesco Dolcini]

From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

Add two properties to select the IN1L/DMICDAT1 and IN2R/DMICDAT2
functionality:
- wlf,in1l-as-dmicdat1
- wlf,in1r-as-dmicdat2

Add a property to describe the GPIO configuration registers, that can be
used to set the four multifunction pins:
- wlf,gpio-cfg

Add a property to describe the mic bias control registers:
- wlf,mic-cfg

Add two properties to describe the Dynamic Range Controller (DRC),
allowing multiple named configurations where each config sets the 4 DRC
registers (R40-R43):
- wlf,drc-cfg-regs
- wlf,drc-cfg-names

Add three properties to describe the equalizer (ReTune Mobile), allowing
multiple named configurations (associated with a samplerate) that set
the 24 (R134-R157) EQ registers:
- wlf,retune-mobile-cfg-regs
- wlf,retune-mobile-cfg-names
- wlf,retune-mobile-cfg-rates

Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 .../devicetree/bindings/sound/wlf,wm8904.yaml | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml b/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml
index 329260cf0fa0..b89d9db02848 100644
--- a/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml
+++ b/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml
@@ -38,6 +38,74 @@ properties:
   DCVDD-supply: true
   MICVDD-supply: true
 
+  wlf,in1l-as-dmicdat1:
+    type: boolean
+    description:
+      Use IN1L/DMICDAT1 as DMICDAT1, enabling the DMIC input path.
+
+  wlf,in1r-as-dmicdat2:
+    type: boolean
+    description:
+      Use IN1R/DMICDAT2 as DMICDAT2, enabling the DMIC input path.
+
+  wlf,gpio-cfg:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 4
+    maxItems: 4
+    description:
+      Default register values for R121/122/123/124 (GPIO Control).
+      If any entry has the value 0xFFFF, the related register won't be set.
+    default: [0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF]
+
+  wlf,mic-cfg:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 2
+    maxItems: 2
+    description:
+      Default register values for R6/R7 (Mic Bias Control).
+    default: [0, 0]
+
+  wlf,drc-cfg-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description:
+      List of strings for the available DRC modes.
+      If absent, DRC is disabled.
+
+  wlf,drc-cfg-regs:
+    $ref: /schemas/types.yaml#/definitions/uint16-array
+    description:
+      Default register values for R40/41/42/43 (DRC).
+      The list must be 4 times the length of wlf,drc-cfg-names.
+      If absent, DRC is disabled.
+
+  wlf,retune-mobile-cfg-names:
+    $ref: /schemas/types.yaml#/definitions/string-array
+    description:
+      List of strings for the available retune modes.
+      If absent, retune is disabled.
+
+  wlf,retune-mobile-cfg-rates:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description:
+      List of rates for the available retune modes.
+      The list must be the same length as wlf,retune-mobile-cfg-names.
+      If absent, retune is disabled.
+
+  wlf,retune-mobile-cfg-regs:
+    $ref: /schemas/types.yaml#/definitions/uint16-array
+    description:
+      Default register values for R134/.../157 (EQ).
+      The list must be 24 times the length of wlf,retune-mobile-cfg-names.
+      If absent, retune is disabled.
+
+dependencies:
+  wlf,drc-cfg-names: [ 'wlf,drc-cfg-regs' ]
+  wlf,drc-cfg-regs: [ 'wlf,drc-cfg-names' ]
+
+  wlf,retune-mobile-cfg-names: [ 'wlf,retune-mobile-cfg-rates', 'wlf,retune-mobile-cfg-regs' ]
+  wlf,retune-mobile-cfg-regs: [ 'wlf,retune-mobile-cfg-names', 'wlf,retune-mobile-cfg-rates' ]
+  wlf,retune-mobile-cfg-rates: [ 'wlf,retune-mobile-cfg-names', 'wlf,retune-mobile-cfg-regs' ]
+
 required:
   - compatible
   - reg
@@ -70,5 +138,25 @@ examples:
             DBVDD-supply = <&reg_1p8v>;
             DCVDD-supply = <&reg_1p8v>;
             MICVDD-supply = <&reg_1p8v>;
+
+            wlf,drc-cfg-names = "default", "peaklimiter", "tradition", "soft", "music";
+            wlf,drc-cfg-regs =
+                /* coded default: KNEE_IP = KNEE_OP = 0, HI_COMP = LO_COMP = 1  */
+                /bits/ 16 <0x01af 0x3248 0x0000 0x0000>,
+                /* coded default: KNEE_IP = -24, KNEE_OP = -6, HI_COMP = 1/4, LO_COMP = 1 */
+                /bits/ 16 <0x04af 0x324b 0x0010 0x0408>,
+                /* coded default: KNEE_IP = -42, KNEE_OP = -3, HI_COMP = 0, LO_COMP = 1 */
+                /bits/ 16 <0x04af 0x324b 0x0028 0x0704>,
+                /* coded default: KNEE_IP = -45, KNEE_OP = -9, HI_COMP = 1/8, LO_COMP = 1 */
+                /bits/ 16 <0x04af 0x324b 0x0018 0x078c>,
+                /* coded default: KNEE_IP = -30, KNEE_OP = -10.5, HI_COMP = 1/4, LO_COMP = 1 */
+                /bits/ 16 <0x04af 0x324b 0x0010 0x050e>;
+
+            wlf,gpio-cfg = <
+                0x0018 /* GPIO1 => DMIC_CLK */
+                0xffff /* GPIO2 => don't touch */
+                0xffff /* GPIO3 => don't touch */
+                0xffff /* GPIO4 => don't touch */
+            >;
         };
     };
-- 
2.39.5

Re: [PATCH v1 3/5] ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC and EQ support

[Rob Herring]

On Thu, Feb 06, 2025 at 05:31:50PM +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> 
> Add two properties to select the IN1L/DMICDAT1 and IN2R/DMICDAT2
> functionality:
> - wlf,in1l-as-dmicdat1
> - wlf,in1r-as-dmicdat2
> 
> Add a property to describe the GPIO configuration registers, that can be
> used to set the four multifunction pins:
> - wlf,gpio-cfg
> 
> Add a property to describe the mic bias control registers:
> - wlf,mic-cfg
> 
> Add two properties to describe the Dynamic Range Controller (DRC),
> allowing multiple named configurations where each config sets the 4 DRC
> registers (R40-R43):
> - wlf,drc-cfg-regs
> - wlf,drc-cfg-names
> 
> Add three properties to describe the equalizer (ReTune Mobile), allowing
> multiple named configurations (associated with a samplerate) that set
> the 24 (R134-R157) EQ registers:
> - wlf,retune-mobile-cfg-regs
> - wlf,retune-mobile-cfg-names
> - wlf,retune-mobile-cfg-rates
> 
> Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
> Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  .../devicetree/bindings/sound/wlf,wm8904.yaml | 88 +++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml b/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml
> index 329260cf0fa0..b89d9db02848 100644
> --- a/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml
> +++ b/Documentation/devicetree/bindings/sound/wlf,wm8904.yaml
> @@ -38,6 +38,74 @@ properties:
>    DCVDD-supply: true
>    MICVDD-supply: true
>  
> +  wlf,in1l-as-dmicdat1:
> +    type: boolean
> +    description:
> +      Use IN1L/DMICDAT1 as DMICDAT1, enabling the DMIC input path.
> +
> +  wlf,in1r-as-dmicdat2:
> +    type: boolean
> +    description:
> +      Use IN1R/DMICDAT2 as DMICDAT2, enabling the DMIC input path.
> +
> +  wlf,gpio-cfg:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 4
> +    maxItems: 4
> +    description:
> +      Default register values for R121/122/123/124 (GPIO Control).
> +      If any entry has the value 0xFFFF, the related register won't be set.
> +    default: [0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF]
> +
> +  wlf,mic-cfg:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 2
> +    maxItems: 2
> +    description:
> +      Default register values for R6/R7 (Mic Bias Control).
> +    default: [0, 0]
> +
> +  wlf,drc-cfg-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description:
> +      List of strings for the available DRC modes.
> +      If absent, DRC is disabled.
> +
> +  wlf,drc-cfg-regs:
> +    $ref: /schemas/types.yaml#/definitions/uint16-array
> +    description:
> +      Default register values for R40/41/42/43 (DRC).
> +      The list must be 4 times the length of wlf,drc-cfg-names.
> +      If absent, DRC is disabled.
> +
> +  wlf,retune-mobile-cfg-names:
> +    $ref: /schemas/types.yaml#/definitions/string-array
> +    description:
> +      List of strings for the available retune modes.
> +      If absent, retune is disabled.

Is there no defined set of names?

> +
> +  wlf,retune-mobile-cfg-rates:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      List of rates for the available retune modes.
> +      The list must be the same length as wlf,retune-mobile-cfg-names.
> +      If absent, retune is disabled.
> +
> +  wlf,retune-mobile-cfg-regs:
> +    $ref: /schemas/types.yaml#/definitions/uint16-array
> +    description:
> +      Default register values for R134/.../157 (EQ).
> +      The list must be 24 times the length of wlf,retune-mobile-cfg-names.
> +      If absent, retune is disabled.
> +
> +dependencies:
> +  wlf,drc-cfg-names: [ 'wlf,drc-cfg-regs' ]
> +  wlf,drc-cfg-regs: [ 'wlf,drc-cfg-names' ]
> +
> +  wlf,retune-mobile-cfg-names: [ 'wlf,retune-mobile-cfg-rates', 'wlf,retune-mobile-cfg-regs' ]
> +  wlf,retune-mobile-cfg-regs: [ 'wlf,retune-mobile-cfg-names', 'wlf,retune-mobile-cfg-rates' ]
> +  wlf,retune-mobile-cfg-rates: [ 'wlf,retune-mobile-cfg-names', 'wlf,retune-mobile-cfg-regs' ]

I'm not really a fan of these long lists of properties, but codecs and 
touchscreens seem to need a bunch of parameters.

> +
>  required:
>    - compatible
>    - reg
> @@ -70,5 +138,25 @@ examples:
>              DBVDD-supply = <&reg_1p8v>;
>              DCVDD-supply = <&reg_1p8v>;
>              MICVDD-supply = <&reg_1p8v>;
> +
> +            wlf,drc-cfg-names = "default", "peaklimiter", "tradition", "soft", "music";
> +            wlf,drc-cfg-regs =
> +                /* coded default: KNEE_IP = KNEE_OP = 0, HI_COMP = LO_COMP = 1  */
> +                /bits/ 16 <0x01af 0x3248 0x0000 0x0000>,
> +                /* coded default: KNEE_IP = -24, KNEE_OP = -6, HI_COMP = 1/4, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0010 0x0408>,
> +                /* coded default: KNEE_IP = -42, KNEE_OP = -3, HI_COMP = 0, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0028 0x0704>,
> +                /* coded default: KNEE_IP = -45, KNEE_OP = -9, HI_COMP = 1/8, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0018 0x078c>,
> +                /* coded default: KNEE_IP = -30, KNEE_OP = -10.5, HI_COMP = 1/4, LO_COMP = 1 */
> +                /bits/ 16 <0x04af 0x324b 0x0010 0x050e>;
> +
> +            wlf,gpio-cfg = <
> +                0x0018 /* GPIO1 => DMIC_CLK */
> +                0xffff /* GPIO2 => don't touch */
> +                0xffff /* GPIO3 => don't touch */
> +                0xffff /* GPIO4 => don't touch */
> +            >;

Please make your example full and add all the optional properties.

>          };
>      };
> -- 
> 2.39.5
> 

Re: [PATCH v1 3/5] ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC and EQ support

[Mark Brown]

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

On Tue, Feb 11, 2025 at 04:19:04PM -0600, Rob Herring wrote:
> On Thu, Feb 06, 2025 at 05:31:50PM +0100, Francesco Dolcini wrote:

> > +  wlf,retune-mobile-cfg-names:
> > +    $ref: /schemas/types.yaml#/definitions/string-array
> > +    description:
> > +      List of strings for the available retune modes.
> > +      If absent, retune is disabled.

> Is there no defined set of names?

No, this set of names is for system integrator specified tunings so they
can make up whatever is appropriate for their system and use cases (or
otherwise amuses them).  The names will end up getting exposed directly
to userspace as options that can be selected at runtime.

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

Re: [PATCH v1 3/5] ASoC: dt-bindings: wm8904: Add DMIC, GPIO, MIC and EQ support

[Charles Keepax]

On Thu, Feb 06, 2025 at 05:31:50PM +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> 
> Add two properties to select the IN1L/DMICDAT1 and IN2R/DMICDAT2
> functionality:
> - wlf,in1l-as-dmicdat1
> - wlf,in1r-as-dmicdat2
> 
> Add a property to describe the GPIO configuration registers, that can be
> used to set the four multifunction pins:
> - wlf,gpio-cfg
> 
> Add a property to describe the mic bias control registers:
> - wlf,mic-cfg
> 
> Add two properties to describe the Dynamic Range Controller (DRC),
> allowing multiple named configurations where each config sets the 4 DRC
> registers (R40-R43):
> - wlf,drc-cfg-regs
> - wlf,drc-cfg-names
> 
> Add three properties to describe the equalizer (ReTune Mobile), allowing
> multiple named configurations (associated with a samplerate) that set
> the 24 (R134-R157) EQ registers:
> - wlf,retune-mobile-cfg-regs
> - wlf,retune-mobile-cfg-names
> - wlf,retune-mobile-cfg-rates
> 
> Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
> Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH v1 4/5] ASoC: wm8904: get platform data from DT

[Francesco Dolcini]

From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

Read in optional codec-specific properties from the device tree.

The platform_data structure is not populated when using device trees.
This change parses optional dts properties to populate it.

- wlf,in1l-as-dmicdat1
- wlf,in1r-as-dmicdat2
- wlf,gpio-cfg
- wlf,mic-cfg
- wlf,drc-cfg-regs
- wlf,drc-cfg-names
- wlf,retune-mobile-cfg-regs
- wlf,retune-mobile-cfg-names
- wlf,retune-mobile-cfg-rates

Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 include/sound/wm8904.h    |   3 +
 sound/soc/codecs/wm8904.c | 189 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 191 insertions(+), 1 deletion(-)

diff --git a/include/sound/wm8904.h b/include/sound/wm8904.h
index 88ac1870510e..8b2c16b524f7 100644
--- a/include/sound/wm8904.h
+++ b/include/sound/wm8904.h
@@ -151,6 +151,9 @@ struct wm8904_pdata {
 	int num_retune_mobile_cfgs;
 	struct wm8904_retune_mobile_cfg *retune_mobile_cfgs;
 
+	bool in1l_as_dmicdat1;
+	bool in1r_as_dmicdat2;
+
 	u32 gpio_cfg[WM8904_GPIO_REGS];
 	u32 mic_cfg[WM8904_MIC_REGS];
 };
diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 2082ff12d336..899ef6679f7e 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -2168,6 +2168,184 @@ static const struct of_device_id wm8904_of_match[] = {
 MODULE_DEVICE_TABLE(of, wm8904_of_match);
 #endif
 
+/**
+ * wm8904_read_cfg_reg_arr() - Reads a subarray from a DT u16 array
+ *
+ * @np: pointer to the device_node struct
+ * @regs_property: DT property of interest
+ * @size: size of subarrays within the array
+ * @idx: index of the subarray of interest
+ * @out: output
+ *
+ * Helper to read a subarray from a DT uint16-array,
+ *  divided into equally sized arrays of size `size`
+ *
+ * Subset starts at `idx * size` and is of size `size`
+ *
+ * Return: 0 on success, negative error code otherwise
+ */
+static int wm8904_read_cfg_reg_arr(const struct device_node *np,
+				   const char * const regs_property,
+				   int size, int idx,
+				   u16 * const out)
+{
+	int i, offset, ret;
+
+	offset = idx * size;
+
+	for (i = 0; i < size; i++) {
+		ret = of_property_read_u16_index(np, regs_property, i + offset, &out[i]);
+		if (ret)
+			return ret;
+	}
+	return 0;
+}
+
+static int wm8904_parse_retune_cfg_regs(const struct device_node *np,
+					struct wm8904_pdata *pdata, int cfg_idx)
+{
+	return wm8904_read_cfg_reg_arr(np, "wlf,retune-mobile-cfg-regs",
+				       WM8904_EQ_REGS, cfg_idx,
+				       &pdata->retune_mobile_cfgs[cfg_idx].regs[0]);
+}
+
+static int wm8904_parse_drc_cfg_regs(const struct device_node *np,
+				     struct wm8904_pdata *pdata, int cfg_idx)
+{
+	return wm8904_read_cfg_reg_arr(np, "wlf,drc-cfg-regs",
+				       WM8904_DRC_REGS, cfg_idx,
+				       &pdata->drc_cfgs[cfg_idx].regs[0]);
+}
+
+static int wm8904_parse_drc_cfg_from_of(struct i2c_client *i2c,
+					struct wm8904_pdata *pdata)
+{
+	const struct device_node *np = i2c->dev.of_node;
+	int i, n_cfgs;
+
+	n_cfgs = of_property_count_strings(np, "wlf,drc-cfg-names");
+	if (n_cfgs == -EINVAL)
+		return 0;
+
+	if (n_cfgs <= 0) {
+		dev_err(&i2c->dev, "Could not get wlf,drc-cfg-names length: %d",
+			n_cfgs);
+		return n_cfgs;
+	}
+
+	pdata->drc_cfgs = devm_kzalloc(&i2c->dev,
+				       n_cfgs * sizeof(struct wm8904_drc_cfg),
+				       GFP_KERNEL);
+	if (!pdata->drc_cfgs)
+		return -ENOMEM;
+
+	for (i = 0; i < n_cfgs; i++) {
+		if (wm8904_parse_drc_cfg_regs(np, pdata, i)) {
+			dev_err(&i2c->dev,
+				"Invalid 'wlf,drc-cfg-regs[%i,:]'\n", i);
+			return -EINVAL;
+		}
+
+		if (of_property_read_string_index(np, "wlf,drc-cfg-names", i,
+						  &pdata->drc_cfgs[i].name)) {
+			dev_err(&i2c->dev,
+				"Invalid 'wlf,drc-cfg-names[%i]'\n", i);
+			return -EINVAL;
+		}
+	}
+
+	pdata->num_drc_cfgs = n_cfgs;
+	return 0;
+}
+
+static int wm8904_parse_retune_cfg_from_of(struct i2c_client *i2c,
+					   struct wm8904_pdata *pdata)
+{
+	const struct device_node *np = i2c->dev.of_node;
+	int i, n_cfgs;
+
+	n_cfgs = of_property_count_strings(np, "wlf,retune-mobile-cfg-names");
+	if (n_cfgs == -EINVAL)
+		return 0;
+
+	if (n_cfgs <= 0) {
+		dev_err(&i2c->dev,
+			"Could not get wlf,retune-mobile-cfg-names length: %d",
+			n_cfgs);
+		return n_cfgs;
+	}
+
+	pdata->retune_mobile_cfgs = devm_kzalloc(&i2c->dev,
+						 n_cfgs * sizeof(struct wm8904_retune_mobile_cfg),
+						 GFP_KERNEL);
+	if (!pdata->retune_mobile_cfgs)
+		return -ENOMEM;
+
+	for (i = 0; i < n_cfgs; i++) {
+		if (wm8904_parse_retune_cfg_regs(np, pdata, i)) {
+			dev_err(&i2c->dev,
+				"Invalid 'wlf,retune-mobile-cfg-regs[%i,:]'\n", i);
+			return -EINVAL;
+		}
+
+		if (of_property_read_u32_index(np, "wlf,retune-mobile-cfg-rates", i,
+					       &pdata->retune_mobile_cfgs[i].rate)) {
+			dev_err(&i2c->dev,
+				"Invalid 'wlf,retune-mobile-cfg-rates[%i]'\n", i);
+			return -EINVAL;
+		}
+
+		if (of_property_read_string_index(np, "wlf,retune-mobile-cfg-names", i,
+						  &pdata->drc_cfgs[i].name)) {
+			dev_err(&i2c->dev,
+				"Invalid 'wlf,retune-mobile-cfg-names[%i]'\n", i);
+			return -EINVAL;
+		}
+	}
+
+	pdata->num_retune_mobile_cfgs = n_cfgs;
+	return 0;
+}
+
+static int wm8904_set_pdata_from_of(struct i2c_client *i2c,
+				    struct wm8904_priv *wm8904)
+{
+	const struct device_node *np = i2c->dev.of_node;
+	struct wm8904_pdata *pdata;
+	int ret, i;
+
+	pdata = devm_kzalloc(&i2c->dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->in1l_as_dmicdat1 =
+		of_property_read_bool(np, "wlf,in1l-as-dmicdat1");
+
+	pdata->in1r_as_dmicdat2 =
+		of_property_read_bool(np, "wlf,in1r-as-dmicdat2");
+
+	/* If absent, default to 0xFFFF for GPIO config (i.e.: don't set) */
+	for (i = 0; i < WM8904_GPIO_REGS; i++)
+		pdata->gpio_cfg[i] = 0xFFFF;
+
+	of_property_read_u32_array(np, "wlf,gpio-cfg", pdata->gpio_cfg,
+				   ARRAY_SIZE(pdata->gpio_cfg));
+
+	of_property_read_u32_array(np, "wlf,mic-cfg", pdata->mic_cfg,
+				   ARRAY_SIZE(pdata->mic_cfg));
+
+	ret = wm8904_parse_drc_cfg_from_of(i2c, pdata);
+	if (ret)
+		return ret;
+
+	ret = wm8904_parse_retune_cfg_from_of(i2c, pdata);
+	if (ret)
+		return ret;
+
+	wm8904->pdata = pdata;
+	return 0;
+}
+
 static const struct i2c_device_id wm8904_i2c_id[];
 
 static int wm8904_i2c_probe(struct i2c_client *i2c)
@@ -2199,7 +2377,16 @@ static int wm8904_i2c_probe(struct i2c_client *i2c)
 	wm8904->devtype = (uintptr_t)i2c_get_match_data(i2c);
 
 	i2c_set_clientdata(i2c, wm8904);
-	wm8904->pdata = i2c->dev.platform_data;
+
+	if (i2c->dev.of_node) {
+		ret = wm8904_set_pdata_from_of(i2c, wm8904);
+		if (ret) {
+			dev_err(&i2c->dev, "Failed to set platform data from of: %d\n", ret);
+			return ret;
+		}
+	} else {
+		wm8904->pdata = i2c->dev.platform_data;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(wm8904->supplies); i++)
 		wm8904->supplies[i].supply = wm8904_supply_names[i];
-- 
2.39.5

Re: [PATCH v1 4/5] ASoC: wm8904: get platform data from DT

[Charles Keepax]

On Thu, Feb 06, 2025 at 05:31:51PM +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> 
> Read in optional codec-specific properties from the device tree.
> 
> The platform_data structure is not populated when using device trees.
> This change parses optional dts properties to populate it.
> 
> - wlf,in1l-as-dmicdat1
> - wlf,in1r-as-dmicdat2
> - wlf,gpio-cfg
> - wlf,mic-cfg
> - wlf,drc-cfg-regs
> - wlf,drc-cfg-names
> - wlf,retune-mobile-cfg-regs
> - wlf,retune-mobile-cfg-names
> - wlf,retune-mobile-cfg-rates
> 
> Datasheet: https://statics.cirrus.com/pubs/proDatasheet/WM8904_Rev4.1.pdf
> Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---

Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>

Thanks,
Charles

[PATCH v1 5/5] ASoC: wm8904: add DMIC support

[Francesco Dolcini]

From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>

The WM8904 codec supports both ADC and DMIC inputs.

Get input pin functionality from the platform data and add the necessary
controls depending on the possible additional routing.

The ADC and DMIC share the IN1L/DMICDAT1 and IN1R/DMICDAT2 pins.

When both are connected to an analog input, only the ADC is used. When
both are connected to a DMIC, only the DMIC is used, and a mux is added
to select the DMIC source. When one line is a DMIC and the other an
analog input, the DMIC source is set from the platform data and a mux is
added to select whether to use the ADC or DMIC.

Signed-off-by: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 sound/soc/codecs/wm8904.c | 82 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm8904.c b/sound/soc/codecs/wm8904.c
index 899ef6679f7e..a024cfc136c3 100644
--- a/sound/soc/codecs/wm8904.c
+++ b/sound/soc/codecs/wm8904.c
@@ -844,6 +844,26 @@ static int out_pga_event(struct snd_soc_dapm_widget *w,
 	return 0;
 }
 
+static const char * const dmic_text[] = {
+	"DMIC1", "DMIC2"
+};
+
+static SOC_ENUM_SINGLE_DECL(dmic_enum, WM8904_DIGITAL_MICROPHONE_0,
+			    WM8904_DMIC_SRC_SHIFT, dmic_text);
+
+static const struct snd_kcontrol_new dmic_mux =
+	SOC_DAPM_ENUM("DMIC Mux", dmic_enum);
+
+static const char * const cin_text[] = {
+	"ADC", "DMIC"
+};
+
+static SOC_ENUM_SINGLE_DECL(cin_enum, WM8904_DIGITAL_MICROPHONE_0,
+			    WM8904_DMIC_ENA_SHIFT, cin_text);
+
+static const struct snd_kcontrol_new cin_mux =
+	SOC_DAPM_ENUM("Capture Input", cin_enum);
+
 static const char *input_mode_text[] = {
 	"Single-Ended", "Differential Line", "Differential Mic"
 };
@@ -963,6 +983,15 @@ SND_SOC_DAPM_AIF_OUT("AIFOUTL", "Capture", 0, SND_SOC_NOPM, 0, 0),
 SND_SOC_DAPM_AIF_OUT("AIFOUTR", "Capture", 1, SND_SOC_NOPM, 0, 0),
 };
 
+static const struct snd_soc_dapm_widget wm8904_dmic_dapm_widgets[] = {
+SND_SOC_DAPM_MUX("DMIC Mux", SND_SOC_NOPM, 0, 0, &dmic_mux),
+};
+
+static const struct snd_soc_dapm_widget wm8904_cin_dapm_widgets[] = {
+SND_SOC_DAPM_MUX("Left Capture Input", SND_SOC_NOPM, 0, 0, &cin_mux),
+SND_SOC_DAPM_MUX("Right Capture Input", SND_SOC_NOPM, 0, 0, &cin_mux),
+};
+
 static const struct snd_soc_dapm_widget wm8904_dac_dapm_widgets[] = {
 SND_SOC_DAPM_AIF_IN("AIFINL", "Playback", 0, SND_SOC_NOPM, 0, 0),
 SND_SOC_DAPM_AIF_IN("AIFINR", "Playback", 1, SND_SOC_NOPM, 0, 0),
@@ -1107,6 +1136,24 @@ static const struct snd_soc_dapm_route adc_intercon[] = {
 	{ "ADCR", NULL, "Right Capture PGA" },
 };
 
+static const struct snd_soc_dapm_route dmic_intercon[] = {
+	{ "DMIC Mux", "DMIC1", "IN1L" },
+	{ "DMIC Mux", "DMIC2", "IN1R" },
+
+	{ "ADCL", NULL, "DMIC Mux" },
+	{ "ADCR", NULL, "DMIC Mux" },
+};
+
+static const struct snd_soc_dapm_route cin_intercon[] = {
+	{ "Left Capture Input", "ADC", "Left Capture PGA" },
+	{ "Left Capture Input", "DMIC", "IN1L" },
+	{ "Right Capture Input", "ADC", "Right Capture PGA" },
+	{ "Right Capture Input", "DMIC", "IN1R" },
+
+	{ "ADCL", NULL, "Left Capture Input" },
+	{ "ADCR", NULL, "Right Capture Input" },
+};
+
 static const struct snd_soc_dapm_route dac_intercon[] = {
 	{ "DACL Mux", "Left", "AIFINL" },
 	{ "DACL Mux", "Right", "AIFINR" },
@@ -2052,6 +2099,7 @@ static void wm8904_handle_retune_mobile_pdata(struct snd_soc_component *componen
 
 static void wm8904_handle_pdata(struct snd_soc_component *component)
 {
+	struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component);
 	struct wm8904_priv *wm8904 = snd_soc_component_get_drvdata(component);
 	struct wm8904_pdata *pdata = wm8904->pdata;
 	int ret, i;
@@ -2062,6 +2110,35 @@ static void wm8904_handle_pdata(struct snd_soc_component *component)
 		return;
 	}
 
+	if (pdata->in1l_as_dmicdat1 && pdata->in1r_as_dmicdat2) {
+		dev_dbg(component->dev, "Activating DMICDAT1 and DMICDAT2\n");
+		snd_soc_component_update_bits(component, WM8904_DIGITAL_MICROPHONE_0,
+					      WM8904_DMIC_ENA_MASK,
+					      1 << WM8904_DMIC_ENA_SHIFT);
+
+		/* Need a control and routing to switch between DMIC1 and 2 */
+		snd_soc_dapm_new_controls(dapm, wm8904_dmic_dapm_widgets,
+					  ARRAY_SIZE(wm8904_dmic_dapm_widgets));
+		snd_soc_dapm_add_routes(dapm, dmic_intercon,
+					ARRAY_SIZE(dmic_intercon));
+	} else if (pdata->in1l_as_dmicdat1 || pdata->in1r_as_dmicdat2) {
+		unsigned int dmic_src = pdata->in1l_as_dmicdat1 ? 0 : 1;
+
+		dev_dbg(component->dev, "DMIC_SRC (0 or 1): %d\n", dmic_src);
+		snd_soc_component_update_bits(component, WM8904_DIGITAL_MICROPHONE_0,
+					      WM8904_DMIC_SRC_MASK,
+					      dmic_src << WM8904_DMIC_SRC_SHIFT);
+
+		/* Need a control and routing to switch between DMIC and ADC */
+		snd_soc_dapm_new_controls(dapm, wm8904_cin_dapm_widgets,
+					  ARRAY_SIZE(wm8904_cin_dapm_widgets));
+		snd_soc_dapm_add_routes(dapm, cin_intercon,
+					ARRAY_SIZE(cin_intercon));
+	} else {
+		snd_soc_component_update_bits(component, WM8904_DIGITAL_MICROPHONE_0,
+					      WM8904_DMIC_ENA_MASK, 0);
+	}
+
 	dev_dbg(component->dev, "%d DRC configurations\n", pdata->num_drc_cfgs);
 
 	if (pdata->num_drc_cfgs) {
@@ -2117,10 +2194,11 @@ static int wm8904_probe(struct snd_soc_component *component)
 		return -EINVAL;
 	}
 
-	wm8904_handle_pdata(component);
-
 	wm8904_add_widgets(component);
 
+	/* This can add dependent widgets, so it is done after add_widgets */
+	wm8904_handle_pdata(component);
+
 	return 0;
 }
 
-- 
2.39.5

Re: [PATCH v1 5/5] ASoC: wm8904: add DMIC support

[Charles Keepax]

On Thu, Feb 06, 2025 at 05:31:52PM +0100, Francesco Dolcini wrote:
> From: Ernest Van Hoecke <ernest.vanhoecke@toradex.com>
> +static const struct snd_soc_dapm_route dmic_intercon[] = {
> +	{ "DMIC Mux", "DMIC1", "IN1L" },
> +	{ "DMIC Mux", "DMIC2", "IN1R" },
> +
> +	{ "ADCL", NULL, "DMIC Mux" },
> +	{ "ADCR", NULL, "DMIC Mux" },
> +};
> +
> +static const struct snd_soc_dapm_route cin_intercon[] = {
> +	{ "Left Capture Input", "ADC", "Left Capture PGA" },
> +	{ "Left Capture Input", "DMIC", "IN1L" },
> +	{ "Right Capture Input", "ADC", "Right Capture PGA" },
> +	{ "Right Capture Input", "DMIC", "IN1R" },

Am I misunderstanding things or does something not quite look
right with the routes here? Shouldn't you end up with the these
three situations:

Analogue:
	{ "Left Capture Input", "ADC", "Left Capture PGA" },
	{ "Right Capture Input", "ADC", "Right Capture PGA" },

Digital in1l_as_dmicdat1:
	{ "Left Capture Input", "DMIC", "IN1L" },
	{ "Right Capture Input", "DMIC", "IN1L" },

Digital in1r_as_dmicdat2:
	{ "Left Capture Input", "DMIC", "IN1R" },
	{ "Right Capture Input", "DMIC", "IN1R" },

So I think you need to add the DMIC routes conditionally here.

Also is there not some part of the existing analogue routing that
needs disconnected in the DMIC cases? This only addes routes,
which feels like the existing analogue path is never
disconnected?

Thanks,
Charles

> +
> +	{ "ADCL", NULL, "Left Capture Input" },
> +	{ "ADCR", NULL, "Right Capture Input" },
> +};