[PATCH 0/3] Add "mclk" support for maxim,max9867

[PATCH 0/3] Add "mclk" support for maxim,max9867

[richard.leitner]

This series adds support for the clocks and clock-names properties in
the maxim,max9867 bindings. Furthermore the binding definitions are
converted from txt to yaml.

The mclk property is needed for one of our boards which uses the the
i.MX8MP SAI MCLK as clock for the maxim,max9867.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
Benjamin Bara (1):
      ASoC: maxim,max9867: add "mclk" support

Richard Leitner (2):
      ASoC: dt-bindings: maxim,max9867: convert txt bindings to yaml
      ASoC: dt-bindings: maxim,max9867: add "mclk" property

 .../devicetree/bindings/sound/max9867.txt          | 17 ------
 .../devicetree/bindings/sound/maxim,max9867.yaml   | 61 ++++++++++++++++++++++
 sound/soc/codecs/max9867.c                         | 14 ++++-
 3 files changed, 74 insertions(+), 18 deletions(-)
---
base-commit: c9c3395d5e3dcc6daee66c6908354d47bf98cb0c
change-id: 20230302-max9867-49081908a2ab

Best regards,
-- 
Richard Leitner <richard.leitner@skidata.com>

[PATCH 1/3] ASoC: dt-bindings: maxim,max9867: convert txt bindings to yaml

[richard.leitner]

From: Richard Leitner <richard.leitner@skidata.com>

Convert from max9867.txt to maxim,max9867.yaml and add missing
'#sound-dai-cells' property.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 .../devicetree/bindings/sound/max9867.txt          | 17 --------
 .../devicetree/bindings/sound/maxim,max9867.yaml   | 51 ++++++++++++++++++++++
 2 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/max9867.txt b/Documentation/devicetree/bindings/sound/max9867.txt
deleted file mode 100644
index b8bd914ee697..000000000000
--- a/Documentation/devicetree/bindings/sound/max9867.txt
+++ /dev/null
@@ -1,17 +0,0 @@
-max9867 codec
-
-This device supports I2C mode only.
-
-Required properties:
-
-- compatible : "maxim,max9867"
-- reg : The chip select number on the I2C bus
-
-Example:
-
-&i2c {
-	max9867: max9867@18 {
-		compatible = "maxim,max9867";
-		reg = <0x18>;
-	};
-};
diff --git a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
new file mode 100644
index 000000000000..cefa43c3d34e
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/max9867.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim Integrated MAX9867 CODEC
+
+description: |
+  This device supports I2C only.
+  Pins on the device (for linking into audio routes):
+      * LOUT
+      * ROUT
+      * LINL
+      * LINR
+      * MICL
+      * MICR
+      * DMICL
+      * DMICR
+
+maintainers:
+  - Ladislav Michl <ladis@linux-mips.org>
+
+allOf:
+  - $ref: dai-common.yaml#
+
+properties:
+  compatible:
+    enum:
+      - maxim,max9867
+
+  '#sound-dai-cells':
+    const: 0
+
+  reg:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+examples:
+  - |
+    &i2c {
+            max9867: max9867@18 {
+                    compatible = "maxim,max9867";
+                    #sound-dai-cells = <0>;
+                    reg = <0x18>;
+            };
+    };
+...

-- 
2.39.2

Re: [PATCH 1/3] ASoC: dt-bindings: maxim,max9867: convert txt bindings to yaml

[Rob Herring]

On Thu, 02 Mar 2023 12:55:01 +0100, richard.leitner@linux.dev wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
> 
> Convert from max9867.txt to maxim,max9867.yaml and add missing
> '#sound-dai-cells' property.
> 
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
>  .../devicetree/bindings/sound/max9867.txt          | 17 --------
>  .../devicetree/bindings/sound/maxim,max9867.yaml   | 51 ++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 17 deletions(-)
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/maxim,max9867.yaml: 'oneOf' conditional failed, one must be fixed:
	'unevaluatedProperties' is a required property
	'additionalProperties' is a required property
	hint: Either unevaluatedProperties or additionalProperties must be present
	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
./Documentation/devicetree/bindings/sound/maxim,max9867.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/sound/maxim,max9867.yaml#
Error: Documentation/devicetree/bindings/sound/maxim,max9867.example.dts:18.9-13 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/sound/maxim,max9867.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230302-max9867-v1-1-aa9f7f25db5e@skidata.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 1/3] ASoC: dt-bindings: maxim,max9867: convert txt bindings to yaml

[Richard Leitner]

