diff mbox

ASoC: rsnd: adg :: AUDIO-CLKOUTn can synchronizes with L/R clock.

Message ID 87oa7dtti3.wl%kuninori.morimoto.gx@renesas.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kuninori Morimoto June 7, 2016, 6:21 a.m. UTC
From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

AUDIO-CLKOUTn can synchronizes with L/R clock, and Salvator board
needs it. Otherwise, specific frequency sound will be noisy.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 .../devicetree/bindings/sound/renesas,rsnd.txt         |  2 ++
 sound/soc/sh/rcar/adg.c                                | 18 ++++++++++++++++++
 2 files changed, 20 insertions(+)

Comments

Mark Brown June 7, 2016, 10:54 a.m. UTC | #1
On Tue, Jun 07, 2016 at 06:21:33AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> AUDIO-CLKOUTn can synchronizes with L/R clock, and Salvator board
> needs it. Otherwise, specific frequency sound will be noisy.

Why would a user not want these clocks to be synchronous?  A lot of
CODECs will at least have better performance if their master clock is
synchronous to the audio clocks so it'd be a better default, is there an
advantage to not doing it?
Kuninori Morimoto June 8, 2016, 12:27 a.m. UTC | #2
Hi Mark

> > AUDIO-CLKOUTn can synchronizes with L/R clock, and Salvator board
> > needs it. Otherwise, specific frequency sound will be noisy.
> 
> Why would a user not want these clocks to be synchronous?  A lot of
> CODECs will at least have better performance if their master clock is
> synchronous to the audio clocks so it'd be a better default, is there an
> advantage to not doing it?

I'm now confusing. We can set system clock on audio card, for example
simple-card case, it is called as "system-clock-frequency".
In my understanding, this "system clock" and above "master clock" are same clock.
but "system clock" is fixed rate (= not related to audio clock in many cases).
Because of this, some codec doesn't request synchronous between
master clock <-> audio clock, but, some codec requests synchronous it.
Am I wrong ??
Mark Brown June 8, 2016, 3:14 p.m. UTC | #3
On Wed, Jun 08, 2016 at 12:27:48AM +0000, Kuninori Morimoto wrote:

> > Why would a user not want these clocks to be synchronous?  A lot of
> > CODECs will at least have better performance if their master clock is
> > synchronous to the audio clocks so it'd be a better default, is there an
> > advantage to not doing it?

> I'm now confusing. We can set system clock on audio card, for example
> simple-card case, it is called as "system-clock-frequency".
> In my understanding, this "system clock" and above "master clock" are same clock.
> but "system clock" is fixed rate (= not related to audio clock in many cases).
> Because of this, some codec doesn't request synchronous between
> master clock <-> audio clock, but, some codec requests synchronous it.
> Am I wrong ??

A lot of CODECs can to varying degrees tolerate this but it will tend to
show up in performance numbers, it's often not immediately obvious just
from a listening test.  Usually those that don't mind have a FLL or PLL
they're using to generate the actual audio master clock at a useful rate
for dividing down, or sometimes fancy digital logic to match things
up.

It's not a problem to have this configurable, I'm just thinking it might
be better to have the default be the other way around so that we default
to synchronous but can turn that off if it's required.
Kuninori Morimoto June 8, 2016, 11:52 p.m. UTC | #4
Hi Mark

Thank you for your explanation

> > I'm now confusing. We can set system clock on audio card, for example
> > simple-card case, it is called as "system-clock-frequency".
> > In my understanding, this "system clock" and above "master clock" are same clock.
> > but "system clock" is fixed rate (= not related to audio clock in many cases).
> > Because of this, some codec doesn't request synchronous between
> > master clock <-> audio clock, but, some codec requests synchronous it.
> > Am I wrong ??
> 
> A lot of CODECs can to varying degrees tolerate this but it will tend to
> show up in performance numbers, it's often not immediately obvious just
> from a listening test.  Usually those that don't mind have a FLL or PLL
> they're using to generate the actual audio master clock at a useful rate
> for dividing down, or sometimes fancy digital logic to match things
> up.
> 
> It's not a problem to have this configurable, I'm just thinking it might
> be better to have the default be the other way around so that we default
> to synchronous but can turn that off if it's required.

OK, I see.
I checked existing DT, and default synchronous <-> asynchronous
exchange seems no problem.
So, synchronous will be default in v2
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/sound/renesas,rsnd.txt b/Documentation/devicetree/bindings/sound/renesas,rsnd.txt
index c7b29df..065d9c8 100644
--- a/Documentation/devicetree/bindings/sound/renesas,rsnd.txt
+++ b/Documentation/devicetree/bindings/sound/renesas,rsnd.txt
@@ -373,6 +373,8 @@  Optional properties:
 - #clock-cells			: it must be 0 if your system has audio_clkout
 				  it must be 1 if your system has audio_clkout0/1/2/3
 - clock-frequency		: for all audio_clkout0/1/2/3
+- clkout-lr-synchronous		: boolean property. it indicates that audio_clkoutn
+				  synchronizes with lr-clock.
 
 SSI subnode properties:
 - interrupts			: Should contain SSI interrupt for PIO transfer
diff --git a/sound/soc/sh/rcar/adg.c b/sound/soc/sh/rcar/adg.c
index c4c51a4..1586271 100644
--- a/sound/soc/sh/rcar/adg.c
+++ b/sound/soc/sh/rcar/adg.c
@@ -33,11 +33,15 @@  struct rsnd_adg {
 	struct clk *clkout[CLKOUTMAX];
 	struct clk_onecell_data onecell;
 	struct rsnd_mod mod;
+	u32 flags;
 
 	int rbga_rate_for_441khz; /* RBGA */
 	int rbgb_rate_for_48khz;  /* RBGB */
 };
 
+#define LRCLK_SYNC	(1 << 0)
+#define adg_mode_flags(adg)	(adg->flags)
+
 #define for_each_rsnd_clk(pos, adg, i)		\
 	for (i = 0;				\
 	     (i < CLKMAX) &&			\
@@ -355,6 +359,16 @@  found_clock:
 
 	rsnd_adg_set_ssi_clk(ssi_mod, data);
 
+	if (adg_mode_flags(adg) & LRCLK_SYNC) {
+		struct rsnd_mod *adg_mod = rsnd_mod_get(adg);
+		u32 ckr = 0;
+
+		if (0 == (rate % 8000))
+			ckr = 0x80000000;
+
+		rsnd_mod_bset(adg_mod, SSICKR, 0x80000000, ckr);
+	}
+
 	dev_dbg(dev, "ADG: %s[%d] selects 0x%x for %d\n",
 		rsnd_mod_name(ssi_mod), rsnd_mod_id(ssi_mod),
 		data, rate);
@@ -532,6 +546,7 @@  int rsnd_adg_probe(struct rsnd_priv *priv)
 {
 	struct rsnd_adg *adg;
 	struct device *dev = rsnd_priv_to_dev(priv);
+	struct device_node *np = dev->of_node;
 
 	adg = devm_kzalloc(dev, sizeof(*adg), GFP_KERNEL);
 	if (!adg) {
@@ -545,6 +560,9 @@  int rsnd_adg_probe(struct rsnd_priv *priv)
 	rsnd_adg_get_clkin(priv, adg);
 	rsnd_adg_get_clkout(priv, adg);
 
+	if (of_get_property(np, "clkout-lr-synchronous", NULL))
+		adg->flags = LRCLK_SYNC;
+
 	priv->adg = adg;
 
 	return 0;