[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 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,
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>; + }; + }; +...
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-max9...
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.
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-max9...
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.
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
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
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"; }; }; ...
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
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
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; }
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.
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().
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
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
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
participants (6)
-
Claudiu.Beznea＠microchip.com -
Krzysztof Kozlowski -
Mark Brown -
Richard Leitner -
richard.leitner＠linux.dev
-
Rob Herring