[alsa-devel] [PATCH] ASoC: rsnd: use device dependency clock
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current R-Car sound driver is using device independent audio clock, but it is not good design for DT (= common clock) support. This patch adds device dependent clock support. But, there are some platform which is using old style clock. Old style clock is still supported at this point, but it will be removed soon.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/sh/rcar/adg.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c index a53235c..e82ab75 100644 --- a/sound/soc/sh/rcar/adg.c +++ b/sound/soc/sh/rcar/adg.c @@ -259,8 +259,9 @@ int rsnd_adg_probe(struct platform_device *pdev, { struct rsnd_adg *adg; struct device *dev = rsnd_priv_to_dev(priv); - struct clk *clk; + struct clk *clk, *clk_orig; int i; + bool use_old_style = false;
adg = devm_kzalloc(dev, sizeof(*adg), GFP_KERNEL); if (!adg) { @@ -268,10 +269,34 @@ int rsnd_adg_probe(struct platform_device *pdev, return -ENOMEM; }
- adg->clk[CLKA] = clk_get(NULL, "audio_clk_a"); - adg->clk[CLKB] = clk_get(NULL, "audio_clk_b"); - adg->clk[CLKC] = clk_get(NULL, "audio_clk_c"); - adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal"); + clk_orig = clk_get(dev, NULL); + adg->clk[CLKA] = clk_get(dev, "clk_a"); + adg->clk[CLKB] = clk_get(dev, "clk_b"); + adg->clk[CLKC] = clk_get(dev, "clk_c"); + adg->clk[CLKI] = clk_get(dev, "clk_i"); + + for_each_rsnd_clk(clk, adg, i) { + if (clk_orig == clk) { + dev_warn(dev, + "Audio clock doesn't support new style," + "use old style\n"); + use_old_style = true; + break; + } + } + + /* + * note: + * these exist in order to keep compatible with old style, + * but will be removed soon + */ + if (use_old_style) { + adg->clk[CLKA] = clk_get(NULL, "audio_clk_a"); + adg->clk[CLKB] = clk_get(NULL, "audio_clk_b"); + adg->clk[CLKC] = clk_get(NULL, "audio_clk_c"); + adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal"); + } + for_each_rsnd_clk(clk, adg, i) { if (IS_ERR(clk)) { dev_err(dev, "Audio clock failed\n");
On Thu, Jan 30, 2014 at 11:30:26PM -0800, Kuninori Morimoto wrote:
- clk_orig = clk_get(dev, NULL);
- adg->clk[CLKA] = clk_get(dev, "clk_a");
- adg->clk[CLKB] = clk_get(dev, "clk_b");
- adg->clk[CLKC] = clk_get(dev, "clk_c");
- adg->clk[CLKI] = clk_get(dev, "clk_i");
- for_each_rsnd_clk(clk, adg, i) {
if (clk_orig == clk) {dev_warn(dev,"Audio clock doesn't support new style,""use old style\n");use_old_style = true;break;}- }
This looks really strange though I appreciate it's a workaround for a custom clock implementation. I guess it's not possible for a kernel to be built with both common and legacy clock implementations? If that is the case I think it would be clearer to just use an ifdef.
It's also better to not split error messages between lines, that makes it easier to grep for the code that generated the error.
Hi Mark
- clk_orig = clk_get(dev, NULL);
- adg->clk[CLKA] = clk_get(dev, "clk_a");
- adg->clk[CLKB] = clk_get(dev, "clk_b");
- adg->clk[CLKC] = clk_get(dev, "clk_c");
- adg->clk[CLKI] = clk_get(dev, "clk_i");
- for_each_rsnd_clk(clk, adg, i) {
if (clk_orig == clk) {dev_warn(dev,"Audio clock doesn't support new style,""use old style\n");use_old_style = true;break;}- }
This looks really strange though I appreciate it's a workaround for a custom clock implementation. I guess it's not possible for a kernel to be built with both common and legacy clock implementations? If that is the case I think it would be clearer to just use an ifdef.
Sorry, I guess my comment makes confusion. Here "new style" means "external sound clock has relationship with sound driver", "old style" means "external sound clock is independent clock"
This patch adds new clock-relationship rule to rsnd <-> platform. (sound clock should has relationship to sound drvier)
The external sound clocks (= clk_a/b/c/i) are "independent" clock in current linus/master. Then, all clk_get(dev, xxx) will be same clock (= module clock) (It was not strange if clk_get(dev, xxx) returns NULL though, but...)
# Above code can works well both common/legacy clock implementations. # And, it can't select both common/legacy clock in same time.
I will exchange git log comment (= about "new/old" clock), is it OK ?
It's also better to not split error messages between lines, that makes it easier to grep for the code that generated the error.
I see I will fix it in v2 patch
On Tue, Feb 04, 2014 at 04:44:34PM -0800, Kuninori Morimoto wrote:
Here "new style" means "external sound clock has relationship with sound driver", "old style" means "external sound clock is independent clock"
This patch adds new clock-relationship rule to rsnd <-> platform. (sound clock should has relationship to sound drvier)
The external sound clocks (= clk_a/b/c/i) are "independent" clock in current linus/master. Then, all clk_get(dev, xxx) will be same clock (= module clock) (It was not strange if clk_get(dev, xxx) returns NULL though, but...)
# Above code can works well both common/legacy clock implementations. # And, it can't select both common/legacy clock in same time.
I will exchange git log comment (= about "new/old" clock), is it OK ?
OK, I'm hoping that this situation is not expected to last more than a kernel release or so until all the platforms are converted - can we have a comment explaining the issue giving an ETA in the code as well as in the changelog please?
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current R-Car sound driver is using device independent audio clock, but it is not good design for DT support. This patch adds device dependent clock support. But, there are some platform which is using independent audio clock. It is still supported at this point, but it will be removed soon.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- v1 -> v2
- fixup git log comment - add explain on code
sound/soc/sh/rcar/adg.c | 40 +++++++++++++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c index 821791e..8d3a82e 100644 --- a/sound/soc/sh/rcar/adg.c +++ b/sound/soc/sh/rcar/adg.c @@ -385,8 +385,9 @@ int rsnd_adg_probe(struct platform_device *pdev, { struct rsnd_adg *adg; struct device *dev = rsnd_priv_to_dev(priv); - struct clk *clk; + struct clk *clk, *clk_orig; int i; + bool use_old_style = false;
adg = devm_kzalloc(dev, sizeof(*adg), GFP_KERNEL); if (!adg) { @@ -394,10 +395,39 @@ int rsnd_adg_probe(struct platform_device *pdev, return -ENOMEM; }
- adg->clk[CLKA] = clk_get(NULL, "audio_clk_a"); - adg->clk[CLKB] = clk_get(NULL, "audio_clk_b"); - adg->clk[CLKC] = clk_get(NULL, "audio_clk_c"); - adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal"); + clk_orig = clk_get(dev, NULL); + adg->clk[CLKA] = clk_get(dev, "clk_a"); + adg->clk[CLKB] = clk_get(dev, "clk_b"); + adg->clk[CLKC] = clk_get(dev, "clk_c"); + adg->clk[CLKI] = clk_get(dev, "clk_i"); + + /* + * It request device dependent audio clock. + * But above all clks will indicate rsnd module clock + * if platform doesn't it + */ + for_each_rsnd_clk(clk, adg, i) { + if (clk_orig == clk) { + dev_warn(dev, + "doesn't have device dependent clock, use independent clock\n"); + use_old_style = true; + break; + } + } + + /* + * note: + * these exist in order to keep compatible with + * platform which has device independent audio clock, + * but will be removed soon + */ + if (use_old_style) { + adg->clk[CLKA] = clk_get(NULL, "audio_clk_a"); + adg->clk[CLKB] = clk_get(NULL, "audio_clk_b"); + adg->clk[CLKC] = clk_get(NULL, "audio_clk_c"); + adg->clk[CLKI] = clk_get(NULL, "audio_clk_internal"); + } + for_each_rsnd_clk(clk, adg, i) { if (IS_ERR(clk)) { dev_err(dev, "Audio clock failed\n");
On Fri, Feb 07, 2014 at 12:53:06AM -0800, Kuninori Morimoto wrote:
From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
Current R-Car sound driver is using device independent audio clock, but it is not good design for DT support.
Applied, thanks.
participants (2)
-
Kuninori Morimoto -
Mark Brown