[PATCH 0/2] ASoC: add devicetree support for WM8961 codec

[PATCH 0/2] ASoC: add devicetree support for WM8961 codec

[Doug Brown]

This series adds devicetree support for the Wolfson WM8961 codec. The
first patch adds a schema, and the second patch hooks it up in the code.

Doug Brown (2):
  ASoC: dt-bindings: add schema for WM8961
  ASoC: wm8961: add support for devicetree

 .../devicetree/bindings/sound/wlf,wm8961.yaml | 40 +++++++++++++++++++
 sound/soc/codecs/Kconfig                      |  2 +-
 sound/soc/codecs/wm8961.c                     |  6 +++
 3 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/sound/wlf,wm8961.yaml

-- 
2.34.1

[PATCH 1/2] ASoC: dt-bindings: add schema for WM8961

[Doug Brown]

Create a simple DT schema for the existing Wolfson WM8961 driver so that
DT support can be added to the driver.

Signed-off-by: Doug Brown <doug@schmorgal.com>
---
 .../devicetree/bindings/sound/wlf,wm8961.yaml | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/sound/wlf,wm8961.yaml

diff --git a/Documentation/devicetree/bindings/sound/wlf,wm8961.yaml b/Documentation/devicetree/bindings/sound/wlf,wm8961.yaml
new file mode 100644
index 000000000000..73166cf0fdcf
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/wlf,wm8961.yaml
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/wlf,wm8961.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Wolfson WM8961 Ultra-Low Power Stereo CODEC
+
+maintainers:
+  - patches@opensource.cirrus.com
+
+properties:
+  '#sound-dai-cells':
+    const: 0
+
+  compatible:
+    const: wlf,wm8961
+
+  reg:
+    maxItems: 1
+
+required:
+  - '#sound-dai-cells'
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          wm8961: codec@4a {
+                  #sound-dai-cells = <0>;
+                  compatible = "wlf,wm8961";
+                  reg = <0x4a>;
+          };
+    };
-- 
2.34.1

Re: [PATCH 1/2] ASoC: dt-bindings: add schema for WM8961

[Krzysztof Kozlowski]

On 01/10/2022 22:00, Doug Brown wrote:
> Create a simple DT schema for the existing Wolfson WM8961 driver so that
> DT support can be added to the driver.
> 
> Signed-off-by: Doug Brown <doug@schmorgal.com>
> ---
>  .../devicetree/bindings/sound/wlf,wm8961.yaml | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/wlf,wm8961.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/wlf,wm8961.yaml b/Documentation/devicetree/bindings/sound/wlf,wm8961.yaml
> new file mode 100644
> index 000000000000..73166cf0fdcf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/wlf,wm8961.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/wlf,wm8961.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wolfson WM8961 Ultra-Low Power Stereo CODEC
> +
> +maintainers:
> +  - patches@opensource.cirrus.com
> +
> +properties:
> +  '#sound-dai-cells':
> +    const: 0
> +
> +  compatible:
> +    const: wlf,wm8961

Please put compatible first in list of properties (and follow same order
in "required"). It's the most important piece, so we want it to be the
first to see. It also follows the convention of DTS, where compatible is
expected to be first.

> +
> +  reg:
> +    maxItems: 1
> +
> +required:
> +  - '#sound-dai-cells'
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          wm8961: codec@4a {
> +                  #sound-dai-cells = <0>;
> +                  compatible = "wlf,wm8961";

Here compatible first, reg second, then the rest.

> +                  reg = <0x4a>;
> +          };
> +    };

Best regards,
Krzysztof

[PATCH 2/2] ASoC: wm8961: add support for devicetree

[Doug Brown]

This adds support for devicetree to the WM8961 driver so it can be used
with modern DT-based kernels.

Signed-off-by: Doug Brown <doug@schmorgal.com>
---
 sound/soc/codecs/Kconfig  | 2 +-
 sound/soc/codecs/wm8961.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index e3b90c425faf..2b5787ee8d31 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -1929,7 +1929,7 @@ config SND_SOC_WM8960
 	depends on I2C
 
 config SND_SOC_WM8961
-	tristate
+	tristate "Wolfson Microelectronics WM8961 CODEC"
 	depends on I2C
 
 config SND_SOC_WM8962
