diff mbox series

[v4,3/6] ASoC: audio-graph-card: Support setting component plls and sysclks

Message ID 20210108160501.7638-4-rf@opensource.cirrus.com
State New
Headers show
Series Add support for Rpi4b + Cirrus Lochnagar2 and CS47L15 | expand

Commit Message

Richard Fitzgerald Jan. 8, 2021, 4:04 p.m. UTC 
Some codecs need plls and/or sysclks to be configured using the
snd_soc_component_set_[sysclk|pll] functions. These drivers cannot
necessarily be converted to use the clock framework. If the codec is on
a I2C/SPI bus, a nested clk_get would be needed to enable the bus clock.
But the clock framework does not support nested operations and this would
deadlock.

This patch adds new dt properties that list phandles of components with
the pll/sysclk settings to be applied. Multiple settings can be given for
the same phandle to allow for components with multiple clocks and plls.
The plls and sysclks are enabled when the card bias level moves to STANDBY
and disabled when it moves to OFF.

The implementation does not attempt to handle specifying complex clock
ordering interdependencies between components. The plls and sysclks are
applied to a component as it is passed to the card set_bias_level/
set_bias_level_post callbacks. It follows from this that the order
components are configured is the order that they are passed to those
callbacks.

Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
---
 include/sound/simple_card_utils.h     |  25 +++
 sound/soc/generic/audio-graph-card.c  |  13 ++
 sound/soc/generic/simple-card-utils.c | 238 ++++++++++++++++++++++++++
 3 files changed, 276 insertions(+)

Comments

Kuninori Morimoto Jan. 12, 2021, 1:35 a.m. UTC | #1 
Hi Richard

> Some codecs need plls and/or sysclks to be configured using the
> snd_soc_component_set_[sysclk|pll] functions. These drivers cannot
> necessarily be converted to use the clock framework. If the codec is on
> a I2C/SPI bus, a nested clk_get would be needed to enable the bus clock.
> But the clock framework does not support nested operations and this would
> deadlock.
> 
> This patch adds new dt properties that list phandles of components with
> the pll/sysclk settings to be applied. Multiple settings can be given for
> the same phandle to allow for components with multiple clocks and plls.
> The plls and sysclks are enabled when the card bias level moves to STANDBY
> and disabled when it moves to OFF.
> 
> The implementation does not attempt to handle specifying complex clock
> ordering interdependencies between components. The plls and sysclks are
> applied to a component as it is passed to the card set_bias_level/
> set_bias_level_post callbacks. It follows from this that the order
> components are configured is the order that they are passed to those
> callbacks.
> 
> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
> ---

As I mentioned in v3, adding *general* pll to common card driver is
maybe difficult.
Using your own customized audio-graph-card driver is better idea,
instead of adding code to common driver.

I think Sameer's Tegra driver (= [3/6]) is good sample for you ?

	https://lore.kernel.org/r/1606413823-19885-1-git-send-email-spujar@nvidia.com

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Richard Fitzgerald Jan. 12, 2021, 10:22 a.m. UTC | #2 
On 12/01/2021 01:35, Kuninori Morimoto wrote:
> 
> Hi Richard
> 
>> Some codecs need plls and/or sysclks to be configured using the
>> snd_soc_component_set_[sysclk|pll] functions. These drivers cannot
>> necessarily be converted to use the clock framework. If the codec is on
>> a I2C/SPI bus, a nested clk_get would be needed to enable the bus clock.
>> But the clock framework does not support nested operations and this would
>> deadlock.
>>
>> This patch adds new dt properties that list phandles of components with
>> the pll/sysclk settings to be applied. Multiple settings can be given for
>> the same phandle to allow for components with multiple clocks and plls.
>> The plls and sysclks are enabled when the card bias level moves to STANDBY
>> and disabled when it moves to OFF.
>>
>> The implementation does not attempt to handle specifying complex clock
>> ordering interdependencies between components. The plls and sysclks are
>> applied to a component as it is passed to the card set_bias_level/
>> set_bias_level_post callbacks. It follows from this that the order
>> components are configured is the order that they are passed to those
>> callbacks.
>>
>> Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com>
>> ---
> 
> As I mentioned in v3, adding *general* pll to common card driver is
> maybe difficult.

You did say that. But you did not say why.
Can you be more specific about what problem you see with adding it
to the generic driver?

> Using your own customized audio-graph-card driver is better idea,
> instead of adding code to common driver.

I just don't want to duplicate code without good reason.

> 
> I think Sameer's Tegra driver (= [3/6]) is good sample for you ?
> 
> 	https://lore.kernel.org/r/1606413823-19885-1-git-send-email-spujar@nvidia.com
> 
> Thank you for your help !!
> 
> Best regards
> ---
> Kuninori Morimoto
>
Kuninori Morimoto Jan. 13, 2021, midnight UTC | #3 
Hi Richard

