2023-03-02 11:57:34

by Richard Leitner

[permalink] [raw]
Subject: [PATCH 0/3] Add "mclk" support for maxim,max9867

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 <[email protected]>
---
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 <[email protected]>



2023-03-02 11:57:38

by Richard Leitner

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

From: Richard Leitner <[email protected]>

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

Signed-off-by: Richard Leitner <[email protected]>
---
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


2023-03-02 11:57:41

by Richard Leitner

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

From: Richard Leitner <[email protected]>

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

Signed-off-by: Richard Leitner <[email protected]>
---
.../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 <[email protected]>
+
+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


2023-03-02 11:57:44

by Richard Leitner

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

From: Benjamin Bara <[email protected]>

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

Signed-off-by: Benjamin Bara <[email protected]>
---
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 <[email protected]>
//

+#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


2023-03-02 12:20:30

by Mark Brown

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

On Thu, Mar 02, 2023 at 12:55:03PM +0100, [email protected] 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.


Attachments:
(No filename) (405.00 B)
signature.asc (488.00 B)
Download all attachments

2023-03-02 12:46:01

by Claudiu Beznea

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

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().

2023-03-02 12:51:56

by Richard Leitner

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

On Thu, Mar 02, 2023 at 12:20:18PM +0000, Mark Brown wrote:
> On Thu, Mar 02, 2023 at 12:55:03PM +0100, [email protected] 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


2023-03-02 13:05:10

by Rob Herring

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


On Thu, 02 Mar 2023 12:55:01 +0100, [email protected] wrote:
> From: Richard Leitner <[email protected]>
>
> Convert from max9867.txt to maxim,max9867.yaml and add missing
> '#sound-dai-cells' property.
>
> Signed-off-by: Richard Leitner <[email protected]>
> ---
> .../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/[email protected]

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.


2023-03-02 13:31:23

by Krzysztof Kozlowski

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

On 02/03/2023 12:55, [email protected] wrote:
> From: Richard Leitner <[email protected]>
>
> 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


2023-03-02 13:31:55

by Krzysztof Kozlowski

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

On 02/03/2023 12:55, [email protected] wrote:
> From: Richard Leitner <[email protected]>
>
> Add clocks and clock-names properties to require a "mclk" definition for
> the maxim,max9867 codec.
>
> Signed-off-by: Richard Leitner <[email protected]>
> ---
> 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


2023-03-02 13:48:18

by Richard Leitner

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

On Thu, Mar 02, 2023 at 02:31:14PM +0100, Krzysztof Kozlowski wrote:
> On 02/03/2023 12:55, [email protected] wrote:
> > From: Richard Leitner <[email protected]>
> >
> > 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

2023-03-02 14:35:08

by Richard Leitner

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

On Thu, Mar 02, 2023 at 07:05:02AM -0600, Rob Herring wrote:
>
> On Thu, 02 Mar 2023 12:55:01 +0100, [email protected] wrote:
> > From: Richard Leitner <[email protected]>
> >
> > Convert from max9867.txt to maxim,max9867.yaml and add missing
> > '#sound-dai-cells' property.
> >
> > Signed-off-by: Richard Leitner <[email protected]>
> > ---
> > .../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/[email protected]
>
> 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.
>

2023-03-02 14:46:51

by Richard Leitner

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

Hi Claudiu,

On Thu, Mar 02, 2023 at 12:45:50PM +0000, [email protected] 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

2023-03-02 14:57:05

by Richard Leitner

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

On Thu, Mar 02, 2023 at 02:31:45PM +0100, Krzysztof Kozlowski wrote:
> On 02/03/2023 12:55, [email protected] wrote:
> > From: Richard Leitner <[email protected]>
> >
> > Add clocks and clock-names properties to require a "mclk" definition for
> > the maxim,max9867 codec.
> >
> > Signed-off-by: Richard Leitner <[email protected]>
> > ---
> > 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

2023-03-03 10:01:01

by Claudiu Beznea

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

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, [email protected] 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