[alsa-devel] [PATCH 0/3] ASoC: make simple-card a bit more versatile
In the process of porting some boards to DT, I noticed the simple-card driver is already capable of handling most cases just fine. The missing bits I've spotted are:
* The clock that is passed in is only read, never set, which is not sufficient for boards that support several sampling rates with different base frequencies
* The sysclk id that is passed to snd_soc_dai_set_sysclk() is hard-coded
* There's currently no support for calling into snd_soc_dai_set_clkdiv()
The following three patches fix all those and make simple-card a good match for the board I'm working with.
Thanks, Daniel
Daniel Mack (3): ASoC: simple-card: set cpu dai clk in hw_params ASoC: simple-card: make sysclk index configurable ASoC: simple-card: add support for clock divider setup
.../devicetree/bindings/sound/simple-card.txt | 13 ++++++ include/sound/simple_card_utils.h | 19 ++++++++ sound/soc/generic/simple-card-utils.c | 50 ++++++++++++++++++++++ sound/soc/generic/simple-card.c | 49 +++++++++++++++++++-- 4 files changed, 127 insertions(+), 4 deletions(-)
The simple-card driver currently accepts a clock node in the cpu dai sub-node and only uses it as an alternative to the 'system-clock-frequency' property to get the current frequency.
This patch adds another use of the passed clock node. If mclk-fs is specified, the clock will be set to the calculated rate (stream rate * mclk_fs) in hw_params. This allows platforms to pass a tuneable clock as phandle that will automatically be set to the right rates.
Signed-off-by: Daniel Mack daniel@zonque.org --- Documentation/devicetree/bindings/sound/simple-card.txt | 5 +++++ sound/soc/generic/simple-card.c | 4 ++++ 2 files changed, 9 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index 17c13e74667d..a4c72d09cd45 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -86,6 +86,11 @@ Optional CPU/CODEC subnodes properties: in dai startup() and disabled with clk_disable_unprepare() in dai shutdown(). + If a clock is specified and a + multiplication factor is given with + mclk-fs, the clock will be set to the + calculated mclk frequency when the + stream starts. - system-clock-direction-out : specifies clock direction as 'out' on initialization. It is useful for some aCPUs with fixed clocks. diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 6959a74a6f49..ca529a6cab06 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -154,6 +154,10 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream,
if (mclk_fs) { mclk = params_rate(params) * mclk_fs; + + if (dai_props->cpu_dai.clk) + clk_set_rate(dai_props->cpu_dai.clk, mclk); + ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, SND_SOC_CLOCK_IN); if (ret && ret != -ENOTSUPP)
Hi Daniel
The simple-card driver currently accepts a clock node in the cpu dai sub-node and only uses it as an alternative to the 'system-clock-frequency' property to get the current frequency.
This patch adds another use of the passed clock node. If mclk-fs is specified, the clock will be set to the calculated rate (stream rate * mclk_fs) in hw_params. This allows platforms to pass a tuneable clock as phandle that will automatically be set to the right rates.
Signed-off-by: Daniel Mack daniel@zonque.org
(snip)
if (mclk_fs) { mclk = params_rate(params) * mclk_fs;
if (dai_props->cpu_dai.clk)
clk_set_rate(dai_props->cpu_dai.clk, mclk);
- ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, SND_SOC_CLOCK_IN); if (ret && ret != -ENOTSUPP)
Having codec is nice balance ?
Best regards --- Kuninori Morimoto
On Tuesday, May 29, 2018 03:38 AM, Kuninori Morimoto wrote:
The simple-card driver currently accepts a clock node in the cpu dai sub-node and only uses it as an alternative to the 'system-clock-frequency' property to get the current frequency.
This patch adds another use of the passed clock node. If mclk-fs is specified, the clock will be set to the calculated rate (stream rate * mclk_fs) in hw_params. This allows platforms to pass a tuneable clock as phandle that will automatically be set to the right rates.
Signed-off-by: Daniel Mack daniel@zonque.org
(snip)
if (mclk_fs) { mclk = params_rate(params) * mclk_fs;
if (dai_props->cpu_dai.clk)
clk_set_rate(dai_props->cpu_dai.clk, mclk);
- ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, SND_SOC_CLOCK_IN); if (ret && ret != -ENOTSUPP)
Having codec is nice balance ?
Ah, yes, why not. Will post a v2.
Thanks, Daniel
On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote:
if (mclk_fs) { mclk = params_rate(params) * mclk_fs;
if (dai_props->cpu_dai.clk)
clk_set_rate(dai_props->cpu_dai.clk, mclk);
We're ignoring the return value here.
On Tuesday, May 29, 2018 01:16 PM, Mark Brown wrote:
On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote:
if (mclk_fs) { mclk = params_rate(params) * mclk_fs;
if (dai_props->cpu_dai.clk)
clk_set_rate(dai_props->cpu_dai.clk, mclk);
We're ignoring the return value here.
On purpose actually. Not all clocks might be settable, and in that case, this is a no-op. You think we should bail or warn?
On Tue, May 29, 2018 at 01:17:45PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:16 PM, Mark Brown wrote:
On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote:
if (dai_props->cpu_dai.clk)
clk_set_rate(dai_props->cpu_dai.clk, mclk);
We're ignoring the return value here.
On purpose actually. Not all clocks might be settable, and in that case, this is a no-op. You think we should bail or warn?
If we need to set the rate and fail to set it then clearly we shouldn't just carry on ignoring the error. You might want some more involved logic there around checking if it's actually a rate change before you error out, and possibly some logic to carry on with whatever the rate is and a reduced set of resulting sample rates.
On Tuesday, May 29, 2018 01:32 PM, Mark Brown wrote:
On Tue, May 29, 2018 at 01:17:45PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:16 PM, Mark Brown wrote:
On Mon, May 28, 2018 at 09:35:01PM +0200, Daniel Mack wrote:
if (dai_props->cpu_dai.clk)
clk_set_rate(dai_props->cpu_dai.clk, mclk);
We're ignoring the return value here.
On purpose actually. Not all clocks might be settable, and in that case, this is a no-op. You think we should bail or warn?
If we need to set the rate and fail to set it then clearly we shouldn't just carry on ignoring the error. You might want some more involved logic there around checking if it's actually a rate change before you error out, and possibly some logic to carry on with whatever the rate is and a reduced set of resulting sample rates.
Fair enough. I'll add that error checking at least, that makes sense.
Thanks for the feedback! Daniel
The simple-card driver currently hard-codes the clk_id parameter in snd_soc_dai_set_sysclk() to 0. Make this configrable for both CPU and codec dai sub-nodes.
This still has the limitation that only one clk_id can be configured, but it should help some more platforms to use simple-card in favor to a more specific machine driver.
Signed-off-by: Daniel Mack daniel@zonque.org --- Documentation/devicetree/bindings/sound/simple-card.txt | 3 +++ include/sound/simple_card_utils.h | 1 + sound/soc/generic/simple-card-utils.c | 3 +++ sound/soc/generic/simple-card.c | 10 ++++++---- 4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index a4c72d09cd45..c8d268285a9e 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -94,6 +94,9 @@ Optional CPU/CODEC subnodes properties: - system-clock-direction-out : specifies clock direction as 'out' on initialization. It is useful for some aCPUs with fixed clocks. +- system-clock-index : index of the system clock to use when + the mclk frequency is on the CPU/CODEC + DAI. Defaults to 0.
Example 1 - single DAI link:
diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index 7e25afce6566..ebdf52c9884f 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -21,6 +21,7 @@ struct asoc_simple_dai { unsigned int tx_slot_mask; unsigned int rx_slot_mask; struct clk *clk; + int sysclk_id; };
struct asoc_simple_card_data { diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 3751a07de6aa..493c9b1f057e 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -199,6 +199,9 @@ int asoc_simple_card_parse_clk(struct device *dev, if (of_property_read_bool(node, "system-clock-direction-out")) simple_dai->clk_direction = SND_SOC_CLOCK_OUT;
+ if (!of_property_read_u32(node, "system-clock-index", &val)) + simple_dai->sysclk_id = val; + dev_dbg(dev, "%s : sysclk = %d, direction %d\n", name, simple_dai->sysclk, simple_dai->clk_direction);
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index ca529a6cab06..5b4afa624395 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -158,13 +158,15 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream, if (dai_props->cpu_dai.clk) clk_set_rate(dai_props->cpu_dai.clk, mclk);
- ret = snd_soc_dai_set_sysclk(codec_dai, 0, mclk, - SND_SOC_CLOCK_IN); + ret = snd_soc_dai_set_sysclk(codec_dai, + dai_props->codec_dai.sysclk_id, + mclk, SND_SOC_CLOCK_IN); if (ret && ret != -ENOTSUPP) goto err;
- ret = snd_soc_dai_set_sysclk(cpu_dai, 0, mclk, - SND_SOC_CLOCK_OUT); + ret = snd_soc_dai_set_sysclk(cpu_dai, + dai_props->cpu_dai.sysclk_id, + mclk, SND_SOC_CLOCK_OUT); if (ret && ret != -ENOTSUPP) goto err; }
Hi Daniel
The simple-card driver currently hard-codes the clk_id parameter in snd_soc_dai_set_sysclk() to 0. Make this configrable for both CPU and codec dai sub-nodes.
This still has the limitation that only one clk_id can be configured, but it should help some more platforms to use simple-card in favor to a more specific machine driver.
Signed-off-by: Daniel Mack daniel@zonque.org
Documentation/devicetree/bindings/sound/simple-card.txt | 3 +++ include/sound/simple_card_utils.h | 1 + sound/soc/generic/simple-card-utils.c | 3 +++ sound/soc/generic/simple-card.c | 10 ++++++---- 4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index a4c72d09cd45..c8d268285a9e 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -94,6 +94,9 @@ Optional CPU/CODEC subnodes properties:
- system-clock-direction-out : specifies clock direction as 'out' on initialization. It is useful for some aCPUs with fixed clocks.
+- system-clock-index : index of the system clock to use when
the mclk frequency is on the CPU/CODEC
DAI. Defaults to 0.
I'm not a DT guy, but I think DT doesn't want to have index directly ? I don't know detail, but I guess DT want to have like
system-mclock = <&xxxx 3>
Best regards --- Kuninori Morimoto
On Tuesday, May 29, 2018 03:35 AM, Kuninori Morimoto wrote:
The simple-card driver currently hard-codes the clk_id parameter in snd_soc_dai_set_sysclk() to 0. Make this configrable for both CPU and codec dai sub-nodes.
This still has the limitation that only one clk_id can be configured, but it should help some more platforms to use simple-card in favor to a more specific machine driver.
Signed-off-by: Daniel Mack daniel@zonque.org
Documentation/devicetree/bindings/sound/simple-card.txt | 3 +++ include/sound/simple_card_utils.h | 1 + sound/soc/generic/simple-card-utils.c | 3 +++ sound/soc/generic/simple-card.c | 10 ++++++---- 4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index a4c72d09cd45..c8d268285a9e 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -94,6 +94,9 @@ Optional CPU/CODEC subnodes properties:
- system-clock-direction-out : specifies clock direction as 'out' on initialization. It is useful for some aCPUs with fixed clocks.
+- system-clock-index : index of the system clock to use when
the mclk frequency is on the CPU/CODEC
DAI. Defaults to 0.
I'm not a DT guy, but I think DT doesn't want to have index directly ? I don't know detail, but I guess DT want to have like
system-mclock = <&xxxx 3>
Hmm, no. That index doesn't describe a particular output of a clock phandle but an internal detail of the CPU or CODEC DAI on the other end. Most DAIs will use 0 here, like your code had it before.
The DAI can be both a producer and a consumer of a clock, depending on the audio clocking setup, and these details are not exposed in DT.
Thanks, Daniel
On Mon, May 28, 2018 at 09:35:02PM +0200, Daniel Mack wrote:
The simple-card driver currently hard-codes the clk_id parameter in snd_soc_dai_set_sysclk() to 0. Make this configrable for both CPU and codec dai sub-nodes.
This still has the limitation that only one clk_id can be configured, but it should help some more platforms to use simple-card in favor to a more specific machine driver.
If we want to get more complex usage of clocks in the DT we should be moving the CODECs over to using the standard clock bindings for this stuff rather than inventing custom ASoC clock bindings for it. That way we don't have to deal with the pain of trying to join things up in the future.
As a practical matter we also don't have any CODECs specifying any bindings for ASoC level clock IDs right now either.
On Tuesday, May 29, 2018 01:24 PM, Mark Brown wrote:
On Mon, May 28, 2018 at 09:35:02PM +0200, Daniel Mack wrote:
The simple-card driver currently hard-codes the clk_id parameter in snd_soc_dai_set_sysclk() to 0. Make this configrable for both CPU and codec dai sub-nodes.
This still has the limitation that only one clk_id can be configured, but it should help some more platforms to use simple-card in favor to a more specific machine driver.
If we want to get more complex usage of clocks in the DT we should be moving the CODECs over to using the standard clock bindings for this stuff rather than inventing custom ASoC clock bindings for it. That way we don't have to deal with the pain of trying to join things up in the future.
This will get rather complex too though, because most codec and cpu dais can act as clock source xor clock consumer, depending on how the hardware is built. Would you want to represent everything, bit clocks, frame clocks, master clocks etc as clock nodes?
If that's the case, could you depict how the DT bindings should look like by example?
Thanks, Daniel
On Tue, May 29, 2018 at 10:23:48PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:24 PM, Mark Brown wrote:
If we want to get more complex usage of clocks in the DT we should be moving the CODECs over to using the standard clock bindings for this stuff rather than inventing custom ASoC clock bindings for it. That way we don't have to deal with the pain of trying to join things up in the future.
This will get rather complex too though, because most codec and cpu dais can act as clock source xor clock consumer, depending on how the hardware is built. Would you want to represent everything, bit clocks, frame clocks, master clocks etc as clock nodes?
We're probably OK just going down to the master clocks mostly I think. For the clocks on the actual bus we can probably have helpers in the core that generate the clocks and the drivers just tell the core the rates.
If that's the case, could you depict how the DT bindings should look like by example?
I've not been able to dig far enough into the clock bindings yet. That's not super helpful for you I appreciate, TBH nobody seemed to particularly need this so it wasn't super urgent - most of the things using alternative clocks on devices seem to also need custom machine drivers for other reasons.
On Tue, May 29, 2018 at 10:23:48PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:24 PM, Mark Brown wrote:
On Mon, May 28, 2018 at 09:35:02PM +0200, Daniel Mack wrote:
The simple-card driver currently hard-codes the clk_id parameter in snd_soc_dai_set_sysclk() to 0. Make this configrable for both CPU and codec dai sub-nodes.
This still has the limitation that only one clk_id can be configured, but it should help some more platforms to use simple-card in favor to a more specific machine driver.
If we want to get more complex usage of clocks in the DT we should be moving the CODECs over to using the standard clock bindings for this stuff rather than inventing custom ASoC clock bindings for it. That way we don't have to deal with the pain of trying to join things up in the future.
This will get rather complex too though, because most codec and cpu dais can act as clock source xor clock consumer, depending on how the hardware is built. Would you want to represent everything, bit clocks, frame clocks, master clocks etc as clock nodes?
I have no idea if you need all the clocks or not, but they certainly shouldn't be a node per clock. Rather the codec and/or cpu dai should be a clock provider and can provide N clocks.
If that's the case, could you depict how the DT bindings should look like by example?
In clock providers, you just add #clock-cells. Then the consumer side defines 'clocks'. Which clock(s) comes from the dai is defined by the index in the 'clocks' property for that node.
Both ends can be providers, but who is active is determined by defining the actual connections with 'clocks'. Or perhaps you have both directions shown and there is some other means to select which one is active such as solving for who can provide the desired freq.
Hope that helps.
Rob
On Thursday, May 31, 2018 07:02 PM, Rob Herring wrote:
On Tue, May 29, 2018 at 10:23:48PM +0200, Daniel Mack wrote:
If that's the case, could you depict how the DT bindings should look like by example?
In clock providers, you just add #clock-cells. Then the consumer side defines 'clocks'. Which clock(s) comes from the dai is defined by the index in the 'clocks' property for that node.
Both ends can be providers, but who is active is determined by defining the actual connections with 'clocks'. Or perhaps you have both directions shown and there is some other means to select which one is active such as solving for who can provide the desired freq.
How about entities that are a clock producer and consume their clocks themselves? That's a rather typical thing for DAIs. Is that expressible in DT?
Thanks, Daniel
On Thu, May 31, 2018 at 3:03 PM, Daniel Mack daniel@zonque.org wrote:
On Thursday, May 31, 2018 07:02 PM, Rob Herring wrote:
On Tue, May 29, 2018 at 10:23:48PM +0200, Daniel Mack wrote:
If that's the case, could you depict how the DT bindings should look like by example?
In clock providers, you just add #clock-cells. Then the consumer side defines 'clocks'. Which clock(s) comes from the dai is defined by the index in the 'clocks' property for that node.
Both ends can be providers, but who is active is determined by defining the actual connections with 'clocks'. Or perhaps you have both directions shown and there is some other means to select which one is active such as solving for who can provide the desired freq.
How about entities that are a clock producer and consume their clocks themselves? That's a rather typical thing for DAIs. Is that expressible in DT?
And I assume something else consumes those clocks, too? If not, you can just handle all that within the driver and don't need the clock binding or clock framework.
But yes, you can do something like this:
myclk: clock-controller { #clock-cells = <1>; clocks = <&myclk 123>, <&otherclk 1>; };
You'll have to handle clk registration and clk_get in the right order to avoid deferring probe on yourself.
Rob
Add support to call into snd_soc_dai_set_clkdiv() for both CPU and codec DAIs from simple-card's hw_params().
This allows platforms to set hardware-specific values without providing an own machine driver.
Signed-off-by: Daniel Mack daniel@zonque.org --- .../devicetree/bindings/sound/simple-card.txt | 5 +++ include/sound/simple_card_utils.h | 18 +++++++++ sound/soc/generic/simple-card-utils.c | 47 ++++++++++++++++++++++ sound/soc/generic/simple-card.c | 35 ++++++++++++++++ 4 files changed, 105 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/simple-card.txt b/Documentation/devicetree/bindings/sound/simple-card.txt index c8d268285a9e..d3d83256ab5d 100644 --- a/Documentation/devicetree/bindings/sound/simple-card.txt +++ b/Documentation/devicetree/bindings/sound/simple-card.txt @@ -97,6 +97,10 @@ Optional CPU/CODEC subnodes properties: - system-clock-index : index of the system clock to use when the mclk frequency is on the CPU/CODEC DAI. Defaults to 0. +- clock-dividers : Array of index/value pairs for CPU/CODEC + specific clock dividers to configure at + stream start time. Optional. No divider + is configured if not specified.
Example 1 - single DAI link:
@@ -152,6 +156,7 @@ sound { format = "i2s"; cpu { sound-dai = <&audio1 0>; + clock-dividers = <0 4>; }; codec { sound-dai = <&tda998x 0>; diff --git a/include/sound/simple_card_utils.h b/include/sound/simple_card_utils.h index ebdf52c9884f..ed3180f975af 100644 --- a/include/sound/simple_card_utils.h +++ b/include/sound/simple_card_utils.h @@ -12,6 +12,11 @@
#include <sound/soc.h>
+struct asoc_simple_clkdiv { + int div_id; + int div; +}; + struct asoc_simple_dai { const char *name; unsigned int sysclk; @@ -22,6 +27,8 @@ struct asoc_simple_dai { unsigned int rx_slot_mask; struct clk *clk; int sysclk_id; + struct asoc_simple_clkdiv *clkdiv; + int num_clkdiv; };
struct asoc_simple_card_data { @@ -55,6 +62,17 @@ int asoc_simple_card_parse_clk(struct device *dev, int asoc_simple_card_clk_enable(struct asoc_simple_dai *dai); void asoc_simple_card_clk_disable(struct asoc_simple_dai *dai);
+#define asoc_simple_card_parse_clkdiv_cpu(dev, node, dai_link, simple_dai) \ + asoc_simple_card_parse_clkdiv(dev, node, simple_dai, \ + dai_link->cpu_dai_name) +#define asoc_simple_card_parse_clkdiv_codec(dev, node, dai_link, simple_dai) \ + asoc_simple_card_parse_clkdiv(dev, node, simple_dai, \ + dai_link->codec_dai_name) +int asoc_simple_card_parse_clkdiv(struct device *dev, + struct device_node *node, + struct asoc_simple_dai *simple_dai, + const char *name); + #define asoc_simple_card_parse_cpu(node, dai_link, \ list_name, cells_name, is_single_link) \ asoc_simple_card_parse_dai(node, &dai_link->cpu_of_node, \ diff --git a/sound/soc/generic/simple-card-utils.c b/sound/soc/generic/simple-card-utils.c index 493c9b1f057e..ad7912871240 100644 --- a/sound/soc/generic/simple-card-utils.c +++ b/sound/soc/generic/simple-card-utils.c @@ -209,6 +209,53 @@ int asoc_simple_card_parse_clk(struct device *dev, } EXPORT_SYMBOL_GPL(asoc_simple_card_parse_clk);
+int asoc_simple_card_parse_clkdiv(struct device *dev, + struct device_node *node, + struct asoc_simple_dai *simple_dai, + const char *name) +{ + const char *pname = "clock-dividers"; + int i, ret, count; + u32 val; + + count = of_property_count_u32_elems(node, pname); + if (count <= 0) + return 0; + + if (count & 1) { + dev_err(dev, "Invalid number of values for %s property", pname); + return -EINVAL; + } + + simple_dai->num_clkdiv = count / 2; + simple_dai->clkdiv = devm_kcalloc(dev, simple_dai->num_clkdiv, + sizeof(struct asoc_simple_clkdiv), + GFP_KERNEL); + if (!simple_dai->clkdiv) + return -ENOMEM; + + for (i = 0; i < simple_dai->num_clkdiv; i++) { + ret = of_property_read_u32_index(node, pname, i * 2, &val); + if (ret < 0) + return ret; + + simple_dai->clkdiv[i].div_id = val; + + ret = of_property_read_u32_index(node, pname, i * 2 + 1, &val); + if (ret < 0) + return ret; + + simple_dai->clkdiv[i].div = val; + + dev_dbg(dev, "%s : clkdiv #%d = %d\n", name, + simple_dai->clkdiv[i].div_id, + simple_dai->clkdiv[i].div); + } + + return 0; +} +EXPORT_SYMBOL_GPL(asoc_simple_card_parse_clkdiv); + int asoc_simple_card_parse_dai(struct device_node *node, struct device_node **dai_of_node, const char **dai_name, diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c index 5b4afa624395..56d1d774feac 100644 --- a/sound/soc/generic/simple-card.c +++ b/sound/soc/generic/simple-card.c @@ -135,6 +135,23 @@ static void asoc_simple_card_shutdown(struct snd_pcm_substream *substream) asoc_simple_card_clk_disable(&dai_props->codec_dai); }
+static int asoc_simple_card_set_clkdiv(struct snd_soc_dai *dai, + unsigned int mclk, + const struct asoc_simple_dai *simple_dai) +{ + int ret, i; + + for (i = 0; i < simple_dai->num_clkdiv; i++) { + ret = snd_soc_dai_set_clkdiv(dai, + simple_dai->clkdiv[i].div_id, + simple_dai->clkdiv[i].div); + if (ret < 0) + return ret; + } + + return 0; +} + static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -164,11 +181,21 @@ static int asoc_simple_card_hw_params(struct snd_pcm_substream *substream, if (ret && ret != -ENOTSUPP) goto err;
+ ret = asoc_simple_card_set_clkdiv(codec_dai, mclk, + &dai_props->codec_dai); + if (ret && ret != -ENOTSUPP) + goto err; + ret = snd_soc_dai_set_sysclk(cpu_dai, dai_props->cpu_dai.sysclk_id, mclk, SND_SOC_CLOCK_OUT); if (ret && ret != -ENOTSUPP) goto err; + + ret = asoc_simple_card_set_clkdiv(cpu_dai, mclk, + &dai_props->cpu_dai); + if (ret && ret != -ENOTSUPP) + goto err; } return 0; err: @@ -287,6 +314,14 @@ static int asoc_simple_card_dai_link_of(struct device_node *node, if (ret < 0) goto dai_link_of_err;
+ ret = asoc_simple_card_parse_clkdiv_cpu(dev, cpu, dai_link, cpu_dai); + if (ret < 0) + goto dai_link_of_err; + + ret = asoc_simple_card_parse_clkdiv_codec(dev, codec, dai_link, codec_dai); + if (ret < 0) + goto dai_link_of_err; + ret = asoc_simple_card_canonicalize_dailink(dai_link); if (ret < 0) goto dai_link_of_err;
Hi
Add support to call into snd_soc_dai_set_clkdiv() for both CPU and codec DAIs from simple-card's hw_params().
This allows platforms to set hardware-specific values without providing an own machine driver.
Signed-off-by: Daniel Mack daniel@zonque.org
Acked-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Best regards --- Kuninori Morimoto
On Mon, May 28, 2018 at 09:35:03PM +0200, Daniel Mack wrote:
Add support to call into snd_soc_dai_set_clkdiv() for both CPU and codec DAIs from simple-card's hw_params().
This allows platforms to set hardware-specific values without providing an own machine driver.
Why do we have anything that actually needs this? The devices with a set_clkdiv() operation are pretty legacy at this point and most of them should just be figuring out the dividers for themselves anyway.
On Tuesday, May 29, 2018 01:35 PM, Mark Brown wrote:
On Mon, May 28, 2018 at 09:35:03PM +0200, Daniel Mack wrote:
Add support to call into snd_soc_dai_set_clkdiv() for both CPU and codec DAIs from simple-card's hw_params().
This allows platforms to set hardware-specific values without providing an own machine driver.
Why do we have anything that actually needs this? The devices with a set_clkdiv() operation are pretty legacy at this point and most of them should just be figuring out the dividers for themselves anyway.
The PXA platform CPU DAI still has it, and grepping also reveals some Freescale and Samsung platforms.
If that's considered legacy, we shouldn't add bindings for it of course. I can look into the PXA driver and see if it can auto-detect the dividers itself. It won't be straight forward though as the clocking setup also depends on configurations such as TDM slots, external vs. internal clocks etc. Meh.
Thanks, Daniel
On Tue, May 29, 2018 at 10:29:10PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:35 PM, Mark Brown wrote:
Why do we have anything that actually needs this? The devices with a set_clkdiv() operation are pretty legacy at this point and most of them should just be figuring out the dividers for themselves anyway.
The PXA platform CPU DAI still has it, and grepping also reveals some Freescale and Samsung platforms.
PXA is definitely very legacy, as are some of the Samsung and Freescale ones - PXA in particular hasn't had much love in years.
If that's considered legacy, we shouldn't add bindings for it of course. I can look into the PXA driver and see if it can auto-detect the dividers itself. It won't be straight forward though as the clocking setup also depends on configurations such as TDM slots, external vs. internal clocks etc. Meh.
It's usually relatively straightforward - something along the lines of work out what your BCLK needs to be and everything else flows from that.
On Wednesday, May 30, 2018 11:10 AM, Mark Brown wrote:
On Tue, May 29, 2018 at 10:29:10PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:35 PM, Mark Brown wrote:
Why do we have anything that actually needs this? The devices with a set_clkdiv() operation are pretty legacy at this point and most of them should just be figuring out the dividers for themselves anyway.
The PXA platform CPU DAI still has it, and grepping also reveals some Freescale and Samsung platforms.
PXA is definitely very legacy, as are some of the Samsung and Freescale ones - PXA in particular hasn't had much love in years.
Yes, I'm currently preparing a larger series of cleanups that I'll send for 4.18. I hope I can also tackle the clkdiv/pll things with that.
Thanks, Daniel
On Wednesday, May 30, 2018 11:12 AM, Daniel Mack wrote:
On Wednesday, May 30, 2018 11:10 AM, Mark Brown wrote:
On Tue, May 29, 2018 at 10:29:10PM +0200, Daniel Mack wrote:
On Tuesday, May 29, 2018 01:35 PM, Mark Brown wrote:
Why do we have anything that actually needs this? The devices with a set_clkdiv() operation are pretty legacy at this point and most of them should just be figuring out the dividers for themselves anyway.
The PXA platform CPU DAI still has it, and grepping also reveals some Freescale and Samsung platforms.
PXA is definitely very legacy, as are some of the Samsung and Freescale ones - PXA in particular hasn't had much love in years.
Yes, I'm currently preparing a larger series of cleanups that I'll send for 4.18. I hope I can also tackle the clkdiv/pll things with that.
I'd like to get some of this done for 4.19, but for that, I'd need Robert's patch that addresses DMA compat issues in your tree.
Not sure if you've followed the thread titled '[PATCH v3 09/14] ASoC: pxa: remove the dmaengine compat need' - could you chime in there?
Thanks, Daniel
On Sat, Jun 23, 2018 at 08:41:27PM +0200, Daniel Mack wrote:
I'd like to get some of this done for 4.19, but for that, I'd need Robert's patch that addresses DMA compat issues in your tree.
Not sure if you've followed the thread titled '[PATCH v3 09/14] ASoC: pxa: remove the dmaengine compat need' - could you chime in there?
No, I reviewed my patch in there. What's going on?
On Monday, June 25, 2018 01:24 PM, Mark Brown wrote:
On Sat, Jun 23, 2018 at 08:41:27PM +0200, Daniel Mack wrote:
I'd like to get some of this done for 4.19, but for that, I'd need Robert's patch that addresses DMA compat issues in your tree.
Not sure if you've followed the thread titled '[PATCH v3 09/14] ASoC: pxa: remove the dmaengine compat need' - could you chime in there?
No, I reviewed my patch in there. What's going on?
I'd like this patch to go through your tree, if that's okay. Then I can post some more patches on top without causing a conflict in the merge window.
Robert is fine with that.
Thanks, Daniel
On Mon, Jun 25, 2018 at 02:55:12PM +0200, Daniel Mack wrote:
On Monday, June 25, 2018 01:24 PM, Mark Brown wrote:
No, I reviewed my patch in there. What's going on?
I'd like this patch to go through your tree, if that's okay. Then I can post some more patches on top without causing a conflict in the merge window.
Robert is fine with that.
That's fine by me. Is the latest version v3?
On Monday, June 25, 2018 02:57 PM, Mark Brown wrote:
On Mon, Jun 25, 2018 at 02:55:12PM +0200, Daniel Mack wrote:
On Monday, June 25, 2018 01:24 PM, Mark Brown wrote:
No, I reviewed my patch in there. What's going on?
I'd like this patch to go through your tree, if that's okay. Then I can post some more patches on top without causing a conflict in the merge window.
Robert is fine with that.
That's fine by me. Is the latest version v3?
Yes.
Thanks!
participants (4)
-
Daniel Mack
-
Kuninori Morimoto
-
Mark Brown
-
Rob Herring