> > As I mentioned in v3, adding *general* pll to common card driver is
> > maybe difficult.
> 
> You did say that. But you did not say why.
> Can you be more specific about what problem you see with adding it
> to the generic driver?
> 
> > Using your own customized audio-graph-card driver is better idea,
> > instead of adding code to common driver.
> 
> I just don't want to duplicate code without good reason.

Ahh, sorry for my unclear comment.
I think "PLL settings" is very board/platform specific,
so, adding such code to common driver will be issue in the future.
This is the reason why I don't want add it to audio-graph-card.

But, as I mentioned above and Sameer is already doing,
you can reuse audio-graph-card and customize it.

	Reuse audio-graph-card + Use your own PLL code
	= your own customized audio-graph-card

You can reuse audio-graph-card code by calling graph_parse_of(),
and customize before/after that.
I think no duplicate code is needed.

I hope it can help you.

Thank you for your help !!

Best regards
---
Kuninori Morimoto
Mark Brown Jan. 13, 2021, 3:51 p.m. UTC | #4 
On Wed, Jan 13, 2021 at 09:00:19AM +0900, Kuninori Morimoto wrote:

> Ahh, sorry for my unclear comment.
> I think "PLL settings" is very board/platform specific,
> so, adding such code to common driver will be issue in the future.
> This is the reason why I don't want add it to audio-graph-card.

I don't think it's *that* weird, they're a fairly common feature of
devices and in terms of integration aren't particularly different to
sysclks, though this is for more complex CODECs.
diff mbox series

Patch

diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h
index ba4a3e1897b9..1f755250f9c7 100644
--- a/include/sound/simple_card_utils.h
+++ b/include/sound/simple_card_utils.h
@@ -38,6 +38,16 @@  struct asoc_simple_jack {
 	struct snd_soc_jack_gpio gpio;
 };
 
+struct asoc_simple_sysclk_pll {
+	struct device_node *node;
+	struct clk *clk;
+	int id;
+	int source;
+	unsigned int freq_out;
+	int dir;
+	bool enabled;
+};
+
 struct asoc_simple_priv {
 	struct snd_soc_card snd_card;
 	struct simple_dai_props {
@@ -59,6 +69,11 @@  struct asoc_simple_priv {
 	const struct snd_soc_ops *ops;
 	unsigned int dpcm_selectable:1;
 	unsigned int force_dpcm:1;
+
+	struct asoc_simple_sysclk_pll *sysclks;
+	int num_sysclks;
+	struct asoc_simple_sysclk_pll *plls;
+	int num_plls;
 };
 #define simple_priv_to_card(priv)	(&(priv)->snd_card)
 #define simple_priv_to_props(priv, i)	((priv)->dai_props + (i))
@@ -97,6 +112,14 @@  void asoc_simple_shutdown(struct snd_pcm_substream *substream);
 int asoc_simple_hw_params(struct snd_pcm_substream *substream,
 			  struct snd_pcm_hw_params *params);
 int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd);
+
+int asoc_simple_set_bias_level(struct snd_soc_card *card,
+			       struct snd_soc_dapm_context *dapm,
+			       enum snd_soc_bias_level level);
+int asoc_simple_set_bias_level_post(struct snd_soc_card *card,
+				    struct snd_soc_dapm_context *dapm,
+				    enum snd_soc_bias_level level);
+
 int asoc_simple_be_hw_params_fixup(struct snd_soc_pcm_runtime *rtd,
 				   struct snd_pcm_hw_params *params);
 
@@ -131,6 +154,8 @@  int asoc_simple_parse_widgets(struct snd_soc_card *card,
 				      char *prefix);
 int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
 				   char *prefix);
+int asoc_simple_parse_sysclks(struct asoc_simple_priv *priv, char *prefix);
+int asoc_simple_parse_plls(struct asoc_simple_priv *priv, char *prefix);
 
 int asoc_simple_init_jack(struct snd_soc_card *card,
 			       struct asoc_simple_jack *sjack,
diff --git a/sound/soc/generic/audio-graph-card.c b/sound/soc/generic/audio-graph-card.c
index 16a04a678828..9aa15d5ecbd0 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -592,6 +592,19 @@  int graph_parse_of(struct asoc_simple_priv *priv, struct device *dev)
 
 	snd_soc_card_set_drvdata(card, priv);
 
+	ret = asoc_simple_parse_sysclks(priv, NULL);
+	if (ret < 0)
+		goto err;
+
+	ret = asoc_simple_parse_plls(priv, NULL);
+	if (ret < 0)
+		goto err;
+
+	if (priv->num_sysclks || priv->num_plls) {
+		card->set_bias_level = asoc_simple_set_bias_level;
+		card->set_bias_level_post = asoc_simple_set_bias_level_post;
+	}
+
 	asoc_simple_debug_info(priv);
 
 	ret = devm_snd_soc_register_card(dev, card);
diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c
index 6cada4c1e283..a9b1849e328b 100644
--- a/sound/soc/generic/simple-card-utils.c
+++ b/sound/soc/generic/simple-card-utils.c
@@ -399,6 +399,162 @@  int asoc_simple_dai_init(struct snd_soc_pcm_runtime *rtd)
 }
 EXPORT_SYMBOL_GPL(asoc_simple_dai_init);
 
