| Message ID | 1513270438-18523-3-git-send-email-olivier.moysan@st.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Dec 14, 2017 at 05:53:58PM +0100, Olivier Moysan wrote: > When defined in device tree, MCLK1 and MCLK2 are used > as sysclk for aif1 and aif2 interfaces respectively. That's not a valid assumption as far as I remember? The AIFs can use either MCLK depending on the system configuration I think. > If clock rate is let 0, the frequency provided by > wm8994_set_dai_sysclk() is used instead. I'd expect this the other way around, if we didn't specify a frequency then read it from the input otherwise try to use clk_set_rate() to propagate things up.
On Thu, Dec 14, 2017 at 05:36:24PM +0000, Mark Brown wrote: > On Thu, Dec 14, 2017 at 05:53:58PM +0100, Olivier Moysan wrote: > > When defined in device tree, MCLK1 and MCLK2 are used > > as sysclk for aif1 and aif2 interfaces respectively. > > That's not a valid assumption as far as I remember? The AIFs can use > either MCLK depending on the system configuration I think. > Indeed yes the code added here is literally just above the code that selects that, it should be fairly simple to update the patch to take this into account. Thanks, Charles
Hello Mark, Thanks for your comment. On 12/14/2017 06:36 PM, Mark Brown wrote: > On Thu, Dec 14, 2017 at 05:53:58PM +0100, Olivier Moysan wrote: >> When defined in device tree, MCLK1 and MCLK2 are used >> as sysclk for aif1 and aif2 interfaces respectively. > > That's not a valid assumption as far as I remember? The AIFs can use > either MCLK depending on the system configuration I think. > You are right. wm8994 allows to select either MCLK for each AIF. From this point of view, the current patch is too limiting. MCLK information in DT allows to enforce MCLK use, but an additionnal information is required to determine AIF MCLK assignment. Available properties in codec DAI node, such as clocks property, cannot help here. Maybe a DAPM linked to a control is a better way to select AIF source, When source is not provided by clk_id in wm8994_set_dai_sysclk(). In this case, wm8994_set_dai_sysclk() would only have to check if clock source is not already set. Please let me know if this option sounds better to you. >> If clock rate is let 0, the frequency provided by >> wm8994_set_dai_sysclk() is used instead. > > I'd expect this the other way around, if we didn't specify a frequency > then read it from the input otherwise try to use clk_set_rate() to > propagate things up. > If I implement a control to select the AIF source, I will drop the code related to mclk clock provider. Regards Olivier
On Fri, Dec 15, 2017 at 03:15:22PM +0000, Olivier MOYSAN wrote: > You are right. wm8994 allows to select either MCLK for each AIF. > From this point of view, the current patch is too limiting. > MCLK information in DT allows to enforce MCLK use, but an additionnal > information is required to determine AIF MCLK assignment. > Available properties in codec DAI node, such as clocks property, cannot > help here. > Maybe a DAPM linked to a control is a better way to select AIF source, > When source is not provided by clk_id in wm8994_set_dai_sysclk(). > In this case, wm8994_set_dai_sysclk() would only have to check > if clock source is not already set. > Please let me know if this option sounds better to you. What are you trying to accomplish here? You appear to be trying to move the system clocking configuration from the machine driver to the CODEC which is not how things are supposed to work.
Hello Mark, On 12/19/2017 10:35 AM, Mark Brown wrote: > On Fri, Dec 15, 2017 at 03:15:22PM +0000, Olivier MOYSAN wrote: > >> You are right. wm8994 allows to select either MCLK for each AIF. >> From this point of view, the current patch is too limiting. >> MCLK information in DT allows to enforce MCLK use, but an additionnal >> information is required to determine AIF MCLK assignment. >> Available properties in codec DAI node, such as clocks property, cannot >> help here. > >> Maybe a DAPM linked to a control is a better way to select AIF source, >> When source is not provided by clk_id in wm8994_set_dai_sysclk(). >> In this case, wm8994_set_dai_sysclk() would only have to check >> if clock source is not already set. > >> Please let me know if this option sounds better to you. > > What are you trying to accomplish here? You appear to be trying to move > the system clocking configuration from the machine driver to the CODEC > which is not how things are supposed to work. > As a generic machine, simple or audio graph cards are not able to manage codec clock muxing. If we exclude the management of muxing through codec controls, the remaining solution is to handle it fully through clock framework. The current patch only supports a limited range of muxing capabilities of the codec. To have a full management of the muxing, I think it is necessary to add a device tree node for each codec interface and to define an aif clock in these nodes. Then parent clock assignment of these aif clocks would allow to handle the muxing. Please, let me known if is this the right direction for you. BRs olivier
On Wed, Dec 20, 2017 at 12:42:10PM +0000, Olivier MOYSAN wrote: > As a generic machine, simple or audio graph cards are not able to manage > codec clock muxing. > If we exclude the management of muxing through codec controls, > the remaining solution is to handle it fully through clock framework. > The current patch only supports a limited range of muxing capabilities > of the codec. > To have a full management of the muxing, I think it is necessary to add > a device tree node for each codec interface and to define an aif clock > in these nodes. > Then parent clock assignment of these aif clocks would allow to handle > the muxing. Controlling clocking through a clock API binding would be good, yes. That'd solve a bunch of other problems with use of multi-purpose clocks for audio as well.
Hello Mark, On 12/20/2017 04:50 PM, Mark Brown wrote: > On Wed, Dec 20, 2017 at 12:42:10PM +0000, Olivier MOYSAN wrote: > >> As a generic machine, simple or audio graph cards are not able to manage >> codec clock muxing. >> If we exclude the management of muxing through codec controls, >> the remaining solution is to handle it fully through clock framework. >> The current patch only supports a limited range of muxing capabilities >> of the codec. >> To have a full management of the muxing, I think it is necessary to add >> a device tree node for each codec interface and to define an aif clock >> in these nodes. >> Then parent clock assignment of these aif clocks would allow to handle >> the muxing. > > Controlling clocking through a clock API binding would be good, yes. > That'd solve a bunch of other problems with use of multi-purpose clocks > for audio as well. > Thanks for your feedback. I will implement this in a patch v2. BRs
diff --git a/sound/soc/codecs/wm8994.c b/sound/soc/codecs/wm8994.c index 21ffd64..7a84e37 100644 --- a/sound/soc/codecs/wm8994.c +++ b/sound/soc/codecs/wm8994.c @@ -11,6 +11,7 @@ * published by the Free Software Foundation. */ +#include <linux/clk.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/init.h> @@ -2376,18 +2377,35 @@ static int wm8994_set_dai_sysclk(struct snd_soc_dai *dai, { struct snd_soc_codec *codec = dai->codec; struct wm8994_priv *wm8994 = snd_soc_codec_get_drvdata(codec); + struct wm8994 *control = wm8994->wm8994; + struct wm8994_pdata *pdata = &control->pdata; + unsigned long rate; int i; switch (dai->id) { case 1: + if (pdata->mclk1) { + rate = clk_get_rate(pdata->mclk1); + if (rate) + freq = (unsigned int)rate; + clk_id = WM8994_SYSCLK_MCLK1; + } + break; case 2: + if (pdata->mclk2) { + rate = clk_get_rate(pdata->mclk2); + if (rate) + freq = (unsigned int)rate; + clk_id = WM8994_SYSCLK_MCLK2; + } break; - default: /* AIF3 shares clocking with AIF1/2 */ return -EINVAL; } + dev_info(codec->dev, "%s:.clock id %d\n", __func__, clk_id); + switch (clk_id) { case WM8994_SYSCLK_MCLK1: wm8994->sysclk[dai->id - 1] = WM8994_SYSCLK_MCLK1;
When defined in device tree, MCLK1 and MCLK2 are used as sysclk for aif1 and aif2 interfaces respectively. If clock rate is let 0, the frequency provided by wm8994_set_dai_sysclk() is used instead. Signed-off-by: Olivier Moysan <olivier.moysan@st.com> --- sound/soc/codecs/wm8994.c | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-)