2020-07-07 10:09:31

by Agrawal, Akshu

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

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


2020-07-07 10:31:48

by Mark Brown

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

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.


Attachments:
(No filename) (611.00 B)
signature.asc (499.00 B)
Download all attachments

2020-07-13 01:18:16

by Agrawal, Akshu

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


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.

2020-07-13 10:53:36

by Mark Brown

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

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?


Attachments:
(No filename) (635.00 B)
signature.asc (499.00 B)
Download all attachments