[PATCH] ASoC: da7219: read fmw property to get mclk for non-dts systems
From: Akshu Agrawal akshu.agrawal@amd.com
Non-dts based systems can use ACPI DSDT to pass on the mclk to da7219. This enables da7219 mclk to be linked to system clock. Enable/Disable of the mclk is already handled in the codec so platform drivers don't have to explicitly do handling of mclk.
Signed-off-by: Akshu Agrawal akshu.agrawal@amd.com Signed-off-by: Curtis Malainey cujomalainey@chromium.org Cc: Karthik Ramasubramanian kramasub@google.com --- include/sound/da7219.h | 2 ++ sound/soc/codecs/da7219.c | 8 +++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/sound/da7219.h b/include/sound/da7219.h index dde4542b084f3..4cf4c52be3f7f 100644 --- a/include/sound/da7219.h +++ b/include/sound/da7219.h @@ -40,6 +40,8 @@ struct da7219_pdata {
const char *dai_clk_names[DA7219_DAI_NUM_CLKS];
+ const char *mclk_name; + /* Mic */ enum da7219_micbias_voltage micbias_lvl; enum da7219_mic_amp_in_sel mic_amp_in_sel; diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c index 311ea7918b312..800f61d993043 100644 --- a/sound/soc/codecs/da7219.c +++ b/sound/soc/codecs/da7219.c @@ -1782,6 +1782,8 @@ static struct da7219_pdata *da7219_fw_to_pdata(struct device *dev) pdata->dai_clk_names[DA7219_DAI_WCLK_IDX], pdata->dai_clk_names[DA7219_DAI_BCLK_IDX]);
+ device_property_read_string(dev, "dlg,mclk-name", &pdata->mclk_name); + if (device_property_read_u32(dev, "dlg,micbias-lvl", &of_val32) >= 0) pdata->micbias_lvl = da7219_fw_micbias_lvl(dev, of_val32); else @@ -2521,7 +2523,11 @@ static int da7219_probe(struct snd_soc_component *component) da7219_handle_pdata(component);
/* Check if MCLK provided */ - da7219->mclk = clk_get(component->dev, "mclk"); + if (da7219->pdata->mclk_name) + da7219->mclk = clk_get(NULL, da7219->pdata->mclk_name); + if (!da7219->mclk) + da7219->mclk = clk_get(component->dev, "mclk"); + if (IS_ERR(da7219->mclk)) { if (PTR_ERR(da7219->mclk) != -ENOENT) { ret = PTR_ERR(da7219->mclk);
On Mon, Mar 04, 2024 at 01:14:43PM -0800, cujomalainey@chromium.org wrote:
Non-dts based systems can use ACPI DSDT to pass on the mclk to da7219. This enables da7219 mclk to be linked to system clock. Enable/Disable of the mclk is already handled in the codec so platform drivers don't have to explicitly do handling of mclk.
...
- device_property_read_string(dev, "dlg,mclk-name", &pdata->mclk_name);
...
- da7219->mclk = clk_get(component->dev, "mclk");
- if (da7219->pdata->mclk_name)
da7219->mclk = clk_get(NULL, da7219->pdata->mclk_name);- if (!da7219->mclk)
da7219->mclk = clk_get(component->dev, "mclk");
I would never have guessed from the changelog that what this change actually does is provide a mechanism for overriding the name we use to request the MCLK. I had thought this was adding clock handling to a driver that had none. The changelog should say what the change is doing.
Having a firmware property for this is obviously broken for DT systems, this should be limited to ACPI systems if it's going to be there at all. It would be nicer if it were implementeded by having some ACPI specific code link whatever the configured clock name is to "mclk" - I don't know if the clock API has an equivalent to regulator_register_supply_alias() but that's the sort of thing I'm thinking of.
On Mon, Mar 4, 2024 at 1:23 PM Mark Brown broonie@kernel.org wrote:
On Mon, Mar 04, 2024 at 01:14:43PM -0800, cujomalainey@chromium.org wrote:
Non-dts based systems can use ACPI DSDT to pass on the mclk to da7219. This enables da7219 mclk to be linked to system clock. Enable/Disable of the mclk is already handled in the codec so platform drivers don't have to explicitly do handling of mclk.
...
device_property_read_string(dev, "dlg,mclk-name", &pdata->mclk_name);...
da7219->mclk = clk_get(component->dev, "mclk");
if (da7219->pdata->mclk_name)da7219->mclk = clk_get(NULL, da7219->pdata->mclk_name);if (!da7219->mclk)da7219->mclk = clk_get(component->dev, "mclk");I would never have guessed from the changelog that what this change actually does is provide a mechanism for overriding the name we use to request the MCLK. I had thought this was adding clock handling to a driver that had none. The changelog should say what the change is doing.
No problem, I can clean it up, I figured a good starting point would be to just revive the original that was sent in 2018 https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org...
We have been carrying this in our tree the whole time, just caught it and figure I would try and get it up stream again
Having a firmware property for this is obviously broken for DT systems, this should be limited to ACPI systems if it's going to be there at all. It would be nicer if it were implementeded by having some ACPI specific code link whatever the configured clock name is to "mclk" - I don't know if the clock API has an equivalent to regulator_register_supply_alias() but that's the sort of thing I'm thinking of.
I will take a look at this, note that it appears the original author is no longer at AMD as the emails are bouncing.
On Mon, Mar 04, 2024 at 01:50:27PM -0800, Curtis Malainey wrote:
On Mon, Mar 4, 2024 at 1:23 PM Mark Brown broonie@kernel.org wrote:
I would never have guessed from the changelog that what this change actually does is provide a mechanism for overriding the name we use to request the MCLK. I had thought this was adding clock handling to a driver that had none. The changelog should say what the change is doing.
No problem, I can clean it up, I figured a good starting point would be to just revive the original that was sent in 2018 https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org...
That seems to have had 0day find build failures immediately so didn't get looked at :/
On Mon, Mar 4, 2024 at 1:50 PM Curtis Malainey cujomalainey@google.com wrote:
On Mon, Mar 4, 2024 at 1:23 PM Mark Brown broonie@kernel.org wrote:
On Mon, Mar 04, 2024 at 01:14:43PM -0800, cujomalainey@chromium.org wrote:
No problem, I can clean it up, I figured a good starting point would be to just revive the original that was sent in 2018 https://mailman.alsa-project.org/hyperkitty/list/alsa-devel@alsa-project.org...
We have been carrying this in our tree the whole time, just caught it and figure I would try and get it up stream again
So after a bunch of debugging on our 5.10 kernel it actually appears this property is useless and the system is already attaching the mclk under the clock defined in the property. Maybe someone pushed a coreboot update to fix this later which rendered this patch useless?
Having a firmware property for this is obviously broken for DT systems, this should be limited to ACPI systems if it's going to be there at all. It would be nicer if it were implementeded by having some ACPI specific code link whatever the configured clock name is to "mclk" - I don't know if the clock API has an equivalent to regulator_register_supply_alias() but that's the sort of thing I'm thinking of.
I will take a look at this, note that it appears the original author is no longer at AMD as the emails are bouncing.
To answer this for the sake of sharing knowledge, there is a clk_add_alias API.
participants (3)
-
cujomalainey＠chromium.org -
Curtis Malainey -
Mark Brown