* [PATCH 0/2] clk: davinci: some more fixes @ 2018-05-11 14:10 ` Sekhar Nori 0 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-11 14:10 UTC (permalink / raw) To: David Lechner, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Sekhar Nori, Kevin Hilman Hi, Some more fixes to make DM646x and DM365 EVMs boot after common clock framework conversion. With this, I have tested all DaVinci SoCs. Sekhar Nori (2): clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled clk: davinci: psc-dm365: fix few clocks drivers/clk/davinci/pll-dm646x.c | 2 +- drivers/clk/davinci/psc-dm365.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) -- 2.16.2 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/2] clk: davinci: some more fixes @ 2018-05-11 14:10 ` Sekhar Nori 0 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-11 14:10 UTC (permalink / raw) To: linux-arm-kernel Hi, Some more fixes to make DM646x and DM365 EVMs boot after common clock framework conversion. With this, I have tested all DaVinci SoCs. Sekhar Nori (2): clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled clk: davinci: psc-dm365: fix few clocks drivers/clk/davinci/pll-dm646x.c | 2 +- drivers/clk/davinci/psc-dm365.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) -- 2.16.2 ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled 2018-05-11 14:10 ` Sekhar Nori @ 2018-05-11 14:10 ` Sekhar Nori -1 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-11 14:10 UTC (permalink / raw) To: David Lechner, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Sekhar Nori, Kevin Hilman PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot be disabled. Mark it so to prevent unused clock disable infrastructure from disabling it. Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- drivers/clk/davinci/pll-dm646x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c index eb96dd72b6b7..5bdf1cb5fda8 100644 --- a/drivers/clk/davinci/pll-dm646x.c +++ b/drivers/clk/davinci/pll-dm646x.c @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = { .flags = 0, }; -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip) { -- 2.16.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled @ 2018-05-11 14:10 ` Sekhar Nori 0 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-11 14:10 UTC (permalink / raw) To: linux-arm-kernel PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot be disabled. Mark it so to prevent unused clock disable infrastructure from disabling it. Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- drivers/clk/davinci/pll-dm646x.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c index eb96dd72b6b7..5bdf1cb5fda8 100644 --- a/drivers/clk/davinci/pll-dm646x.c +++ b/drivers/clk/davinci/pll-dm646x.c @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = { .flags = 0, }; -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip) { -- 2.16.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled 2018-05-11 14:10 ` Sekhar Nori @ 2018-05-12 1:01 ` David Lechner -1 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-12 1:01 UTC (permalink / raw) To: Sekhar Nori, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Kevin Hilman On 05/11/2018 09:10 AM, Sekhar Nori wrote: > PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot > be disabled. Mark it so to prevent unused clock disable > infrastructure from disabling it. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/clk/davinci/pll-dm646x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c > index eb96dd72b6b7..5bdf1cb5fda8 100644 > --- a/drivers/clk/davinci/pll-dm646x.c > +++ b/drivers/clk/davinci/pll-dm646x.c > @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = { > .flags = 0, > }; > > -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); > +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); > > int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip) > { > Reviewed-by: David Lechner <david@lechnology.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled @ 2018-05-12 1:01 ` David Lechner 0 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-12 1:01 UTC (permalink / raw) To: linux-arm-kernel On 05/11/2018 09:10 AM, Sekhar Nori wrote: > PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot > be disabled. Mark it so to prevent unused clock disable > infrastructure from disabling it. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/clk/davinci/pll-dm646x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c > index eb96dd72b6b7..5bdf1cb5fda8 100644 > --- a/drivers/clk/davinci/pll-dm646x.c > +++ b/drivers/clk/davinci/pll-dm646x.c > @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = { > .flags = 0, > }; > > -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); > +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); > > int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip) > { > Reviewed-by: David Lechner <david@lechnology.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled 2018-05-11 14:10 ` Sekhar Nori @ 2018-05-12 21:20 ` David Lechner -1 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-12 21:20 UTC (permalink / raw) To: Sekhar Nori, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Kevin Hilman On 05/11/2018 09:10 AM, Sekhar Nori wrote: > PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot > be disabled. Mark it so to prevent unused clock disable > infrastructure from disabling it. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/clk/davinci/pll-dm646x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c > index eb96dd72b6b7..5bdf1cb5fda8 100644 > --- a/drivers/clk/davinci/pll-dm646x.c > +++ b/drivers/clk/davinci/pll-dm646x.c > @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = { > .flags = 0, > }; > > -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); > +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); > > int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip) > { > FYI, this only applies on top of "clk: davinci: pll: allow dev == NULL". Not sure if that was intentional. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled @ 2018-05-12 21:20 ` David Lechner 0 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-12 21:20 UTC (permalink / raw) To: linux-arm-kernel On 05/11/2018 09:10 AM, Sekhar Nori wrote: > PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot > be disabled. Mark it so to prevent unused clock disable > infrastructure from disabling it. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/clk/davinci/pll-dm646x.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/clk/davinci/pll-dm646x.c b/drivers/clk/davinci/pll-dm646x.c > index eb96dd72b6b7..5bdf1cb5fda8 100644 > --- a/drivers/clk/davinci/pll-dm646x.c > +++ b/drivers/clk/davinci/pll-dm646x.c > @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info dm646x_pll2_info = { > .flags = 0, > }; > > -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); > +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); > > int dm646x_pll2_init(struct device *dev, void __iomem *base, struct regmap *cfgchip) > { > FYI, this only applies on top of "clk: davinci: pll: allow dev == NULL". Not sure if that was intentional. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled 2018-05-12 21:20 ` David Lechner @ 2018-05-14 10:05 ` Sekhar Nori -1 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-14 10:05 UTC (permalink / raw) To: David Lechner, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Kevin Hilman On Sunday 13 May 2018 02:50 AM, David Lechner wrote: > On 05/11/2018 09:10 AM, Sekhar Nori wrote: >> PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot >> be disabled. Mark it so to prevent unused clock disable >> infrastructure from disabling it. >> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> drivers/clk/davinci/pll-dm646x.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/davinci/pll-dm646x.c >> b/drivers/clk/davinci/pll-dm646x.c >> index eb96dd72b6b7..5bdf1cb5fda8 100644 >> --- a/drivers/clk/davinci/pll-dm646x.c >> +++ b/drivers/clk/davinci/pll-dm646x.c >> @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info >> dm646x_pll2_info = { >> .flags = 0, >> }; >> -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); >> +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); >> int dm646x_pll2_init(struct device *dev, void __iomem *base, >> struct regmap *cfgchip) >> { >> > > FYI, this only applies on top of "clk: davinci: pll: allow dev == NULL". > Not sure if that was intentional. Not actually. I will resend the series as it applies to v4.17-rc1. Thanks, Sekhar ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled @ 2018-05-14 10:05 ` Sekhar Nori 0 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-14 10:05 UTC (permalink / raw) To: linux-arm-kernel On Sunday 13 May 2018 02:50 AM, David Lechner wrote: > On 05/11/2018 09:10 AM, Sekhar Nori wrote: >> PLL2 SYSCLK1 on DM646x is connected to DDR2 PHY and cannot >> be disabled. Mark it so to prevent unused clock disable >> infrastructure from disabling it. >> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> ? drivers/clk/davinci/pll-dm646x.c | 2 +- >> ? 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/clk/davinci/pll-dm646x.c >> b/drivers/clk/davinci/pll-dm646x.c >> index eb96dd72b6b7..5bdf1cb5fda8 100644 >> --- a/drivers/clk/davinci/pll-dm646x.c >> +++ b/drivers/clk/davinci/pll-dm646x.c >> @@ -72,7 +72,7 @@ static const struct davinci_pll_clk_info >> dm646x_pll2_info = { >> ????? .flags = 0, >> ? }; >> ? -SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, 0); >> +SYSCLK(1, pll2_sysclk1, pll2_pllen, 4, SYSCLK_ALWAYS_ENABLED); >> ? ? int dm646x_pll2_init(struct device *dev, void __iomem *base, >> struct regmap *cfgchip) >> ? { >> > > FYI, this only applies on top of "clk: davinci: pll: allow dev == NULL". > Not sure if that was intentional. Not actually. I will resend the series as it applies to v4.17-rc1. Thanks, Sekhar ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks 2018-05-11 14:10 ` Sekhar Nori @ 2018-05-11 14:10 ` Sekhar Nori -1 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-11 14:10 UTC (permalink / raw) To: David Lechner, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Sekhar Nori, Kevin Hilman Fix parent of emac and voice codec PSC clocks. This now matches existing implementation in arch/arm/mach-davinci/dm365.c Also, there is only one power domain on DM365. Fix the power domain of voice codec and vpss dac modules. Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- drivers/clk/davinci/psc-dm365.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c index 5b5b55b0b59a..beeda10fd2f0 100644 --- a/drivers/clk/davinci/psc-dm365.c +++ b/drivers/clk/davinci/psc-dm365.c @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = { LPSC(31, 0, arm, pll2_sysclk2, NULL, LPSC_ALWAYS_ENABLED), LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0), LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0), - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0), - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0), + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0), { } -- 2.16.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks @ 2018-05-11 14:10 ` Sekhar Nori 0 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-11 14:10 UTC (permalink / raw) To: linux-arm-kernel Fix parent of emac and voice codec PSC clocks. This now matches existing implementation in arch/arm/mach-davinci/dm365.c Also, there is only one power domain on DM365. Fix the power domain of voice codec and vpss dac modules. Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- drivers/clk/davinci/psc-dm365.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c index 5b5b55b0b59a..beeda10fd2f0 100644 --- a/drivers/clk/davinci/psc-dm365.c +++ b/drivers/clk/davinci/psc-dm365.c @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = { LPSC(31, 0, arm, pll2_sysclk2, NULL, LPSC_ALWAYS_ENABLED), LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0), LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0), - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0), - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0), + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0), { } -- 2.16.2 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks 2018-05-11 14:10 ` Sekhar Nori @ 2018-05-12 1:42 ` David Lechner -1 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-12 1:42 UTC (permalink / raw) To: Sekhar Nori, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Kevin Hilman On 05/11/2018 09:10 AM, Sekhar Nori wrote: > Fix parent of emac and voice codec PSC clocks. This now matches > existing implementation in arch/arm/mach-davinci/dm365.c > > Also, there is only one power domain on DM365. Fix the power > domain of voice codec and vpss dac modules. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/clk/davinci/psc-dm365.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c > index 5b5b55b0b59a..beeda10fd2f0 100644 > --- a/drivers/clk/davinci/psc-dm365.c > +++ b/drivers/clk/davinci/psc-dm365.c > @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = { > LPSC(31, 0, arm, pll2_sysclk2, NULL, LPSC_ALWAYS_ENABLED), > LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0), > LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0), > - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0), > - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), > - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), > + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0), > + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), > + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), > LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), > LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0), > { } > Slightly off topic... Hmm... Looking at the TRM, I see that there are a bunch of mux clocks that we have not implemented for the DM365 (all of the clocks in the VPSS_CLK_CTRL and PERI_CLKCTL registers). I'm wondering if we should be creating drivers for those like the DA8XX CFGCHIP clock driver. Back on topic... The emac fix here looks good. The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5, It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however this dependency has probably gone unnoticed because so many other devices also use that same clock, it will pretty much always be on. In Figure 38, on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec. So, I am thinking that pll1_sysclk4 is the correct parent for the voice_codec clock here since it is the only one listed in Figure 38 (which is in the PSC section of the TRM). The pll2_sysclk4 clock should probably be used by the video codec device driver directly (well, the DIV2 clock really, but we don't have a driver for that yet). The vpss_dac clock in not very clear in the TRM. In Table 39, the name for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure 37, however, there is not a node that matches exactly. There is "VIDEO DAC", which I take to be the same clock though. This has parent clocks of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6 should be the parent here rather than pll1_sysclk3. According to Figure 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the same as the video DAC since they are also listed separately in Figure 5. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks @ 2018-05-12 1:42 ` David Lechner 0 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-12 1:42 UTC (permalink / raw) To: linux-arm-kernel On 05/11/2018 09:10 AM, Sekhar Nori wrote: > Fix parent of emac and voice codec PSC clocks. This now matches > existing implementation in arch/arm/mach-davinci/dm365.c > > Also, there is only one power domain on DM365. Fix the power > domain of voice codec and vpss dac modules. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > drivers/clk/davinci/psc-dm365.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/davinci/psc-dm365.c b/drivers/clk/davinci/psc-dm365.c > index 5b5b55b0b59a..beeda10fd2f0 100644 > --- a/drivers/clk/davinci/psc-dm365.c > +++ b/drivers/clk/davinci/psc-dm365.c > @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info dm365_psc_info[] = { > LPSC(31, 0, arm, pll2_sysclk2, NULL, LPSC_ALWAYS_ENABLED), > LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0), > LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0), > - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0), > - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), > - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), > + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0), > + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), > + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), > LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), > LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0), > { } > Slightly off topic... Hmm... Looking at the TRM, I see that there are a bunch of mux clocks that we have not implemented for the DM365 (all of the clocks in the VPSS_CLK_CTRL and PERI_CLKCTL registers). I'm wondering if we should be creating drivers for those like the DA8XX CFGCHIP clock driver. Back on topic... The emac fix here looks good. The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5, It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however this dependency has probably gone unnoticed because so many other devices also use that same clock, it will pretty much always be on. In Figure 38, on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec. So, I am thinking that pll1_sysclk4 is the correct parent for the voice_codec clock here since it is the only one listed in Figure 38 (which is in the PSC section of the TRM). The pll2_sysclk4 clock should probably be used by the video codec device driver directly (well, the DIV2 clock really, but we don't have a driver for that yet). The vpss_dac clock in not very clear in the TRM. In Table 39, the name for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure 37, however, there is not a node that matches exactly. There is "VIDEO DAC", which I take to be the same clock though. This has parent clocks of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6 should be the parent here rather than pll1_sysclk3. According to Figure 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the same as the video DAC since they are also listed separately in Figure 5. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks 2018-05-12 1:42 ` David Lechner @ 2018-05-14 9:49 ` Sekhar Nori -1 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-14 9:49 UTC (permalink / raw) To: David Lechner, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Kevin Hilman Hi David, On Saturday 12 May 2018 07:12 AM, David Lechner wrote: > On 05/11/2018 09:10 AM, Sekhar Nori wrote: >> Fix parent of emac and voice codec PSC clocks. This now matches >> existing implementation in arch/arm/mach-davinci/dm365.c >> >> Also, there is only one power domain on DM365. Fix the power >> domain of voice codec and vpss dac modules. >> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> drivers/clk/davinci/psc-dm365.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/clk/davinci/psc-dm365.c >> b/drivers/clk/davinci/psc-dm365.c >> index 5b5b55b0b59a..beeda10fd2f0 100644 >> --- a/drivers/clk/davinci/psc-dm365.c >> +++ b/drivers/clk/davinci/psc-dm365.c >> @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info >> dm365_psc_info[] = { >> LPSC(31, 0, arm, pll2_sysclk2, NULL, >> LPSC_ALWAYS_ENABLED), >> LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0), >> LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0), >> - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0), >> - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), >> - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), >> + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0), >> + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), >> + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), >> LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), >> LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0), >> { } >> > > Slightly off topic... > > Hmm... Looking at the TRM, I see that there are a bunch of mux clocks > that we > have not implemented for the DM365 (all of the clocks in the > VPSS_CLK_CTRL and > PERI_CLKCTL registers). I'm wondering if we should be creating drivers for > those like the DA8XX CFGCHIP clock driver. Yes, but lets leave that to after the current version is merged. That way we have one kernel version which is just replacing the private clock implementation and that will be good to bisect any regressions. VPSS_CLK_CTRL for example is being set directly by the VPSS driver. That driver will have to change too once we model the mux as a proper clock. > > Back on topic... > > The emac fix here looks good. Okay. > > The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the > PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5, > It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however > this dependency has probably gone unnoticed because so many other devices > also use that same clock, it will pretty much always be on. In Figure 38, > on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec. > > So, I am thinking that pll1_sysclk4 is the correct parent for the > voice_codec clock here since it is the only one listed in Figure 38 > (which is in the PSC section of the TRM). The pll2_sysclk4 clock should > probably be used by the video codec device driver directly (well, the > DIV2 clock really, but we don't have a driver for that yet). Well the main thing I went with here is compatibility with existing code which uses pll2_sysclk4. You are right that pll1_sysclk4 is probably a better parent clock to model for the PSC. But, this will break existing functionality as no one will be enabling pll2_sysclk4 then. And I suspect that will break voice codec driver. The documentation for voice codec does not seem to do a good job of explaining what the two clocks are for. And without some thorough testing of voice codec (I have never used it myself), I would just keep the functionality as-is. I can add a comment on how the parent was arrived at though, so there is some reference to when we look at this again. > > The vpss_dac clock in not very clear in the TRM. In Table 39, the name > for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure > 37, however, there is not a node that matches exactly. There is "VIDEO > DAC", which I take to be the same clock though. This has parent clocks > of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6 > should be the parent here rather than pll1_sysclk3. According to Figure > 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the > same as the video DAC since they are also listed separately in Figure 5. The HDVICP is a subsystem in itself, so I am not so sure about that. The problem here is compounded by the fact that some of these video subsystems are not publicly documented. Here too, I think the safer approach is to be compliant with existing code at least for one kernel version. Otherwise, we run into the issue of too many things changing at the same time making it tough to nail regressions. we can add a comment here too on how the parent was arrived at (refer back to existing code). Thanks, Sekhar ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks @ 2018-05-14 9:49 ` Sekhar Nori 0 siblings, 0 replies; 18+ messages in thread From: Sekhar Nori @ 2018-05-14 9:49 UTC (permalink / raw) To: linux-arm-kernel Hi David, On Saturday 12 May 2018 07:12 AM, David Lechner wrote: > On 05/11/2018 09:10 AM, Sekhar Nori wrote: >> Fix parent of emac and voice codec PSC clocks. This now matches >> existing implementation in arch/arm/mach-davinci/dm365.c >> >> Also, there is only one power domain on DM365. Fix the power >> domain of voice codec and vpss dac modules. >> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> ? drivers/clk/davinci/psc-dm365.c | 6 +++--- >> ? 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/clk/davinci/psc-dm365.c >> b/drivers/clk/davinci/psc-dm365.c >> index 5b5b55b0b59a..beeda10fd2f0 100644 >> --- a/drivers/clk/davinci/psc-dm365.c >> +++ b/drivers/clk/davinci/psc-dm365.c >> @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info >> dm365_psc_info[] = { >> ????? LPSC(31, 0, arm,???????? pll2_sysclk2, NULL,?????????????? >> LPSC_ALWAYS_ENABLED), >> ????? LPSC(38, 0, spi3,??????? pll1_sysclk4, spi3_clkdev,??????? 0), >> ????? LPSC(39, 0, spi4,??????? pll1_auxclk,? spi4_clkdev,??????? 0), >> -??? LPSC(40, 0, emac,??????? pll2_sysclk4, emac_clkdev,??????? 0), >> -??? LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), >> -??? LPSC(46, 1, vpss_dac,??? pll1_sysclk3, vpss_dac_clkdev,??? 0), >> +??? LPSC(40, 0, emac,??????? pll1_sysclk4, emac_clkdev,??????? 0), >> +??? LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), >> +??? LPSC(46, 0, vpss_dac,??? pll1_sysclk3, vpss_dac_clkdev,??? 0), >> ????? LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), >> ????? LPSC(50, 0, mjcp,??????? pll1_sysclk3, NULL,?????????????? 0), >> ????? { } >> > > Slightly off topic... > > Hmm... Looking at the TRM, I see that there are a bunch of mux clocks > that we > have not implemented for the DM365 (all of the clocks in the > VPSS_CLK_CTRL and > PERI_CLKCTL registers). I'm wondering if we should be creating drivers for > those like the DA8XX CFGCHIP clock driver. Yes, but lets leave that to after the current version is merged. That way we have one kernel version which is just replacing the private clock implementation and that will be good to bisect any regressions. VPSS_CLK_CTRL for example is being set directly by the VPSS driver. That driver will have to change too once we model the mux as a proper clock. > > Back on topic... > > The emac fix here looks good. Okay. > > The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the > PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5, > It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however > this dependency has probably gone unnoticed because so many other devices > also use that same clock, it will pretty much always be on. In Figure 38, > on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec. > > So, I am thinking that pll1_sysclk4 is the correct parent for the > voice_codec clock here since it is the only one listed in Figure 38 > (which is in the PSC section of the TRM). The pll2_sysclk4 clock should > probably be used by the video codec device driver directly (well, the > DIV2 clock really, but we don't have a driver for that yet). Well the main thing I went with here is compatibility with existing code which uses pll2_sysclk4. You are right that pll1_sysclk4 is probably a better parent clock to model for the PSC. But, this will break existing functionality as no one will be enabling pll2_sysclk4 then. And I suspect that will break voice codec driver. The documentation for voice codec does not seem to do a good job of explaining what the two clocks are for. And without some thorough testing of voice codec (I have never used it myself), I would just keep the functionality as-is. I can add a comment on how the parent was arrived at though, so there is some reference to when we look at this again. > > The vpss_dac clock in not very clear in the TRM. In Table 39, the name > for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure > 37, however, there is not a node that matches exactly. There is "VIDEO > DAC", which I take to be the same clock though. This has parent clocks > of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6 > should be the parent here rather than pll1_sysclk3. According to Figure > 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the > same as the video DAC since they are also listed separately in Figure 5. The HDVICP is a subsystem in itself, so I am not so sure about that. The problem here is compounded by the fact that some of these video subsystems are not publicly documented. Here too, I think the safer approach is to be compliant with existing code at least for one kernel version. Otherwise, we run into the issue of too many things changing at the same time making it tough to nail regressions. we can add a comment here too on how the parent was arrived at (refer back to existing code). Thanks, Sekhar ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks 2018-05-14 9:49 ` Sekhar Nori @ 2018-05-14 15:19 ` David Lechner -1 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-14 15:19 UTC (permalink / raw) To: Sekhar Nori, Michael Turquette, Stephen Boyd Cc: Linux clk Mailing List, Linux ARM Mailing List, Kevin Hilman On 05/14/2018 04:49 AM, Sekhar Nori wrote: > Hi David, > > On Saturday 12 May 2018 07:12 AM, David Lechner wrote: >> On 05/11/2018 09:10 AM, Sekhar Nori wrote: >>> Fix parent of emac and voice codec PSC clocks. This now matches >>> existing implementation in arch/arm/mach-davinci/dm365.c >>> >>> Also, there is only one power domain on DM365. Fix the power >>> domain of voice codec and vpss dac modules. >>> >>> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >>> --- >>> drivers/clk/davinci/psc-dm365.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/clk/davinci/psc-dm365.c >>> b/drivers/clk/davinci/psc-dm365.c >>> index 5b5b55b0b59a..beeda10fd2f0 100644 >>> --- a/drivers/clk/davinci/psc-dm365.c >>> +++ b/drivers/clk/davinci/psc-dm365.c >>> @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info >>> dm365_psc_info[] = { >>> LPSC(31, 0, arm, pll2_sysclk2, NULL, >>> LPSC_ALWAYS_ENABLED), >>> LPSC(38, 0, spi3, pll1_sysclk4, spi3_clkdev, 0), >>> LPSC(39, 0, spi4, pll1_auxclk, spi4_clkdev, 0), >>> - LPSC(40, 0, emac, pll2_sysclk4, emac_clkdev, 0), >>> - LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), >>> - LPSC(46, 1, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), >>> + LPSC(40, 0, emac, pll1_sysclk4, emac_clkdev, 0), >>> + LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), >>> + LPSC(46, 0, vpss_dac, pll1_sysclk3, vpss_dac_clkdev, 0), >>> LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), >>> LPSC(50, 0, mjcp, pll1_sysclk3, NULL, 0), >>> { } >>> >> >> Slightly off topic... >> >> Hmm... Looking at the TRM, I see that there are a bunch of mux clocks >> that we >> have not implemented for the DM365 (all of the clocks in the >> VPSS_CLK_CTRL and >> PERI_CLKCTL registers). I'm wondering if we should be creating drivers for >> those like the DA8XX CFGCHIP clock driver. > > Yes, but lets leave that to after the current version is merged. That > way we have one kernel version which is just replacing the private clock > implementation and that will be good to bisect any regressions. > VPSS_CLK_CTRL for example is being set directly by the VPSS driver. That > driver will have to change too once we model the mux as a proper clock. > >> >> Back on topic... >> >> The emac fix here looks good. > > Okay. > >> >> The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the >> PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5, >> It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however >> this dependency has probably gone unnoticed because so many other devices >> also use that same clock, it will pretty much always be on. In Figure 38, >> on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec. >> >> So, I am thinking that pll1_sysclk4 is the correct parent for the >> voice_codec clock here since it is the only one listed in Figure 38 >> (which is in the PSC section of the TRM). The pll2_sysclk4 clock should >> probably be used by the video codec device driver directly (well, the >> DIV2 clock really, but we don't have a driver for that yet). > > Well the main thing I went with here is compatibility with existing code > which uses pll2_sysclk4. You are right that pll1_sysclk4 is probably a > better parent clock to model for the PSC. But, this will break existing > functionality as no one will be enabling pll2_sysclk4 then. And I > suspect that will break voice codec driver. The documentation for voice > codec does not seem to do a good job of explaining what the two clocks > are for. And without some thorough testing of voice codec (I have never > used it myself), I would just keep the functionality as-is. > > I can add a comment on how the parent was arrived at though, so there is > some reference to when we look at this again. > >> >> The vpss_dac clock in not very clear in the TRM. In Table 39, the name >> for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure >> 37, however, there is not a node that matches exactly. There is "VIDEO >> DAC", which I take to be the same clock though. This has parent clocks >> of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6 >> should be the parent here rather than pll1_sysclk3. According to Figure >> 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the >> same as the video DAC since they are also listed separately in Figure 5. > > The HDVICP is a subsystem in itself, so I am not so sure about that. The > problem here is compounded by the fact that some of these video > subsystems are not publicly documented. Here too, I think the safer > approach is to be compliant with existing code at least for one kernel > version. Otherwise, we run into the issue of too many things changing at > the same time making it tough to nail regressions. > > we can add a comment here too on how the parent was arrived at (refer > back to existing code). > > Thanks, > Sekhar > Making it just match the existing clock code sounds like a good plan to me. And it would be nice to add the suggested comments since you will be doing a v2 for the other patch anyway. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks @ 2018-05-14 15:19 ` David Lechner 0 siblings, 0 replies; 18+ messages in thread From: David Lechner @ 2018-05-14 15:19 UTC (permalink / raw) To: linux-arm-kernel On 05/14/2018 04:49 AM, Sekhar Nori wrote: > Hi David, > > On Saturday 12 May 2018 07:12 AM, David Lechner wrote: >> On 05/11/2018 09:10 AM, Sekhar Nori wrote: >>> Fix parent of emac and voice codec PSC clocks. This now matches >>> existing implementation in arch/arm/mach-davinci/dm365.c >>> >>> Also, there is only one power domain on DM365. Fix the power >>> domain of voice codec and vpss dac modules. >>> >>> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >>> --- >>> ? drivers/clk/davinci/psc-dm365.c | 6 +++--- >>> ? 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/clk/davinci/psc-dm365.c >>> b/drivers/clk/davinci/psc-dm365.c >>> index 5b5b55b0b59a..beeda10fd2f0 100644 >>> --- a/drivers/clk/davinci/psc-dm365.c >>> +++ b/drivers/clk/davinci/psc-dm365.c >>> @@ -65,9 +65,9 @@ static const struct davinci_lpsc_clk_info >>> dm365_psc_info[] = { >>> ????? LPSC(31, 0, arm,???????? pll2_sysclk2, NULL, >>> LPSC_ALWAYS_ENABLED), >>> ????? LPSC(38, 0, spi3,??????? pll1_sysclk4, spi3_clkdev,??????? 0), >>> ????? LPSC(39, 0, spi4,??????? pll1_auxclk,? spi4_clkdev,??????? 0), >>> -??? LPSC(40, 0, emac,??????? pll2_sysclk4, emac_clkdev,??????? 0), >>> -??? LPSC(44, 1, voice_codec, pll1_sysclk3, voice_codec_clkdev, 0), >>> -??? LPSC(46, 1, vpss_dac,??? pll1_sysclk3, vpss_dac_clkdev,??? 0), >>> +??? LPSC(40, 0, emac,??????? pll1_sysclk4, emac_clkdev,??????? 0), >>> +??? LPSC(44, 0, voice_codec, pll2_sysclk4, voice_codec_clkdev, 0), >>> +??? LPSC(46, 0, vpss_dac,??? pll1_sysclk3, vpss_dac_clkdev,??? 0), >>> ????? LPSC(47, 0, vpss_master, pll1_sysclk5, vpss_master_clkdev, 0), >>> ????? LPSC(50, 0, mjcp,??????? pll1_sysclk3, NULL,?????????????? 0), >>> ????? { } >>> >> >> Slightly off topic... >> >> Hmm... Looking at the TRM, I see that there are a bunch of mux clocks >> that we >> have not implemented for the DM365 (all of the clocks in the >> VPSS_CLK_CTRL and >> PERI_CLKCTL registers). I'm wondering if we should be creating drivers for >> those like the DA8XX CFGCHIP clock driver. > > Yes, but lets leave that to after the current version is merged. That > way we have one kernel version which is just replacing the private clock > implementation and that will be good to bisect any regressions. > VPSS_CLK_CTRL for example is being set directly by the VPSS driver. That > driver will have to change too once we model the mux as a proper clock. > >> >> Back on topic... >> >> The emac fix here looks good. > > Okay. > >> >> The TRM (sprufg5a.pdf) shows that there is a DIV2 clock (part of the >> PERI_CLKCTL register) between PLL2 SYSCLK4 and Voice Codec in Figure 5, >> It also shows PLL1 SYSCLK4 as a second parent clock to Voice Codec, however >> this dependency has probably gone unnoticed because so many other devices >> also use that same clock, it will pretty much always be on. In Figure 38, >> on the other hand, PLL1 SYSCLK4 is shown as the only parent of Voice Codec. >> >> So, I am thinking that pll1_sysclk4 is the correct parent for the >> voice_codec clock here since it is the only one listed in Figure 38 >> (which is in the PSC section of the TRM). The pll2_sysclk4 clock should >> probably be used by the video codec device driver directly (well, the >> DIV2 clock really, but we don't have a driver for that yet). > > Well the main thing I went with here is compatibility with existing code > which uses pll2_sysclk4. You are right that pll1_sysclk4 is probably a > better parent clock to model for the PSC. But, this will break existing > functionality as no one will be enabling pll2_sysclk4 then. And I > suspect that will break voice codec driver. The documentation for voice > codec does not seem to do a good job of explaining what the two clocks > are for. And without some thorough testing of voice codec (I have never > used it myself), I would just keep the functionality as-is. > > I can add a comment on how the parent was arrived at though, so there is > some reference to when we look at this again. > >> >> The vpss_dac clock in not very clear in the TRM. In Table 39, the name >> for LPSC 46, which we are calling "vpss_dac", is "VDAC CLK". In Figure >> 37, however, there is not a node that matches exactly. There is "VIDEO >> DAC", which I take to be the same clock though. This has parent clocks >> of PLL1 SYSCLK6 and PLL1 AUXCLK. So, I am guessing that pll1_sysclk6 >> should be the parent here rather than pll1_sysclk3. According to Figure >> 5, PLL1 SYSCLK3 only feeds HDVPID and MJCP, which I don't think are the >> same as the video DAC since they are also listed separately in Figure 5. > > The HDVICP is a subsystem in itself, so I am not so sure about that. The > problem here is compounded by the fact that some of these video > subsystems are not publicly documented. Here too, I think the safer > approach is to be compliant with existing code at least for one kernel > version. Otherwise, we run into the issue of too many things changing at > the same time making it tough to nail regressions. > > we can add a comment here too on how the parent was arrived at (refer > back to existing code). > > Thanks, > Sekhar > Making it just match the existing clock code sounds like a good plan to me. And it would be nice to add the suggested comments since you will be doing a v2 for the other patch anyway. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-05-14 15:19 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-11 14:10 [PATCH 0/2] clk: davinci: some more fixes Sekhar Nori 2018-05-11 14:10 ` Sekhar Nori 2018-05-11 14:10 ` [PATCH 1/2] clk: davinci: pll-dm646x: keep PLL2 SYSCLK1 always enabled Sekhar Nori 2018-05-11 14:10 ` Sekhar Nori 2018-05-12 1:01 ` David Lechner 2018-05-12 1:01 ` David Lechner 2018-05-12 21:20 ` David Lechner 2018-05-12 21:20 ` David Lechner 2018-05-14 10:05 ` Sekhar Nori 2018-05-14 10:05 ` Sekhar Nori 2018-05-11 14:10 ` [PATCH 2/2] clk: davinci: psc-dm365: fix few clocks Sekhar Nori 2018-05-11 14:10 ` Sekhar Nori 2018-05-12 1:42 ` David Lechner 2018-05-12 1:42 ` David Lechner 2018-05-14 9:49 ` Sekhar Nori 2018-05-14 9:49 ` Sekhar Nori 2018-05-14 15:19 ` David Lechner 2018-05-14 15:19 ` David Lechner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.