* [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: broonie; +Cc: alsa-devel, linux-samsung-soc, linux-arm-kernel, 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.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
* Re: [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: Sylwester Nawrocki, broonie Cc: alsa-devel, linux-samsung-soc, 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
* Re: [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: Tushar Behera; +Cc: broonie, alsa-devel, linux-samsung-soc, 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
* Re: [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: Sylwester Nawrocki Cc: broonie, alsa-devel, linux-samsung-soc, 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
* Re: [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: Sylwester Nawrocki Cc: Tushar Behera, alsa-devel, linux-samsung-soc, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 510 bytes --] 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. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [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: Sylwester Nawrocki; +Cc: alsa-devel, linux-samsung-soc, linux-arm-kernel [-- Attachment #1: Type: text/plain, Size: 191 bytes --] 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. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] ^ 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