+static bool asoc_simple_node_is_component(struct snd_soc_component *component,
+					  struct device_node *node)
+{
+	struct device_node *comp_node;
+
+	comp_node = component->dev->of_node;
+	if (!comp_node && component->dev->parent)
+		comp_node = component->dev->parent->of_node;
+
+	return (comp_node == node);
+}
+
+int asoc_simple_set_bias_level(struct snd_soc_card *card,
+			       struct snd_soc_dapm_context *dapm,
+			       enum snd_soc_bias_level level)
+{
+	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_component *component = dapm->component;
+	int i, ret;
+
+	if (!component)
+		return 0;
+
+	switch (level) {
+	case SND_SOC_BIAS_STANDBY:
+		if (dapm->bias_level != SND_SOC_BIAS_OFF)
+			break;
+
+		/*
+		 * A component's PLLs normally supply its SYSCLKs so enable
+		 * the PLLs first.
+		 */
+		for (i = 0; i < priv->num_plls; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->plls[i].node))
+				continue;
+
+			ret = clk_prepare_enable(priv->plls[i].clk);
+			if (ret) {
+				dev_err(card->dev,
+					"(%s) failed to enable pll %d parent clock: %d\n",
+					component->name, priv->plls[i].id, ret);
+				return ret;
+			}
+
+			ret = snd_soc_component_set_pll(component,
+							priv->plls[i].id,
+							priv->plls[i].source,
+							clk_get_rate(priv->plls[i].clk),
+							priv->plls[i].freq_out);
+			if (ret) {
+				dev_err(card->dev, "(%s) failed to set pll %d: %d\n",
+					component->name, priv->plls[i].id, ret);
+
+				clk_disable_unprepare(priv->plls[i].clk);
+				return ret;
+			}
+
+			priv->plls[i].enabled = true;
+		}
+
+		for (i = 0; i < priv->num_sysclks; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->sysclks[i].node))
+				continue;
+
+			ret = clk_prepare_enable(priv->sysclks[i].clk);
+			if (ret) {
+				dev_err(card->dev,
+					"(%s) failed to enable sysclk %d parent clock: %d\n",
+					component->name, priv->sysclks[i].id, ret);
+				return ret;
+			}
+
+			ret = snd_soc_component_set_sysclk(component,
+							   priv->sysclks[i].id,
+							   priv->sysclks[i].source,
+							   clk_get_rate(priv->sysclks[i].clk),
+							   priv->sysclks[i].dir);
+			if (ret) {
+				dev_err(card->dev, "(%s) failed to set sysclk %d: %d\n",
+					component->name, priv->sysclks[i].id, ret);
+
+				clk_disable_unprepare(priv->sysclks[i].clk);
+				return ret;
+			}
+
+			priv->sysclks[i].enabled = true;
+		}
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_set_bias_level);
+
+int asoc_simple_set_bias_level_post(struct snd_soc_card *card,
+				    struct snd_soc_dapm_context *dapm,
+				    enum snd_soc_bias_level level)
+{
+	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card);
+	struct snd_soc_component *component = dapm->component;
+	int i, ret;
+
+	if (!component)
+		goto out;
+
+	switch (level) {
+	case SND_SOC_BIAS_OFF:
+		for (i = 0; i < priv->num_sysclks; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->sysclks[i].node))
+				continue;
+
+			if (!priv->sysclks[i].enabled)
+				continue;
+
+			ret = snd_soc_component_set_sysclk(component,
+							   priv->sysclks[i].id,
+							   0,
+							   0,
+							   priv->sysclks[i].dir);
+			if (ret)
+				dev_warn(card->dev, "(%s) failed to disable sysclk %d: %d\n",
+					 component->name, priv->sysclks[i].id, ret);
+
+			clk_disable_unprepare(priv->sysclks[i].clk);
+			priv->sysclks[i].enabled = false;
+		}
+
+		for (i = 0; i < priv->num_plls; ++i) {
+			if (!asoc_simple_node_is_component(component, priv->plls[i].node))
+				continue;
+
+			if (!priv->plls[i].enabled)
+				continue;
+
+			ret = snd_soc_component_set_pll(component, priv->plls[i].id, 0, 0, 0);
+			if (ret)
+				dev_warn(card->dev, "(%s) failed to disable pll %d: %d\n",
+					 component->name, priv->plls[i].id, ret);
+
+			clk_disable_unprepare(priv->plls[i].clk);
+			priv->plls[i].enabled = false;
+		}
+		break;
+	default:
+		break;
+	}
+
+out:
+	dapm->bias_level = level;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(asoc_simple_set_bias_level_post);
+
 void asoc_simple_canonicalize_platform(struct snd_soc_dai_link *dai_link)
 {
 	/* Assumes platform == cpu */
@@ -433,6 +589,7 @@  EXPORT_SYMBOL_GPL(asoc_simple_canonicalize_cpu);
 
 int asoc_simple_clean_reference(struct snd_soc_card *card)
 {
+	struct asoc_simple_priv *priv = snd_soc_card_get_drvdata(card);
 	struct snd_soc_dai_link *dai_link;
 	int i;
 
@@ -440,6 +597,15 @@  int asoc_simple_clean_reference(struct snd_soc_card *card)
 		of_node_put(dai_link->cpus->of_node);
 		of_node_put(dai_link->codecs->of_node);
 	}
+
+	if (priv) {
+		for (i = 0; i < priv->num_sysclks; ++i)
+			of_node_put(priv->sysclks[i].node);
+
+		for (i = 0; i < priv->num_plls; ++i)
+			of_node_put(priv->plls[i].node);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(asoc_simple_clean_reference);
@@ -538,6 +704,78 @@  int asoc_simple_parse_pin_switches(struct snd_soc_card *card,
 }
 EXPORT_SYMBOL_GPL(asoc_simple_parse_pin_switches);
 
+static int asoc_simple_parse_sysclks_plls(struct asoc_simple_priv *priv,
+					  char *prefix,
+					  char *prop_root_name,
+					  struct asoc_simple_sysclk_pll **out,
+					  int *out_count)
+{
+	struct device *dev = priv->snd_card.dev;
+	struct device_node *node = dev->of_node;
+	struct of_phandle_args args;
+	int n_elem, i, ret;
+	char prop[128];
+	char clocks_prop[128];
+	const char *src_clk_name;
+	struct clk *in_clk;
+
+	if (!prefix)
+		prefix = "";
+
+	snprintf(prop, sizeof(prop), "%s%s", prefix, prop_root_name);
+	snprintf(clocks_prop, sizeof(clocks_prop), "%s-clocks", prop);
+	n_elem = of_count_phandle_with_fixed_args(node, prop, 3);
+	if (n_elem == -ENOENT || n_elem == 0) {
+		return 0;
+	} else if (n_elem < 0) {
+		dev_err(dev, "Failed to parse %s: %d\n", prop, n_elem);
+		return n_elem;
+	}
+
+	*out = devm_kcalloc(dev, n_elem, sizeof(**out), GFP_KERNEL);
+	if (!*out)
+		return -ENOMEM;
+	*out_count = 0;
+
+	for (i = 0; i < n_elem; ++i) {
+		ret = of_property_read_string_index(node, clocks_prop, i, &src_clk_name);
+		if (ret < 0)
+			return ret;
+
+		in_clk = devm_clk_get(dev, src_clk_name);
+		if (IS_ERR(in_clk))
+			return PTR_ERR(in_clk);
+
+		ret = of_parse_phandle_with_fixed_args(node, prop, 3, i, &args);
+		if (ret < 0)
+			return ret;
+
+		(*out)[i].node		= args.np;
+		(*out)[i].clk		= in_clk;
+		(*out)[i].id		= args.args[0];
+		(*out)[i].source	= args.args[1];
+		(*out)[i].dir		= args.args[2]; /* for sysclks */
+		(*out)[i].freq_out	= args.args[2]; /* for plls */
+		++*out_count;
+	}
+
+	return 0;
+}
+
+int asoc_simple_parse_sysclks(struct asoc_simple_priv *priv, char *prefix)
+{
+	return asoc_simple_parse_sysclks_plls(priv, prefix, "sysclks",
+					      &priv->sysclks, &priv->num_sysclks);
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_sysclks);
+
+int asoc_simple_parse_plls(struct asoc_simple_priv *priv, char *prefix)
+{
+	return asoc_simple_parse_sysclks_plls(priv, prefix, "plls",
+					      &priv->plls, &priv->num_plls);
+}
+EXPORT_SYMBOL_GPL(asoc_simple_parse_plls);
+
 int asoc_simple_init_jack(struct snd_soc_card *card,
 			  struct asoc_simple_jack *sjack,
 			  int is_hp, char *prefix,