On Thu, Mar 02, 2023 at 07:05:02AM -0600, Rob Herring wrote:
> 
> On Thu, 02 Mar 2023 12:55:01 +0100, richard.leitner@linux.dev wrote:
> > From: Richard Leitner <richard.leitner@skidata.com>
> > 
> > Convert from max9867.txt to maxim,max9867.yaml and add missing
> > '#sound-dai-cells' property.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> > ---
> >  .../devicetree/bindings/sound/max9867.txt          | 17 --------
> >  .../devicetree/bindings/sound/maxim,max9867.yaml   | 51 ++++++++++++++++++++++
> >  2 files changed, 51 insertions(+), 17 deletions(-)
> > 
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):

Thank you for the pointer, Rob!

Will fix those in v2 and from now on run 'make DT_CHECKER_FLAGS=-m
dt_binding_check' before sending any patches 😉

regards;rl

> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/sound/maxim,max9867.yaml: 'oneOf' conditional failed, one must be fixed:
> 	'unevaluatedProperties' is a required property
> 	'additionalProperties' is a required property
> 	hint: Either unevaluatedProperties or additionalProperties must be present
> 	from schema $id: http://devicetree.org/meta-schemas/core.yaml#
> ./Documentation/devicetree/bindings/sound/maxim,max9867.yaml: $id: relative path/filename doesn't match actual path or filename
> 	expected: http://devicetree.org/schemas/sound/maxim,max9867.yaml#
> Error: Documentation/devicetree/bindings/sound/maxim,max9867.example.dts:18.9-13 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/sound/maxim,max9867.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1508: dt_binding_check] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230302-max9867-v1-1-aa9f7f25db5e@skidata.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 

Re: [PATCH 1/3] ASoC: dt-bindings: maxim,max9867: convert txt bindings to yaml

[Krzysztof Kozlowski]

On 02/03/2023 12:55, richard.leitner@linux.dev wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
> 
> Convert from max9867.txt to maxim,max9867.yaml and add missing
> '#sound-dai-cells' property.

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

Except wrong ID (and missing test):

