* [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface
@ 2014-05-19 17:30 Sylwester Nawrocki
2014-05-20 5:14 ` Tushar Behera
2014-05-20 22:21 ` Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Sylwester Nawrocki @ 2014-05-19 17:30 UTC (permalink / raw)
To: linux-arm-kernel
This patch adds support for setting source clock of the "Core CLK"
of the IIS Multi Audio Interface.
Signed-off-by: Sylwester Nawrocki <s.nawrocki@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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface
2014-05-19 17:30 [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface Sylwester Nawrocki
@ 2014-05-20 5:14 ` Tushar Behera
2014-05-20 9:10 ` Sylwester Nawrocki
2014-05-20 22:21 ` Mark Brown
1 sibling, 1 reply; 6+ messages in thread
From: Tushar Behera @ 2014-05-20 5:14 UTC (permalink / raw)
To: linux-arm-kernel
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.nawrocki@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_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?
The default value for audio playback right now is 00 (2'b), which needs
to be fixed anyways.
--
Tushar Behera
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface
2014-05-20 5:14 ` Tushar Behera
@ 2014-05-20 9:10 ` Sylwester Nawrocki
2014-05-20 10:01 ` Tushar Behera
2014-05-20 10:10 ` Mark Brown
0 siblings, 2 replies; 6+ messages in thread
From: Sylwester Nawrocki @ 2014-05-20 9:10 UTC (permalink / raw)
To: linux-arm-kernel
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.nawrocki@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):
> 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_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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface
2014-05-20 9:10 ` Sylwester Nawrocki
@ 2014-05-20 10:01 ` Tushar Behera
2014-05-20 10:10 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Tushar Behera @ 2014-05-20 10:01 UTC (permalink / raw)
To: linux-arm-kernel
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.nawrocki@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_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.
>
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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface
2014-05-20 9:10 ` Sylwester Nawrocki
2014-05-20 10:01 ` Tushar Behera
@ 2014-05-20 10:10 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-05-20 10:10 UTC (permalink / raw)
To: linux-arm-kernel
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140520/b741032e/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface
2014-05-19 17:30 [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface Sylwester Nawrocki
2014-05-20 5:14 ` Tushar Behera
@ 2014-05-20 22:21 ` Mark Brown
1 sibling, 0 replies; 6+ messages in thread
From: Mark Brown @ 2014-05-20 22:21 UTC (permalink / raw)
To: linux-arm-kernel
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140520/b433431f/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-05-20 22:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-19 17:30 [PATCH] ASoC: samsung: Allow setting OP_CLK of the IIS Multi Audio Interface Sylwester Nawrocki
2014-05-20 5:14 ` Tushar Behera
2014-05-20 9:10 ` Sylwester Nawrocki
2014-05-20 10:01 ` Tushar Behera
2014-05-20 10:10 ` Mark Brown
2014-05-20 22:21 ` Mark Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).