Re: [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface

2014-05-20 Thread Sylwester Nawrocki 
On 20/05/14 07:14, Tushar Behera wrote:
 On 05/19/2014 11:00 PM, Sylwester Nawrocki wrote:
 This patch adds support for setting source clock of the Core CLK
 of the IIS Multi Audio Interface.

 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 ---
  sound/soc/samsung/i2s.c |4 
  sound/soc/samsung/i2s.h |1 +
  2 files changed, 5 insertions(+)

 diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
 index 048ead9..ae02811 100644
 --- a/sound/soc/samsung/i2s.c
 +++ b/sound/soc/samsung/i2s.c
 @@ -451,6 +451,10 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
  u32 mod = readl(i2s-addr + I2SMOD);
  
  switch (clk_id) {
 +case SAMSUNG_I2S_OPCLK:
 +mod = ~MOD_OPCLK_MASK;
 +mod |= dir;
 
 I am assuming here that dir is one of SND_SOC_CLOCK_IN or
 SND_SOC_CLOCK_OUT. In that case, you need to take care of offset (30).

And that's not a correct assumption. I also got similarly confused when 
first seeing this in our downstream kernels. 'dir' is supposed to be one 
of the MOD_OPCLK_* constants, as defined in sound/soc/samsung/i2s-regs.h.

#define MOD_OPCLK_CDCLK_OUT (0  30)
#define MOD_OPCLK_CDCLK_IN  (1  30)
#define MOD_OPCLK_BCLK_OUT  (2  30)
#define MOD_OPCLK_PCLK  (3  30)
#define MOD_OPCLK_MASK  (3  30)

So the clock is reconfigured with calls like:
snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_OPCLK, 0, MOD_OPCLK_PCLK);

 Also the value of this bit-field doesn't match with SND_SOC_CLOCK_XXX
 macros.
 
 Bit-field (2'b):
 00Codec Clock out
 01Codec Clock in
 10Bit clock out
 11Audio bus clock
 
 Value of macros:
 SND_SOC_CLOCK_IN  0
 SND_SOC_CLOCK_OUT 1

 In the manual, this field is suggested to be Audio bus clock always. Is
 there an use-case where we might need to update this?

I've checked couple boards and AFAICS we're always setting MOD_OPCLK_PCLK,
however it's still different from the default value after reset - 
MOD_OPCLK_CDCLK_OUT. 
So how do you think this should be addressed ? Isn't it better to give
options to the machine drivers to alter these clock settings, rather than
hard coding it in the I2S driver ? Let's not forget it covers multiple 
Samsung SoC series.

 The default value for audio playback right now is 00 (2'b), which needs
 to be fixed anyways.

--
Regards,
Sylwester
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface

2014-05-20 Thread Tushar Behera 
On 05/20/2014 02:40 PM, Sylwester Nawrocki wrote:
 On 20/05/14 07:14, Tushar Behera wrote:
 On 05/19/2014 11:00 PM, Sylwester Nawrocki wrote:
 This patch adds support for setting source clock of the Core CLK
 of the IIS Multi Audio Interface.

 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 ---
  sound/soc/samsung/i2s.c |4 
  sound/soc/samsung/i2s.h |1 +
  2 files changed, 5 insertions(+)

 diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
 index 048ead9..ae02811 100644
 --- a/sound/soc/samsung/i2s.c
 +++ b/sound/soc/samsung/i2s.c
 @@ -451,6 +451,10 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
 u32 mod = readl(i2s-addr + I2SMOD);
  
 switch (clk_id) {
 +   case SAMSUNG_I2S_OPCLK:
 +   mod = ~MOD_OPCLK_MASK;
 +   mod |= dir;

 I am assuming here that dir is one of SND_SOC_CLOCK_IN or
 SND_SOC_CLOCK_OUT. In that case, you need to take care of offset (30).
 
 And that's not a correct assumption. I also got similarly confused when 
 first seeing this in our downstream kernels. 'dir' is supposed to be one 
 of the MOD_OPCLK_* constants, as defined in sound/soc/samsung/i2s-regs.h.
 
 #define MOD_OPCLK_CDCLK_OUT   (0  30)
 #define MOD_OPCLK_CDCLK_IN(1  30)
 #define MOD_OPCLK_BCLK_OUT(2  30)
 #define MOD_OPCLK_PCLK(3  30)
 #define MOD_OPCLK_MASK(3  30)
 
 So the clock is reconfigured with calls like:
 snd_soc_dai_set_sysclk(cpu_dai, SAMSUNG_I2S_OPCLK, 0, MOD_OPCLK_PCLK);
 

Ok.

 Also the value of this bit-field doesn't match with SND_SOC_CLOCK_XXX
 macros.

 Bit-field (2'b):
 00   Codec Clock out
 01   Codec Clock in
 10   Bit clock out
 11   Audio bus clock

 Value of macros:
 SND_SOC_CLOCK_IN 0
 SND_SOC_CLOCK_OUT1
 
 In the manual, this field is suggested to be Audio bus clock always. Is
 there an use-case where we might need to update this?
 
 I've checked couple boards and AFAICS we're always setting MOD_OPCLK_PCLK,
 however it's still different from the default value after reset - 
 MOD_OPCLK_CDCLK_OUT. 
 So how do you think this should be addressed ? Isn't it better to give
 options to the machine drivers to alter these clock settings, rather than
 hard coding it in the I2S driver ? Let's not forget it covers multiple 
 Samsung SoC series.
 

I don't have any objection to this patch. Just that, I could not find
any use-case where we would not like to set the value as MOD_OPCLK_PCLK.

 The default value for audio playback right now is 00 (2'b), which needs
 to be fixed anyways.
 
 --
 Regards,
 Sylwester
 


-- 
Tushar Behera
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface

2014-05-20 Thread Mark Brown 
On Tue, May 20, 2014 at 11:10:08AM +0200, Sylwester Nawrocki wrote:

 So how do you think this should be addressed ? Isn't it better to give
 options to the machine drivers to alter these clock settings, rather than
 hard coding it in the I2S driver ? Let's not forget it covers multiple 
 Samsung SoC series.

Yes, it's better to make it controllable - ideally this could be moved
into the clock API but for ASoC level stuff set_sysclk() with a sensible
default is how it's supposed to be done.


signature.asc
Description: Digital signature

Re: [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface

2014-05-20 Thread Mark Brown 
On Mon, May 19, 2014 at 07:30:38PM +0200, Sylwester Nawrocki wrote:
 This patch adds support for setting source clock of the Core CLK
 of the IIS Multi Audio Interface.

Applied, thanks.


signature.asc
Description: Digital signature

[PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface

2014-05-19 Thread Sylwester Nawrocki 
This patch adds support for setting source clock of the Core CLK
of the IIS Multi Audio Interface.

Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
---
 sound/soc/samsung/i2s.c |4 
 sound/soc/samsung/i2s.h |1 +
 2 files changed, 5 insertions(+)

diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
index 048ead9..ae02811 100644
--- a/sound/soc/samsung/i2s.c
+++ b/sound/soc/samsung/i2s.c
@@ -451,6 +451,10 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
u32 mod = readl(i2s-addr + I2SMOD);
 
switch (clk_id) {
+   case SAMSUNG_I2S_OPCLK:
+   mod = ~MOD_OPCLK_MASK;
+   mod |= dir;
+   break;
case SAMSUNG_I2S_CDCLK:
/* Shouldn't matter in GATING(CLOCK_IN) mode */
if (dir == SND_SOC_CLOCK_IN)
diff --git a/sound/soc/samsung/i2s.h b/sound/soc/samsung/i2s.h
index 7966afc..21ff24e 100644
--- a/sound/soc/samsung/i2s.h
+++ b/sound/soc/samsung/i2s.h
@@ -18,5 +18,6 @@
 #define SAMSUNG_I2S_RCLKSRC_0  0
 #define SAMSUNG_I2S_RCLKSRC_1  1
 #define SAMSUNG_I2S_CDCLK  2
+#define SAMSUNG_I2S_OPCLK  3
 
 #endif /* __SND_SOC_SAMSUNG_I2S_H */
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface

2014-05-19 Thread Tushar Behera 
On 05/19/2014 11:00 PM, Sylwester Nawrocki wrote:
 This patch adds support for setting source clock of the Core CLK
 of the IIS Multi Audio Interface.
 
 Signed-off-by: Sylwester Nawrocki s.nawro...@samsung.com
 ---
  sound/soc/samsung/i2s.c |4 
  sound/soc/samsung/i2s.h |1 +
  2 files changed, 5 insertions(+)
 
 diff --git a/sound/soc/samsung/i2s.c b/sound/soc/samsung/i2s.c
 index 048ead9..ae02811 100644
 --- a/sound/soc/samsung/i2s.c
 +++ b/sound/soc/samsung/i2s.c
 @@ -451,6 +451,10 @@ static int i2s_set_sysclk(struct snd_soc_dai *dai,
   u32 mod = readl(i2s-addr + I2SMOD);
  
   switch (clk_id) {
 + case SAMSUNG_I2S_OPCLK:
 + mod = ~MOD_OPCLK_MASK;
 + mod |= dir;

I am assuming here that dir is one of SND_SOC_CLOCK_IN or
SND_SOC_CLOCK_OUT. In that case, you need to take care of offset (30).

Also the value of this bit-field doesn't match with SND_SOC_CLOCK_XXX
macros.

Bit-field (2'b):
00  Codec Clock out
01  Codec Clock in
10  Bit clock out
11  Audio bus clock

Value of macros:
SND_SOC_CLOCK_IN0
SND_SOC_CLOCK_OUT   1

In the manual, this field is suggested to be Audio bus clock always. Is
there an use-case where we might need to update this?

The default value for audio playback right now is 00 (2'b), which needs
to be fixed anyways.

-- 
Tushar Behera
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html