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@xxxxxxxxxxx> >> --- >> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html