diff --git a/sound/soc/codecs/wm8961.c b/sound/soc/codecs/wm8961.c
index 7dc6aaf65576..539096184eda 100644
--- a/sound/soc/codecs/wm8961.c
+++ b/sound/soc/codecs/wm8961.c
@@ -971,6 +971,12 @@ static const struct i2c_device_id wm8961_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, wm8961_i2c_id);
 
+static const struct of_device_id wm8961_of_match[] = {
+	{ .compatible = "wlf,wm8961", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, wm8961_of_match);
+
 static struct i2c_driver wm8961_i2c_driver = {
 	.driver = {
 		.name = "wm8961",
-- 
2.34.1

Re: [PATCH 2/2] ASoC: wm8961: add support for devicetree

[Doug Brown]

On 10/1/2022 3:23 PM, kernel test robot wrote:

>>> sound/soc/codecs/wm8961.c:974:34: warning: 'wm8961_of_match' defined but not used [-Wunused-const-variable=]
>       974 | static const struct of_device_id wm8961_of_match[] = {
>           |                                  ^~~~~~~~~~~~~~~

Oops, nice catch by the kernel test robot. I will submit a V2 patch that
does this part exactly how the wm8960 driver does it, including
.of_match_table in wm8961_i2c_driver. Waiting to see if I get any other
feedback on V1 first.

Thanks,
Doug

Re: [PATCH 2/2] ASoC: wm8961: add support for devicetree

[Krzysztof Kozlowski]

On 02/10/2022 07:04, Doug Brown wrote:
> On 10/1/2022 3:23 PM, kernel test robot wrote:
> 
>>>> sound/soc/codecs/wm8961.c:974:34: warning: 'wm8961_of_match' defined but not used [-Wunused-const-variable=]
>>       974 | static const struct of_device_id wm8961_of_match[] = {
>>           |                                  ^~~~~~~~~~~~~~~
> 
> Oops, nice catch by the kernel test robot. I will submit a V2 patch that

Now I see the report about issue I wrote to you. It's not particular
nice catch of robot... it's visible from the code and easily testable by
yourself. Even without compile test... The code was just not tested for
warnings.

> does this part exactly how the wm8960 driver does it, including
> .of_match_table in wm8961_i2c_driver. Waiting to see if I get any other
> feedback on V1 first.

maybe_unused instead of ifdefs.

Best regards,
Krzysztof

Re: [PATCH 2/2] ASoC: wm8961: add support for devicetree

[Doug Brown]

Hi Krzysztof,

On 10/2/2022 1:35 AM, Krzysztof Kozlowski wrote:

> Now I see the report about issue I wrote to you. It's not particular
> nice catch of robot... it's visible from the code and easily testable by
> yourself. Even without compile test... The code was just not tested for
> warnings.

Thanks for your thorough and timely review. It is much appreciated. I
will address everything you mentioned in V2, and will do a better job of
checking for warnings with multiple configs going forward.

Doug

Re: [PATCH 2/2] ASoC: wm8961: add support for devicetree

[Krzysztof Kozlowski]

On 01/10/2022 22:00, Doug Brown wrote:
> This adds support for devicetree to the WM8961 driver so it can be used

Do not use "This commit/patch adds ...".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

Just "Add support for ..."


> with modern DT-based kernels.
> 
> Signed-off-by: Doug Brown <doug@schmorgal.com>
> ---
>  sound/soc/codecs/Kconfig  | 2 +-
>  sound/soc/codecs/wm8961.c | 6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index e3b90c425faf..2b5787ee8d31 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -1929,7 +1929,7 @@ config SND_SOC_WM8960
>  	depends on I2C
>  
>  config SND_SOC_WM8961
> -	tristate
> +	tristate "Wolfson Microelectronics WM8961 CODEC"

This is independent change. Please split to separate commit.

>  	depends on I2C
>  
>  config SND_SOC_WM8962
> diff --git a/sound/soc/codecs/wm8961.c b/sound/soc/codecs/wm8961.c
> index 7dc6aaf65576..539096184eda 100644
> --- a/sound/soc/codecs/wm8961.c
> +++ b/sound/soc/codecs/wm8961.c
> @@ -971,6 +971,12 @@ static const struct i2c_device_id wm8961_i2c_id[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, wm8961_i2c_id);
>  
> +static const struct of_device_id wm8961_of_match[] = {
> +	{ .compatible = "wlf,wm8961", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, wm8961_of_match);

Compile-test with W=1 and without CONFIG_OF. Is there a warning here?

Best regards,
Krzysztof