| Message ID | 20180205142230.9755-2-s.nawrocki@samsung.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream, archived |
| Headers | show |
Hi Sylwester, On 2018년 02월 05일 23:22, Sylwester Nawrocki wrote: > CLK_SET_RATE_PARENT flag is added to definitions of clocks on a path > starting from CLK_SCLK_I2S1 up to AUD_PLL in order to allow setting > required audio root clock frequency for the I2S1 block. This is now > only done for the I2S1 block, related CMU_TOP, CMU_PERIC clock > definitions are changed. > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > --- > drivers/clk/samsung/clk-exynos5433.c | 22 +++++++++++----------- > 1 file changed, 11 insertions(+), 11 deletions(-) > > diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c > index 74b70ddab4d6..d74361736e64 100644 > --- a/drivers/clk/samsung/clk-exynos5433.c > +++ b/drivers/clk/samsung/clk-exynos5433.c > @@ -246,14 +246,14 @@ static const struct samsung_fixed_rate_clock top_fixed_clks[] __initconst = { > > static const struct samsung_mux_clock top_mux_clks[] __initconst = { > /* MUX_SEL_TOP0 */ > - MUX(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, > - 4, 1), > + MUX_F(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, > + 4, 1, CLK_SET_RATE_PARENT, 0), If you add CLK_SET_RATE_PARENT to 'mout_aud_pll' and mout_aud_pll changes the rate, fout_aud_pll's rate will be changed. But, fout_aud_pll is also the parent of 'mout_aud_pll_user'. It might change the rate of children of mout_aud_pll_user. mout_aud_pll_user would not want to change the parent's clock. fout_aud_pll 2 2 196608009 0 0 mout_aud_pll_user 1 1 196608009 0 0 mout_aud_pll 0 0 196608009 0 0 > MUX(CLK_MOUT_ISP_PLL, "mout_isp_pll", mout_isp_pll_p, MUX_SEL_TOP0, > 0, 1), > > /* MUX_SEL_TOP1 */ > - MUX(CLK_MOUT_AUD_PLL_USER_T, "mout_aud_pll_user_t", > - mout_aud_pll_user_p, MUX_SEL_TOP1, 12, 1), > + MUX_F(CLK_MOUT_AUD_PLL_USER_T, "mout_aud_pll_user_t", > + mout_aud_pll_user_p, MUX_SEL_TOP1, 12, 1, CLK_SET_RATE_PARENT, 0), > MUX(CLK_MOUT_MPHY_PLL_USER, "mout_mphy_pll_user", mout_mphy_pll_user_p, > MUX_SEL_TOP1, 8, 1), > MUX(CLK_MOUT_MFC_PLL_USER, "mout_mfc_pll_user", mout_mfc_pll_user_p, > @@ -370,8 +370,8 @@ static const struct samsung_mux_clock top_mux_clks[] __initconst = { > MUX_SEL_TOP_PERIC1, 16, 1), > MUX(CLK_MOUT_SCLK_SPDIF, "mout_sclk_spdif", mout_sclk_spdif_p, > MUX_SEL_TOP_PERIC1, 12, 2), > - MUX(CLK_MOUT_SCLK_AUDIO1, "mout_sclk_audio1", mout_sclk_audio1_p, > - MUX_SEL_TOP_PERIC1, 4, 2), > + MUX_F(CLK_MOUT_SCLK_AUDIO1, "mout_sclk_audio1", mout_sclk_audio1_p, > + MUX_SEL_TOP_PERIC1, 4, 2, CLK_SET_RATE_PARENT, 0), > MUX(CLK_MOUT_SCLK_AUDIO0, "mout_sclk_audio0", mout_sclk_audio0_p, > MUX_SEL_TOP_PERIC1, 0, 2), > > @@ -524,12 +524,12 @@ static const struct samsung_div_clock top_div_clks[] __initconst = { > DIV_TOP_PERIC2, 0, 4), > > /* DIV_TOP_PERIC3 */ > - DIV(CLK_DIV_SCLK_I2S1, "div_sclk_i2s1", "sclk_audio1", > - DIV_TOP_PERIC3, 16, 6), > + DIV_F(CLK_DIV_SCLK_I2S1, "div_sclk_i2s1", "sclk_audio1", > + DIV_TOP_PERIC3, 16, 6, CLK_SET_RATE_PARENT, 0), > DIV(CLK_DIV_SCLK_PCM1, "div_sclk_pcm1", "sclk_audio1", > DIV_TOP_PERIC3, 8, 8), > - DIV(CLK_DIV_SCLK_AUDIO1, "div_sclk_audio1", "mout_sclk_audio1", > - DIV_TOP_PERIC3, 4, 4), > + DIV_F(CLK_DIV_SCLK_AUDIO1, "div_sclk_audio1", "mout_sclk_audio1", > + DIV_TOP_PERIC3, 4, 4, CLK_SET_RATE_PARENT, 0), > DIV(CLK_DIV_SCLK_AUDIO0, "div_sclk_audio0", "mout_sclk_audio0", > DIV_TOP_PERIC3, 0, 4), > > @@ -693,7 +693,7 @@ static const struct samsung_gate_clock top_gate_clks[] __initconst = { > GATE(CLK_SCLK_SLIMBUS, "sclk_slimbus", "mout_sclk_slimbus", > MUX_ENABLE_TOP_PERIC1, 16, 0, 0), > GATE(CLK_SCLK_AUDIO1, "sclk_audio1", "div_sclk_audio1", > - MUX_ENABLE_TOP_PERIC1, 4, 0, 0), > + MUX_ENABLE_TOP_PERIC1, 4, CLK_SET_RATE_PARENT, 0), > GATE(CLK_SCLK_AUDIO0, "sclk_audio0", "div_sclk_audio0", > MUX_ENABLE_TOP_PERIC1, 0, 0, 0), > }; >
Hi Chanwoo, On 02/06/2018 05:06 AM, Chanwoo Choi wrote: >> drivers/clk/samsung/clk-exynos5433.c | 22 +++++++++++----------- >> 1 file changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c >> index 74b70ddab4d6..d74361736e64 100644 >> --- a/drivers/clk/samsung/clk-exynos5433.c >> +++ b/drivers/clk/samsung/clk-exynos5433.c >> @@ -246,14 +246,14 @@ static const struct samsung_fixed_rate_clock top_fixed_clks[] __initconst = { >> >> static const struct samsung_mux_clock top_mux_clks[] __initconst = { >> /* MUX_SEL_TOP0 */ >> - MUX(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >> - 4, 1), >> + MUX_F(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >> + 4, 1, CLK_SET_RATE_PARENT, 0), > > If you add CLK_SET_RATE_PARENT to 'mout_aud_pll' and mout_aud_pll changes the rate, > fout_aud_pll's rate will be changed. But, fout_aud_pll is also the parent > of 'mout_aud_pll_user'. It might change the rate of children of mout_aud_pll_user. > mout_aud_pll_user would not want to change the parent's clock. > > fout_aud_pll 2 2 196608009 0 0 > mout_aud_pll_user 1 1 196608009 0 0 > mout_aud_pll 0 0 196608009 0 0 I'd say the range of changes is such that the consumers of the affected child clocks can cope and could adjust to the changed frequencies. Those consumer devices are all components/peripherals of the audio subsystem (LPASS) and, for example, in case of TM2 there is no issues at all with varying the AUD PLL frequency depending on the HDMI audio sample rate. The other audio path uses the audio CODEC's internal PLL as the root clock source. The AUD PLL frequency will need to be adjusted somehow anyway, we could also get the PLL clock directly and set it's rate, instead of relying on that rate propagation algorithm. I think we could also export a function from the exynos-lpass mfd driver for setting the PLL's rate directly, after listing the AUD PLL clock in the lpass DT node. That would be more flexible API, easier to adopt for various use cases/boards, now we have only TM2. I can't list the PLL clock in the sound node, that would not have passed the DT maintainters' review.
Hi Sylwester, On 2018년 02월 08일 00:18, Sylwester Nawrocki wrote: > Hi Chanwoo, > > On 02/06/2018 05:06 AM, Chanwoo Choi wrote: >>> drivers/clk/samsung/clk-exynos5433.c | 22 +++++++++++----------- >>> 1 file changed, 11 insertions(+), 11 deletions(-) >>> >>> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c >>> index 74b70ddab4d6..d74361736e64 100644 >>> --- a/drivers/clk/samsung/clk-exynos5433.c >>> +++ b/drivers/clk/samsung/clk-exynos5433.c >>> @@ -246,14 +246,14 @@ static const struct samsung_fixed_rate_clock top_fixed_clks[] __initconst = { >>> >>> static const struct samsung_mux_clock top_mux_clks[] __initconst = { >>> /* MUX_SEL_TOP0 */ >>> - MUX(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >>> - 4, 1), >>> + MUX_F(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >>> + 4, 1, CLK_SET_RATE_PARENT, 0), >> >> If you add CLK_SET_RATE_PARENT to 'mout_aud_pll' and mout_aud_pll changes the rate, >> fout_aud_pll's rate will be changed. But, fout_aud_pll is also the parent >> of 'mout_aud_pll_user'. It might change the rate of children of mout_aud_pll_user. >> mout_aud_pll_user would not want to change the parent's clock. >> >> fout_aud_pll 2 2 196608009 0 0 >> mout_aud_pll_user 1 1 196608009 0 0 >> mout_aud_pll 0 0 196608009 0 0 > > I'd say the range of changes is such that the consumers of the affected child > clocks can cope and could adjust to the changed frequencies. Those consumer > devices are all components/peripherals of the audio subsystem (LPASS) and, The mout_aud_pll_user has the child clock of serial_3. serial_3 was used for bluetooth on TM2. If you change the aud_pll with CLK_SET_RATE_PARENT, it might affect the bluetooth operation. The bluetooth is only used for transfering the data. Actually, I'm not sure that this patch might affect bluetooth operation or not. > for example, in case of TM2 there is no issues at all with varying the AUD PLL > frequency depending on the HDMI audio sample rate. The other audio path uses > the audio CODEC's internal PLL as the root clock source. The AUD PLL frequency > will need to be adjusted somehow anyway, we could also get the PLL clock > directly and set it's rate, instead of relying on that rate propagation > algorithm. I think we could also export a function from the exynos-lpass mfd > driver for setting the PLL's rate directly, after listing the AUD PLL clock > in the lpass DT node. That would be more flexible API, easier to adopt for > various use cases/boards, now we have only TM2. I can't list the PLL clock > in the sound node, that would not have passed the DT maintainters' review. >
Hi Chanwoo, On 02/09/2018 08:36 AM, Chanwoo Choi wrote: > On 2018년 02월 08일 00:18, Sylwester Nawrocki wrote: >> On 02/06/2018 05:06 AM, Chanwoo Choi wrote: >>>> drivers/clk/samsung/clk-exynos5433.c | 22 +++++++++++----------- >>>> 1 file changed, 11 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c >>>> index 74b70ddab4d6..d74361736e64 100644 >>>> --- a/drivers/clk/samsung/clk-exynos5433.c >>>> +++ b/drivers/clk/samsung/clk-exynos5433.c >>>> @@ -246,14 +246,14 @@ static const struct samsung_fixed_rate_clock top_fixed_clks[] __initconst = { >>>> >>>> static const struct samsung_mux_clock top_mux_clks[] __initconst = { >>>> /* MUX_SEL_TOP0 */ >>>> - MUX(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >>>> - 4, 1), >>>> + MUX_F(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >>>> + 4, 1, CLK_SET_RATE_PARENT, 0), >>> If you add CLK_SET_RATE_PARENT to 'mout_aud_pll' and mout_aud_pll changes the rate, >>> fout_aud_pll's rate will be changed. But, fout_aud_pll is also the parent >>> of 'mout_aud_pll_user'. It might change the rate of children of mout_aud_pll_user. >>> mout_aud_pll_user would not want to change the parent's clock. >>> >>> fout_aud_pll 2 2 196608009 0 0 >>> mout_aud_pll_user 1 1 196608009 0 0 >>> mout_aud_pll 0 0 196608009 0 0 >> I'd say the range of changes is such that the consumers of the affected child >> clocks can cope and could adjust to the changed frequencies. Those consumer >> devices are all components/peripherals of the audio subsystem (LPASS) and, > > The mout_aud_pll_user has the child clock of serial_3. > serial_3 was used for bluetooth on TM2. If you change the aud_pll > with CLK_SET_RATE_PARENT, it might affect the bluetooth operation. > The bluetooth is only used for transfering the data. > > Actually, I'm not sure that this patch might affect bluetooth operation or not. You are right, the AUD PLL frequency adjustments would break the bluetooth's operation. I double checked and in the downstream kernel only one AUD PLL frequency can be set - 196608009. So I will drop this patch and add just a single PLL_36XX_RATE() entry for that frequency, the PMS values have been confirmed by the HW team. Only 48000/9600/192000 sample rates will then be supported natively and others could be through software rate conversion.
Hi Sylwester, On 2018년 02월 12일 20:45, Sylwester Nawrocki wrote: > Hi Chanwoo, > > On 02/09/2018 08:36 AM, Chanwoo Choi wrote: >> On 2018년 02월 08일 00:18, Sylwester Nawrocki wrote: >>> On 02/06/2018 05:06 AM, Chanwoo Choi wrote: >>>>> drivers/clk/samsung/clk-exynos5433.c | 22 +++++++++++----------- >>>>> 1 file changed, 11 insertions(+), 11 deletions(-) >>>>> >>>>> diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c >>>>> index 74b70ddab4d6..d74361736e64 100644 >>>>> --- a/drivers/clk/samsung/clk-exynos5433.c >>>>> +++ b/drivers/clk/samsung/clk-exynos5433.c >>>>> @@ -246,14 +246,14 @@ static const struct samsung_fixed_rate_clock top_fixed_clks[] __initconst = { >>>>> >>>>> static const struct samsung_mux_clock top_mux_clks[] __initconst = { >>>>> /* MUX_SEL_TOP0 */ >>>>> - MUX(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >>>>> - 4, 1), >>>>> + MUX_F(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, >>>>> + 4, 1, CLK_SET_RATE_PARENT, 0), >>>> If you add CLK_SET_RATE_PARENT to 'mout_aud_pll' and mout_aud_pll changes the rate, >>>> fout_aud_pll's rate will be changed. But, fout_aud_pll is also the parent >>>> of 'mout_aud_pll_user'. It might change the rate of children of mout_aud_pll_user. >>>> mout_aud_pll_user would not want to change the parent's clock. >>>> >>>> fout_aud_pll 2 2 196608009 0 0 >>>> mout_aud_pll_user 1 1 196608009 0 0 >>>> mout_aud_pll 0 0 196608009 0 0 >>> I'd say the range of changes is such that the consumers of the affected child >>> clocks can cope and could adjust to the changed frequencies. Those consumer >>> devices are all components/peripherals of the audio subsystem (LPASS) and, >> >> The mout_aud_pll_user has the child clock of serial_3. >> serial_3 was used for bluetooth on TM2. If you change the aud_pll >> with CLK_SET_RATE_PARENT, it might affect the bluetooth operation. >> The bluetooth is only used for transfering the data. >> >> Actually, I'm not sure that this patch might affect bluetooth operation or not. > > You are right, the AUD PLL frequency adjustments would break the bluetooth's > operation. I double checked and in the downstream kernel only one AUD PLL > frequency can be set - 196608009. So I will drop this patch and add just > a single PLL_36XX_RATE() entry for that frequency, the PMS values have been > confirmed by the HW team. Only 48000/9600/192000 sample rates will then be > supported natively and others could be through software rate conversion. > OK. Thanks for your check.
diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c index 74b70ddab4d6..d74361736e64 100644 --- a/drivers/clk/samsung/clk-exynos5433.c +++ b/drivers/clk/samsung/clk-exynos5433.c @@ -246,14 +246,14 @@ static const struct samsung_fixed_rate_clock top_fixed_clks[] __initconst = { static const struct samsung_mux_clock top_mux_clks[] __initconst = { /* MUX_SEL_TOP0 */ - MUX(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, - 4, 1), + MUX_F(CLK_MOUT_AUD_PLL, "mout_aud_pll", mout_aud_pll_p, MUX_SEL_TOP0, + 4, 1, CLK_SET_RATE_PARENT, 0), MUX(CLK_MOUT_ISP_PLL, "mout_isp_pll", mout_isp_pll_p, MUX_SEL_TOP0, 0, 1), /* MUX_SEL_TOP1 */ - MUX(CLK_MOUT_AUD_PLL_USER_T, "mout_aud_pll_user_t", - mout_aud_pll_user_p, MUX_SEL_TOP1, 12, 1), + MUX_F(CLK_MOUT_AUD_PLL_USER_T, "mout_aud_pll_user_t", + mout_aud_pll_user_p, MUX_SEL_TOP1, 12, 1, CLK_SET_RATE_PARENT, 0), MUX(CLK_MOUT_MPHY_PLL_USER, "mout_mphy_pll_user", mout_mphy_pll_user_p, MUX_SEL_TOP1, 8, 1), MUX(CLK_MOUT_MFC_PLL_USER, "mout_mfc_pll_user", mout_mfc_pll_user_p, @@ -370,8 +370,8 @@ static const struct samsung_mux_clock top_mux_clks[] __initconst = { MUX_SEL_TOP_PERIC1, 16, 1), MUX(CLK_MOUT_SCLK_SPDIF, "mout_sclk_spdif", mout_sclk_spdif_p, MUX_SEL_TOP_PERIC1, 12, 2), - MUX(CLK_MOUT_SCLK_AUDIO1, "mout_sclk_audio1", mout_sclk_audio1_p, - MUX_SEL_TOP_PERIC1, 4, 2), + MUX_F(CLK_MOUT_SCLK_AUDIO1, "mout_sclk_audio1", mout_sclk_audio1_p, + MUX_SEL_TOP_PERIC1, 4, 2, CLK_SET_RATE_PARENT, 0), MUX(CLK_MOUT_SCLK_AUDIO0, "mout_sclk_audio0", mout_sclk_audio0_p, MUX_SEL_TOP_PERIC1, 0, 2), @@ -524,12 +524,12 @@ static const struct samsung_div_clock top_div_clks[] __initconst = { DIV_TOP_PERIC2, 0, 4), /* DIV_TOP_PERIC3 */ - DIV(CLK_DIV_SCLK_I2S1, "div_sclk_i2s1", "sclk_audio1", - DIV_TOP_PERIC3, 16, 6), + DIV_F(CLK_DIV_SCLK_I2S1, "div_sclk_i2s1", "sclk_audio1", + DIV_TOP_PERIC3, 16, 6, CLK_SET_RATE_PARENT, 0), DIV(CLK_DIV_SCLK_PCM1, "div_sclk_pcm1", "sclk_audio1", DIV_TOP_PERIC3, 8, 8), - DIV(CLK_DIV_SCLK_AUDIO1, "div_sclk_audio1", "mout_sclk_audio1", - DIV_TOP_PERIC3, 4, 4), + DIV_F(CLK_DIV_SCLK_AUDIO1, "div_sclk_audio1", "mout_sclk_audio1", + DIV_TOP_PERIC3, 4, 4, CLK_SET_RATE_PARENT, 0), DIV(CLK_DIV_SCLK_AUDIO0, "div_sclk_audio0", "mout_sclk_audio0", DIV_TOP_PERIC3, 0, 4), @@ -693,7 +693,7 @@ static const struct samsung_gate_clock top_gate_clks[] __initconst = { GATE(CLK_SCLK_SLIMBUS, "sclk_slimbus", "mout_sclk_slimbus", MUX_ENABLE_TOP_PERIC1, 16, 0, 0), GATE(CLK_SCLK_AUDIO1, "sclk_audio1", "div_sclk_audio1", - MUX_ENABLE_TOP_PERIC1, 4, 0, 0), + MUX_ENABLE_TOP_PERIC1, 4, CLK_SET_RATE_PARENT, 0), GATE(CLK_SCLK_AUDIO0, "sclk_audio0", "div_sclk_audio0", MUX_ENABLE_TOP_PERIC1, 0, 0, 0), };
CLK_SET_RATE_PARENT flag is added to definitions of clocks on a path starting from CLK_SCLK_I2S1 up to AUD_PLL in order to allow setting required audio root clock frequency for the I2S1 block. This is now only done for the I2S1 block, related CMU_TOP, CMU_PERIC clock definitions are changed. Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com> --- drivers/clk/samsung/clk-exynos5433.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)