[alsa-devel] [PATCH 0/5] MAX98088/9 of features
Hi,
I picked up the max98088 patches from Andreas series [1] and ported them to the new kernel.
Regards, Marco
[1] http://lists.infradead.org/pipermail/linux-arm-kernel/ \ 2015-February/324440.html
Andreas Färber (3): ASoC: dt-bindings: add max98088 audio codec ASoC: max98088: add OF support ASoC: max98088: Add master clock handling
Marco Felsch (2): ASoC: dt-bindings: max98088: add external clock binding ASoC: max98988: make it selectable
.../bindings/sound/maxim,max98088.txt | 20 ++++++++++ sound/soc/codecs/Kconfig | 2 +- sound/soc/codecs/max98088.c | 39 +++++++++++++++++++ 3 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/sound/maxim,max98088.txt
From: Andreas Färber afaerber@suse.de
This patch adds the bindings for maxim max98088/9 audio codec.
Signed-off-by: Andreas Färber afaerber@suse.de [m.felsch@pengutronix.de: adapt commit message] [m.felsch@pengutronix.de: adapt formatting] Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- .../devicetree/bindings/sound/maxim,max98088.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/maxim,max98088.txt
diff --git a/Documentation/devicetree/bindings/sound/maxim,max98088.txt b/Documentation/devicetree/bindings/sound/maxim,max98088.txt new file mode 100644 index 000000000000..a79ad5c7af88 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/maxim,max98088.txt @@ -0,0 +1,15 @@ +MAX98088 audio CODEC + +This device supports I2C only. + +Required properties: + +- compatible: "maxim,max98088" or "maxim,max98089". +- reg: The I2C address of the device. + +Example: + +max98089: codec@10 { + compatible = "maxim,max98089"; + reg = <0x10>; +};
From: Andreas Färber afaerber@suse.de
MAX98088 is an older version of the MAX98089 device.
Signed-off-by: Andreas Färber afaerber@suse.de [m.felsch@pengutronix.de: add CONFIG_OF compile switch] [m.felsch@pengutronix.de: adapt commit message] Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- sound/soc/codecs/max98088.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index fb515aaa54fc..9450d5d9c492 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -1742,9 +1742,19 @@ static const struct i2c_device_id max98088_i2c_id[] = { }; MODULE_DEVICE_TABLE(i2c, max98088_i2c_id);
+#if defined(CONFIG_OF) +static const struct of_device_id max98088_of_match[] = { + { .compatible = "maxim,max98088" }, + { .compatible = "maxim,max98089" }, + { } +}; +MODULE_DEVICE_TABLE(of, max98088_of_match); +#endif + static struct i2c_driver max98088_i2c_driver = { .driver = { .name = "max98088", + .of_match_table = of_match_ptr(max98088_of_match), }, .probe = max98088_i2c_probe, .id_table = max98088_i2c_id,
On Tue, Sep 25, 2018 at 04:23:49PM +0200, Marco Felsch wrote:
From: Andreas Färber afaerber@suse.de
MAX98088 is an older version of the MAX98089 device.
With no software visible diffferences?
On 18-09-25 10:11, Mark Brown wrote:
On Tue, Sep 25, 2018 at 04:23:49PM +0200, Marco Felsch wrote:
From: Andreas Färber afaerber@suse.de
MAX98088 is an older version of the MAX98089 device.
With no software visible diffferences?
I reviewd both RMs a 2nd time and see only a realy minor difference for register 0x02 and 0x4e. Both covers a different jack plugin/off handling.
0x02: MAX98088 - Bit6 indicate JACKSNS State MAX98089 - Bit6/7 indicates JACKSNS State
But this register is never read by the driver.
0x4e: MAX98088 - Bit0 is reserved MAX98089 - Bit0 is JDWK
The register has the default value for both chips and this value is set during probe().
So I see no other software differences, but maybe a Maxim FAE can help us?
Regards, Marco
Allow setting the external clock called "mclk" in the device tree.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- Documentation/devicetree/bindings/sound/maxim,max98088.txt | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/maxim,max98088.txt b/Documentation/devicetree/bindings/sound/maxim,max98088.txt index a79ad5c7af88..c575806c9e18 100644 --- a/Documentation/devicetree/bindings/sound/maxim,max98088.txt +++ b/Documentation/devicetree/bindings/sound/maxim,max98088.txt @@ -6,10 +6,15 @@ Required properties:
- compatible: "maxim,max98088" or "maxim,max98089". - reg: The I2C address of the device. +- clocks: the clock provider of MCLK, see ../clock/clock-bindings.txt section + "consumer" for more information. +- clock-names: must be set to "mclk"
Example:
max98089: codec@10 { compatible = "maxim,max98089"; reg = <0x10>; + clocks = <&clks IMX6QDL_CLK_CKO2>; + clock-names = "mclk"; };
From: Andreas Färber afaerber@suse.de
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
Cc: Tushar Behera tushar.behera@linaro.org Cc: Javier Martinez Canillas javier.martinez@collabora.co.uk Signed-off-by: Andreas Färber afaerber@suse.de Acked-by: Tushar Behera trblinux@gmail.com Reviewed-by: Javier Martinez Canillas javier.martinez@collabora.co.uk [m.felsch@pengutronix.de: move mclk request to i2c_probe] [m.felsch@pengutronix.de: make use of snd_soc_component_get_bias_level()] Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- sound/soc/codecs/max98088.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+)
diff --git a/sound/soc/codecs/max98088.c b/sound/soc/codecs/max98088.c index 9450d5d9c492..01f1ea0af9c1 100644 --- a/sound/soc/codecs/max98088.c +++ b/sound/soc/codecs/max98088.c @@ -16,6 +16,7 @@ #include <linux/pm.h> #include <linux/i2c.h> #include <linux/regmap.h> +#include <linux/clk.h> #include <sound/core.h> #include <sound/pcm.h> #include <sound/pcm_params.h> @@ -42,6 +43,7 @@ struct max98088_priv { struct regmap *regmap; enum max98088_type devtype; struct max98088_pdata *pdata; + struct clk *mclk; unsigned int sysclk; struct max98088_cdata dai[2]; int eq_textcnt; @@ -1103,6 +1105,11 @@ static int max98088_dai_set_sysclk(struct snd_soc_dai *dai, if (freq == max98088->sysclk) return 0;
+ if (!IS_ERR(max98088->mclk)) { + freq = clk_round_rate(max98088->mclk, freq); + clk_set_rate(max98088->mclk, freq); + } + /* Setup clocks for slave mode, and using the PLL * PSCLK = 0x01 (when master clk is 10MHz to 20MHz) * 0x02 (when master clk is 20MHz to 30MHz).. @@ -1310,6 +1317,20 @@ static int max98088_set_bias_level(struct snd_soc_component *component, break;
case SND_SOC_BIAS_PREPARE: + /* + * SND_SOC_BIAS_PREPARE is called while preparing for a + * transition to ON or away from ON. If current bias_level + * is SND_SOC_BIAS_ON, then it is preparing for a transition + * away from ON. Disable the clock in that case, otherwise + * enable it. + */ + if (!IS_ERR(max98088->mclk)) { + if (snd_soc_component_get_bias_level(component) == + SND_SOC_BIAS_ON) + clk_disable_unprepare(max98088->mclk); + else + clk_prepare_enable(max98088->mclk); + } break;
case SND_SOC_BIAS_STANDBY: @@ -1725,6 +1746,14 @@ static int max98088_i2c_probe(struct i2c_client *i2c, if (IS_ERR(max98088->regmap)) return PTR_ERR(max98088->regmap);
+ max98088->mclk = devm_clk_get(&i2c->dev, "mclk"); + if (IS_ERR(max98088->mclk)) { + if (PTR_ERR(max98088->mclk) == -EPROBE_DEFER) + return -EPROBE_DEFER; + dev_err(&i2c->dev, "Invalid MCLK\n"); + return -EINVAL; + } + max98088->devtype = id->driver_data;
i2c_set_clientdata(i2c, max98088);
On Tue, Sep 25, 2018 at 04:23:51PM +0200, Marco Felsch wrote:
From: Andreas Färber afaerber@suse.de
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
This changelog suggests that the clock is optional...
- if (!IS_ERR(max98088->mclk)) {
freq = clk_round_rate(max98088->mclk, freq);clk_set_rate(max98088->mclk, freq);- }
...as does much of the code (note BTw that this ignores errors setting the clock)...
- max98088->mclk = devm_clk_get(&i2c->dev, "mclk");
- if (IS_ERR(max98088->mclk)) {
if (PTR_ERR(max98088->mclk) == -EPROBE_DEFER)return -EPROBE_DEFER;dev_err(&i2c->dev, "Invalid MCLK\n");return -EINVAL;- }
...but the probe function makes it mandatory (and also throws away the error code from clk_get() if it's not -EPROBE_DEFER). Is this the best choice? It seems inconsistent anyway.
Hi Mark,
thanks for review.
On 18-09-25 10:17, Mark Brown wrote:
On Tue, Sep 25, 2018 at 04:23:51PM +0200, Marco Felsch wrote:
From: Andreas Färber afaerber@suse.de
If master clock is provided through device tree, then update the master clock frequency during set_sysclk.
This changelog suggests that the clock is optional...
Your are right, it was my fail to make it not optional.
- if (!IS_ERR(max98088->mclk)) {
freq = clk_round_rate(max98088->mclk, freq);clk_set_rate(max98088->mclk, freq);- }
...as does much of the code (note BTw that this ignores errors setting the clock)...
- max98088->mclk = devm_clk_get(&i2c->dev, "mclk");
- if (IS_ERR(max98088->mclk)) {
if (PTR_ERR(max98088->mclk) == -EPROBE_DEFER)return -EPROBE_DEFER;dev_err(&i2c->dev, "Invalid MCLK\n");return -EINVAL;- }
...but the probe function makes it mandatory (and also throws away the error code from clk_get() if it's not -EPROBE_DEFER). Is this the best choice? It seems inconsistent anyway.
One question, should it optional or not? If not I will fix it to return the clk_get error else I will drop it. It shouldn't be optional In my point of view, since it is needed by the chip.
Regards, Marco
On Wed, Sep 26, 2018 at 12:00:30PM +0200, Marco Felsch wrote:
One question, should it optional or not? If not I will fix it to return the clk_get error else I will drop it. It shouldn't be optional In my point of view, since it is needed by the chip.
Optional seems safer, requiring the clock isn't going to work so well on ACPI systems.
On 18-09-26 13:22, Mark Brown wrote:
On Wed, Sep 26, 2018 at 12:00:30PM +0200, Marco Felsch wrote:
One question, should it optional or not? If not I will fix it to return the clk_get error else I will drop it. It shouldn't be optional In my point of view, since it is needed by the chip.
Optional seems safer, requiring the clock isn't going to work so well on ACPI systems.
Okay, I will integrate it in my v2.
Signed-off-by: Marco Felsch m.felsch@pengutronix.de --- sound/soc/codecs/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig index efb095dbcd71..b43f85b38e72 100644 --- a/sound/soc/codecs/Kconfig +++ b/sound/soc/codecs/Kconfig @@ -629,7 +629,7 @@ config SND_SOC_LM49453 tristate
config SND_SOC_MAX98088 - tristate + tristate "Maxim MAX98088/9 Low-Power, Stereo Audio Codec"
config SND_SOC_MAX98090 tristate
participants (2)
-
Marco Felsch -
Mark Brown