[PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Akshu Agrawal]

Non-dts based systems can use ACPI DSDT to pass on the mclk.
Thus add fmw property mclk-name to get the name of the system clk
and link it to rt5682 mclk.

Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
---
 include/sound/rt5682.h    | 1 +
 sound/soc/codecs/rt5682.c | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/sound/rt5682.h b/include/sound/rt5682.h
index e1f790561ac1..8717c02f89b8 100644
--- a/include/sound/rt5682.h
+++ b/include/sound/rt5682.h
@@ -42,6 +42,7 @@ struct rt5682_platform_data {
 	unsigned int dmic_delay;
 
 	const char *dai_clk_names[RT5682_DAI_NUM_CLKS];
+	const char *mclk_name;
 };
 
 #endif
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c
index de40b6cd16cf..8b223bd0929e 100644
--- a/sound/soc/codecs/rt5682.c
+++ b/sound/soc/codecs/rt5682.c
@@ -2849,7 +2849,10 @@ static int rt5682_probe(struct snd_soc_component *component)
 	} else {
 #ifdef CONFIG_COMMON_CLK
 		/* Check if MCLK provided */
-		rt5682->mclk = devm_clk_get(component->dev, "mclk");
+		if (rt5682->pdata.mclk_name)
+			rt5682->mclk = clk_get(NULL, rt5682->pdata.mclk_name);
+		if (!rt5682->mclk)
+			rt5682->mclk = devm_clk_get(component->dev, "mclk");
 		if (IS_ERR(rt5682->mclk)) {
 			if (PTR_ERR(rt5682->mclk) != -ENOENT) {
 				ret = PTR_ERR(rt5682->mclk);
@@ -2976,6 +2979,8 @@ int rt5682_parse_dt(struct rt5682_priv *rt5682, struct device *dev)
 			 rt5682->pdata.dai_clk_names[RT5682_DAI_WCLK_IDX],
 			 rt5682->pdata.dai_clk_names[RT5682_DAI_BCLK_IDX]);
 
+	device_property_read_string(dev, "realtek,mclk-name", &rt5682->pdata.mclk_name);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt5682_parse_dt);
-- 
2.20.1

[PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Akshu Agrawal]

Non-dts based systems can use ACPI DSDT to pass on the mclk.
Thus add fmw property mclk-name to get the name of the system clk
and link it to rt5682 mclk.

Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com>
---
 include/sound/rt5682.h    | 1 +
 sound/soc/codecs/rt5682.c | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/sound/rt5682.h b/include/sound/rt5682.h
index e1f790561ac1..8717c02f89b8 100644
--- a/include/sound/rt5682.h
+++ b/include/sound/rt5682.h
@@ -42,6 +42,7 @@ struct rt5682_platform_data {
 	unsigned int dmic_delay;
 
 	const char *dai_clk_names[RT5682_DAI_NUM_CLKS];
+	const char *mclk_name;
 };
 
 #endif
diff --git a/sound/soc/codecs/rt5682.c b/sound/soc/codecs/rt5682.c
index de40b6cd16cf..8b223bd0929e 100644
--- a/sound/soc/codecs/rt5682.c
+++ b/sound/soc/codecs/rt5682.c
@@ -2849,7 +2849,10 @@ static int rt5682_probe(struct snd_soc_component *component)
 	} else {
 #ifdef CONFIG_COMMON_CLK
 		/* Check if MCLK provided */
-		rt5682->mclk = devm_clk_get(component->dev, "mclk");
+		if (rt5682->pdata.mclk_name)
+			rt5682->mclk = clk_get(NULL, rt5682->pdata.mclk_name);
+		if (!rt5682->mclk)
+			rt5682->mclk = devm_clk_get(component->dev, "mclk");
 		if (IS_ERR(rt5682->mclk)) {
 			if (PTR_ERR(rt5682->mclk) != -ENOENT) {
 				ret = PTR_ERR(rt5682->mclk);
@@ -2976,6 +2979,8 @@ int rt5682_parse_dt(struct rt5682_priv *rt5682, struct device *dev)
 			 rt5682->pdata.dai_clk_names[RT5682_DAI_WCLK_IDX],
 			 rt5682->pdata.dai_clk_names[RT5682_DAI_BCLK_IDX]);
 
+	device_property_read_string(dev, "realtek,mclk-name", &rt5682->pdata.mclk_name);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt5682_parse_dt);
-- 
2.20.1

Re: [PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Mark Brown]

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

On Tue, Jul 07, 2020 at 03:38:25PM +0530, Akshu Agrawal wrote:
> Non-dts based systems can use ACPI DSDT to pass on the mclk.
> Thus add fmw property mclk-name to get the name of the system clk
> and link it to rt5682 mclk.

ACPI doesn't support clocks at all, you need to add a clock binding to
ACPI first.  The idiomatic way to do this would be to have board
specific quirks.

> +	device_property_read_string(dev, "realtek,mclk-name", &rt5682->pdata.mclk_name);
> +

No, this is not at all OK - you're adding this via a device property
which means that this will show up in the DT bindings too.

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

Re: [PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Mark Brown]

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

On Tue, Jul 07, 2020 at 03:38:25PM +0530, Akshu Agrawal wrote:
> Non-dts based systems can use ACPI DSDT to pass on the mclk.
> Thus add fmw property mclk-name to get the name of the system clk
> and link it to rt5682 mclk.

ACPI doesn't support clocks at all, you need to add a clock binding to
ACPI first.  The idiomatic way to do this would be to have board
specific quirks.

> +	device_property_read_string(dev, "realtek,mclk-name", &rt5682->pdata.mclk_name);
> +

No, this is not at all OK - you're adding this via a device property
which means that this will show up in the DT bindings too.

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

Re: [PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Agrawal, Akshu]

On 7/7/2020 4:00 PM, Mark Brown wrote:
> On Tue, Jul 07, 2020 at 03:38:25PM +0530, Akshu Agrawal wrote:
>> Non-dts based systems can use ACPI DSDT to pass on the mclk.
>> Thus add fmw property mclk-name to get the name of the system clk
>> and link it to rt5682 mclk.
> ACPI doesn't support clocks at all, you need to add a clock binding to
> ACPI first.  The idiomatic way to do this would be to have board
> specific quirks.

clk binding is present for AMD ST platform and using the same.

With recent submitted patches I am making them generic for all AMD 
platforms.

Please refer patches:

https://patchwork.kernel.org/patch/11658505/

https://patchwork.kernel.org/patch/11658507/

Thanks,

Akshu

>
>> +	device_property_read_string(dev, "realtek,mclk-name", &rt5682->pdata.mclk_name);
>> +
> No, this is not at all OK - you're adding this via a device property
> which means that this will show up in the DT bindings too.

Re: [PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Agrawal, Akshu]

On 7/7/2020 4:00 PM, Mark Brown wrote:
> On Tue, Jul 07, 2020 at 03:38:25PM +0530, Akshu Agrawal wrote:
>> Non-dts based systems can use ACPI DSDT to pass on the mclk.
>> Thus add fmw property mclk-name to get the name of the system clk
>> and link it to rt5682 mclk.
> ACPI doesn't support clocks at all, you need to add a clock binding to
> ACPI first.  The idiomatic way to do this would be to have board
> specific quirks.

clk binding is present for AMD ST platform and using the same.

With recent submitted patches I am making them generic for all AMD 
platforms.

Please refer patches:

https://patchwork.kernel.org/patch/11658505/

https://patchwork.kernel.org/patch/11658507/

Thanks,

Akshu

>
>> +	device_property_read_string(dev, "realtek,mclk-name", &rt5682->pdata.mclk_name);
>> +
> No, this is not at all OK - you're adding this via a device property
> which means that this will show up in the DT bindings too.

Re: [PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Mark Brown]

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

On Mon, Jul 13, 2020 at 06:46:29AM +0530, Agrawal, Akshu wrote:

> clk binding is present for AMD ST platform and using the same.

This is something you should be doing through UEFI forum as a generic
ACPI thing, and if you need to read the name from the firmware that
really does sound like something that should be raising red flags as a
binding.

> With recent submitted patches I am making them generic for all AMD
> platforms.

> Please refer patches:

> https://patchwork.kernel.org/patch/11658505/

> https://patchwork.kernel.org/patch/11658507/

It looks like there's clock names hard coded into the driver?

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

Re: [PATCH] ASoC: rt5682: Add fmw property to get name of mclk

[Mark Brown]

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

On Mon, Jul 13, 2020 at 06:46:29AM +0530, Agrawal, Akshu wrote:

> clk binding is present for AMD ST platform and using the same.

This is something you should be doing through UEFI forum as a generic
ACPI thing, and if you need to read the name from the firmware that
really does sound like something that should be raising red flags as a
binding.

> With recent submitted patches I am making them generic for all AMD
> platforms.

> Please refer patches:

> https://patchwork.kernel.org/patch/11658505/

> https://patchwork.kernel.org/patch/11658507/

It looks like there's clock names hard coded into the driver?

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