| Message ID | 20180528193503.18905-3-daniel@zonque.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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; }
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(-)