> +
> +examples:
> +  - |
> +    &i2c {
> +            max9867: max9867@18 {

Generic node names, so "codec" and drop the unused label.

Use 4 spaces for example indentation.

> +                    compatible = "maxim,max9867";
> +                    #sound-dai-cells = <0>;
> +                    reg = <0x18>;

Best regards,
Krzysztof

Re: [PATCH 1/3] ASoC: dt-bindings: maxim,max9867: convert txt bindings to yaml

[Richard Leitner]

On Thu, Mar 02, 2023 at 02:31:14PM +0100, Krzysztof Kozlowski wrote:
> On 02/03/2023 12:55, richard.leitner@linux.dev wrote:
> > From: Richard Leitner <richard.leitner@skidata.com>
> > 
> > Convert from max9867.txt to maxim,max9867.yaml and add missing
> > '#sound-dai-cells' property.
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> Except wrong ID (and missing test):
> 
> > +
> > +examples:
> > +  - |
> > +    &i2c {
> > +            max9867: max9867@18 {
> 
> Generic node names, so "codec" and drop the unused label.

Thanks for the review and feedback. I'll fix that in v2.

> 
> Use 4 spaces for example indentation.

Ok. checkpatch.pl didn't complain about that so I thought this was
fine. Are there any other scripts/tools to check for correct formatting
of bindings?

> 
> > +                    compatible = "maxim,max9867";
> > +                    #sound-dai-cells = <0>;
> > +                    reg = <0x18>;
> 
> Best regards,
> Krzysztof
> 

Thanks & regards;rl

[PATCH 2/3] ASoC: dt-bindings: maxim,max9867: add "mclk" property

[richard.leitner]

From: Richard Leitner <richard.leitner@skidata.com>

Add clocks and clock-names properties to require a "mclk" definition for
the maxim,max9867 codec.

Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
---
 Documentation/devicetree/bindings/sound/maxim,max9867.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
index cefa43c3d34e..152340fe9cc7 100644
--- a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
+++ b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
@@ -35,9 +35,17 @@ properties:
   reg:
     maxItems: 1
 
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: "mclk"
+
 required:
   - compatible
   - reg
+  - clocks
+  - clock-names
 
 examples:
   - |
@@ -46,6 +54,8 @@ examples:
                     compatible = "maxim,max9867";
                     #sound-dai-cells = <0>;
                     reg = <0x18>;
+                    clocks = <&audio_blk_ctrl IMX8MP_CLK_AUDIOMIX_SAI3_MCLK1>;
+                    clock-names = "mclk";
             };
     };
 ...

-- 
2.39.2

Re: [PATCH 2/3] ASoC: dt-bindings: maxim,max9867: add "mclk" property

[Krzysztof Kozlowski]

On 02/03/2023 12:55, richard.leitner@linux.dev wrote:
> From: Richard Leitner <richard.leitner@skidata.com>
> 
> Add clocks and clock-names properties to require a "mclk" definition for
> the maxim,max9867 codec.
> 
> Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> ---
>  Documentation/devicetree/bindings/sound/maxim,max9867.yaml | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
> index cefa43c3d34e..152340fe9cc7 100644
> --- a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
> +++ b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
> @@ -35,9 +35,17 @@ properties:
>    reg:
>      maxItems: 1
>  
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: "mclk"

Drop entire property, you do not need it for one clock.

Best regards,
Krzysztof

Re: [PATCH 2/3] ASoC: dt-bindings: maxim,max9867: add "mclk" property

[Richard Leitner]

On Thu, Mar 02, 2023 at 02:31:45PM +0100, Krzysztof Kozlowski wrote:
> On 02/03/2023 12:55, richard.leitner@linux.dev wrote:
> > From: Richard Leitner <richard.leitner@skidata.com>
> > 
> > Add clocks and clock-names properties to require a "mclk" definition for
> > the maxim,max9867 codec.
> > 
> > Signed-off-by: Richard Leitner <richard.leitner@skidata.com>
> > ---
> >  Documentation/devicetree/bindings/sound/maxim,max9867.yaml | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
> > index cefa43c3d34e..152340fe9cc7 100644
> > --- a/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
> > +++ b/Documentation/devicetree/bindings/sound/maxim,max9867.yaml
> > @@ -35,9 +35,17 @@ properties:
> >    reg:
> >      maxItems: 1
> >  
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  clock-names:
> > +    const: "mclk"
> 
> Drop entire property, you do not need it for one clock.

Thanks. Will fix that in v2.

> 
> Best regards,
> Krzysztof
> 

regards;rl

[PATCH 3/3] ASoC: maxim,max9867: add "mclk" support

[richard.leitner]

From: Benjamin Bara <benjamin.bara@skidata.com>

Add basic support for the codecs mclk by enabling it during probing.

Signed-off-by: Benjamin Bara <benjamin.bara@skidata.com>
---
 sound/soc/codecs/max9867.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/max9867.c b/sound/soc/codecs/max9867.c
index e161ab037bf7..b92dd61bb2b2 100644
--- a/sound/soc/codecs/max9867.c
+++ b/sound/soc/codecs/max9867.c
@@ -6,6 +6,7 @@
 // Copyright 2018 Ladislav Michl <ladis@linux-mips.org>
 //
 
+#include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/i2c.h>
 #include <linux/module.h>
@@ -16,6 +17,7 @@
 #include "max9867.h"
 
 struct max9867_priv {
+	struct clk *mclk;
 	struct regmap *regmap;
 	const struct snd_pcm_hw_constraint_list *constraints;
 	unsigned int sysclk, pclk;
@@ -663,8 +665,18 @@ static int max9867_i2c_probe(struct i2c_client *i2c)
 	dev_info(&i2c->dev, "device revision: %x\n", reg);
 	ret = devm_snd_soc_register_component(&i2c->dev, &max9867_component,
 			max9867_dai, ARRAY_SIZE(max9867_dai));
-	if (ret < 0)
+	if (ret < 0) {
 		dev_err(&i2c->dev, "Failed to register component: %d\n", ret);
+		return ret;
+	}
+
+	max9867->mclk = devm_clk_get(&i2c->dev, "mclk");
+	if (IS_ERR(max9867->mclk))
+		return PTR_ERR(max9867->mclk);
+	ret = clk_prepare_enable(max9867->mclk);
+	if (ret < 0)
+		dev_err(&i2c->dev, "Failed to enable MCLK: %d\n", ret);
+
 	return ret;
 }
 

-- 
2.39.2

Re: [PATCH 3/3] ASoC: maxim,max9867: add "mclk" support

[Mark Brown]

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

On Thu, Mar 02, 2023 at 12:55:03PM +0100, richard.leitner@linux.dev wrote:

> +	max9867->mclk = devm_clk_get(&i2c->dev, "mclk");
> +	if (IS_ERR(max9867->mclk))
> +		return PTR_ERR(max9867->mclk);
> +	ret = clk_prepare_enable(max9867->mclk);
> +	if (ret < 0)
> +		dev_err(&i2c->dev, "Failed to enable MCLK: %d\n", ret);
> +

Nothing ever disables the clock - we need a disable in the remove path
at least.

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

Re: [PATCH 3/3] ASoC: maxim,max9867: add "mclk" support

[Claudiu.Beznea]

On 02.03.2023 14:20, Mark Brown wrote:
>> +	max9867->mclk = devm_clk_get(&i2c->dev, "mclk");
>> +	if (IS_ERR(max9867->mclk))
>> +		return PTR_ERR(max9867->mclk);
>> +	ret = clk_prepare_enable(max9867->mclk);
>> +	if (ret < 0)
>> +		dev_err(&i2c->dev, "Failed to enable MCLK: %d\n", ret);
>> +
> Nothing ever disables the clock - we need a disable in the remove path
> at least.

I don't have the full context of this patch but this diff seems a good
candidate for devm_clk_get_enabled().

Re: [PATCH 3/3] ASoC: maxim,max9867: add "mclk" support

[Richard Leitner]

Hi Claudiu,

On Thu, Mar 02, 2023 at 12:45:50PM +0000, Claudiu.Beznea@microchip.com wrote:
> On 02.03.2023 14:20, Mark Brown wrote:
> >> +	max9867->mclk = devm_clk_get(&i2c->dev, "mclk");
> >> +	if (IS_ERR(max9867->mclk))
> >> +		return PTR_ERR(max9867->mclk);
> >> +	ret = clk_prepare_enable(max9867->mclk);
> >> +	if (ret < 0)
> >> +		dev_err(&i2c->dev, "Failed to enable MCLK: %d\n", ret);
> >> +
> > Nothing ever disables the clock - we need a disable in the remove path
> > at least.
> 
> I don't have the full context of this patch but this diff seems a good
> candidate for devm_clk_get_enabled().

Thanks for that pointer, but currently we are thinking of prepare_enable
the clock in SND_SOC_BIAS_ON and disable_unprepare it in SND_SOC_BIAS_OFF
(similar to wm8731.c).
Therefore probe() will only do a devm_clk_get().

Claudiu, Rob: Will this be an acceptable solution?

regards;rl

Re: [PATCH 3/3] ASoC: maxim,max9867: add "mclk" support

[Claudiu.Beznea]

On 02.03.2023 16:46, Richard Leitner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi Claudiu,
> 
> On Thu, Mar 02, 2023 at 12:45:50PM +0000, Claudiu.Beznea@microchip.com wrote:
>> On 02.03.2023 14:20, Mark Brown wrote:
>>>> +  max9867->mclk = devm_clk_get(&i2c->dev, "mclk");
>>>> +  if (IS_ERR(max9867->mclk))
>>>> +          return PTR_ERR(max9867->mclk);
>>>> +  ret = clk_prepare_enable(max9867->mclk);
>>>> +  if (ret < 0)
>>>> +          dev_err(&i2c->dev, "Failed to enable MCLK: %d\n", ret);
>>>> +
>>> Nothing ever disables the clock - we need a disable in the remove path
>>> at least.
>>
>> I don't have the full context of this patch but this diff seems a good
>> candidate for devm_clk_get_enabled().
> 
> Thanks for that pointer, but currently we are thinking of prepare_enable
> the clock in SND_SOC_BIAS_ON and disable_unprepare it in SND_SOC_BIAS_OFF
> (similar to wm8731.c).
> Therefore probe() will only do a devm_clk_get().

Sounds good for me.

> 
> Claudiu, Rob: Will this be an acceptable solution?
> 
> regards;rl

Re: [PATCH 3/3] ASoC: maxim,max9867: add "mclk" support

[Richard Leitner]

On Thu, Mar 02, 2023 at 12:20:18PM +0000, Mark Brown wrote:
> On Thu, Mar 02, 2023 at 12:55:03PM +0100, richard.leitner@linux.dev wrote:
> 
> > +	max9867->mclk = devm_clk_get(&i2c->dev, "mclk");
> > +	if (IS_ERR(max9867->mclk))
> > +		return PTR_ERR(max9867->mclk);
> > +	ret = clk_prepare_enable(max9867->mclk);
> > +	if (ret < 0)
> > +		dev_err(&i2c->dev, "Failed to enable MCLK: %d\n", ret);
> > +
> 
> Nothing ever disables the clock - we need a disable in the remove path
> at least.

Sure. Sorry for missing that. I will send a v2 later today.

regards;rl