| Message ID | 1467750561-13957-2-git-send-email-a.kesavan@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/05/2016 10:29 PM, Abhilash Kesavan wrote: > Exynos7 has the same CPU clock registers layout as that present > in Exynos5433 except for the bits in the MUX_STAT* registers. > Add a new CLK_CPU_HAS_MODIFIED_MUX_STAT flag to handle this change. > --- a/drivers/clk/samsung/clk-cpu.h > +++ b/drivers/clk/samsung/clk-cpu.h > @@ -63,6 +63,8 @@ struct exynos_cpuclk { > /* The CPU clock registers have Exynos5433-compatible layout */ > #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) > +/* Exynos5433-compatible layout with different MUX_STAT register bits */ > +#define CLK_CPU_HAS_MODIFIED_MUX_STAT (1 << 3) It's getting a bit messy, what if there comes another SoC version which has some other modification of exynos5433 registers structure? We would need another variant of HAS_MODIFIED_MUX_STAT flag and we could easily get lost while trying to determine which modification is which. How about indicating explicitly it's an exynos7 bits layout and renaming the flag to something like #define CLK_CPU_HAS_E7_MUX_STAT (1 << 16) ?
Hi On Thursday, July 07, 2016 12:45:57 PM Sylwester Nawrocki wrote: > On 07/05/2016 10:29 PM, Abhilash Kesavan wrote: > > Exynos7 has the same CPU clock registers layout as that present Please precise for which Exynos7 SoC this change is needed (all three of them?). > > in Exynos5433 except for the bits in the MUX_STAT* registers. > > Add a new CLK_CPU_HAS_MODIFIED_MUX_STAT flag to handle this change. > > > --- a/drivers/clk/samsung/clk-cpu.h > > +++ b/drivers/clk/samsung/clk-cpu.h > > @@ -63,6 +63,8 @@ struct exynos_cpuclk { > > /* The CPU clock registers have Exynos5433-compatible layout */ > > #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) > > > +/* Exynos5433-compatible layout with different MUX_STAT register bits */ > > +#define CLK_CPU_HAS_MODIFIED_MUX_STAT (1 << 3) > > It's getting a bit messy, what if there comes another SoC version > which has some other modification of exynos5433 registers structure? > We would need another variant of HAS_MODIFIED_MUX_STAT flag and we > could easily get lost while trying to determine which modification > is which. How about indicating explicitly it's an exynos7 bits > layout and renaming the flag to something like > > #define CLK_CPU_HAS_E7_MUX_STAT (1 << 16) ? ditto Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Hi Sylwester, On Thu, Jul 7, 2016 at 12:45 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > On 07/05/2016 10:29 PM, Abhilash Kesavan wrote: >> Exynos7 has the same CPU clock registers layout as that present >> in Exynos5433 except for the bits in the MUX_STAT* registers. >> Add a new CLK_CPU_HAS_MODIFIED_MUX_STAT flag to handle this change. > >> --- a/drivers/clk/samsung/clk-cpu.h >> +++ b/drivers/clk/samsung/clk-cpu.h >> @@ -63,6 +63,8 @@ struct exynos_cpuclk { >> /* The CPU clock registers have Exynos5433-compatible layout */ >> #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) > >> +/* Exynos5433-compatible layout with different MUX_STAT register bits */ >> +#define CLK_CPU_HAS_MODIFIED_MUX_STAT (1 << 3) > > It's getting a bit messy, what if there comes another SoC version > which has some other modification of exynos5433 registers structure? > We would need another variant of HAS_MODIFIED_MUX_STAT flag and we > could easily get lost while trying to determine which modification > is which. How about indicating explicitly it's an exynos7 bits > layout and renaming the flag to something like > > #define CLK_CPU_HAS_E7_MUX_STAT (1 << 16) ? Will rename the flag to make it SoC specific as per your suggestion. Regards, Abhilash
Hi Bartlomiej, Thanks for the comments. On Thu, Jul 7, 2016 at 1:17 PM, Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> wrote: > > Hi > > On Thursday, July 07, 2016 12:45:57 PM Sylwester Nawrocki wrote: >> On 07/05/2016 10:29 PM, Abhilash Kesavan wrote: >> > Exynos7 has the same CPU clock registers layout as that present > > Please precise for which Exynos7 SoC this change is needed > (all three of them?). As mentioned in my recently posted PMU series, the exynos7 is a quad core A57 based SoC and not meant to be a SoC family. I have reviewed various exynos7xxx UMs in terms of the CPU CMU. Both exynos7580 and exynos7420 have a similar CMU register layout along with the same mux stat bits as exynos7. Exynos7870 on the other hand is quite different. Please let me know what naming convention you would prefer that I use E7/E7420/E7580 ? > >> > in Exynos5433 except for the bits in the MUX_STAT* registers. >> > Add a new CLK_CPU_HAS_MODIFIED_MUX_STAT flag to handle this change. >> >> > --- a/drivers/clk/samsung/clk-cpu.h >> > +++ b/drivers/clk/samsung/clk-cpu.h >> > @@ -63,6 +63,8 @@ struct exynos_cpuclk { >> > /* The CPU clock registers have Exynos5433-compatible layout */ >> > #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) >> >> > +/* Exynos5433-compatible layout with different MUX_STAT register bits */ >> > +#define CLK_CPU_HAS_MODIFIED_MUX_STAT (1 << 3) >> >> It's getting a bit messy, what if there comes another SoC version >> which has some other modification of exynos5433 registers structure? >> We would need another variant of HAS_MODIFIED_MUX_STAT flag and we >> could easily get lost while trying to determine which modification >> is which. How about indicating explicitly it's an exynos7 bits >> layout and renaming the flag to something like >> >> #define CLK_CPU_HAS_E7_MUX_STAT (1 << 16) ? > > ditto > > Best regards, > -- > Bartlomiej Zolnierkiewicz > Samsung R&D Institute Poland > Samsung Electronics >
On Monday, July 11, 2016 04:44:30 PM Abhilash Kesavan wrote: > Hi Bartlomiej, Hi Abhilash, > Thanks for the comments. > > On Thu, Jul 7, 2016 at 1:17 PM, Bartlomiej Zolnierkiewicz > <b.zolnierkie@samsung.com> wrote: > > > > Hi > > > > On Thursday, July 07, 2016 12:45:57 PM Sylwester Nawrocki wrote: > >> On 07/05/2016 10:29 PM, Abhilash Kesavan wrote: > >> > Exynos7 has the same CPU clock registers layout as that present > > > > Please precise for which Exynos7 SoC this change is needed > > (all three of them?). > > As mentioned in my recently posted PMU series, the exynos7 is a quad > core A57 based SoC and not meant to be a SoC family. I suppose that it will get some more meaningful name once released officially (I couldn't find anything about this SoC on the net). > I have reviewed various exynos7xxx UMs in terms of the CPU CMU. Both > exynos7580 and exynos7420 have a similar CMU register layout along > with the same mux stat bits as exynos7. Exynos7870 on the other hand > is quite different. > > Please let me know what naming convention you would prefer that I use > E7/E7420/E7580 ? I would prefer using E7420 naming. > >> > in Exynos5433 except for the bits in the MUX_STAT* registers. > >> > Add a new CLK_CPU_HAS_MODIFIED_MUX_STAT flag to handle this change. > >> > >> > --- a/drivers/clk/samsung/clk-cpu.h > >> > +++ b/drivers/clk/samsung/clk-cpu.h > >> > @@ -63,6 +63,8 @@ struct exynos_cpuclk { > >> > /* The CPU clock registers have Exynos5433-compatible layout */ > >> > #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) > >> > >> > +/* Exynos5433-compatible layout with different MUX_STAT register bits */ > >> > +#define CLK_CPU_HAS_MODIFIED_MUX_STAT (1 << 3) > >> > >> It's getting a bit messy, what if there comes another SoC version > >> which has some other modification of exynos5433 registers structure? > >> We would need another variant of HAS_MODIFIED_MUX_STAT flag and we > >> could easily get lost while trying to determine which modification > >> is which. How about indicating explicitly it's an exynos7 bits > >> layout and renaming the flag to something like > >> > >> #define CLK_CPU_HAS_E7_MUX_STAT (1 << 16) ? > > > > ditto Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c index 8bf7e80..d40d740 100644 --- a/drivers/clk/samsung/clk-cpu.c +++ b/drivers/clk/samsung/clk-cpu.c @@ -322,7 +322,10 @@ static int exynos5433_cpuclk_pre_rate_change(struct clk_notifier_data *ndata, /* select the alternate parent */ mux_reg = readl(base + E5433_MUX_SEL2); writel(mux_reg | 1, base + E5433_MUX_SEL2); - wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 2); + if (cpuclk->flags & CLK_CPU_HAS_MODIFIED_MUX_STAT) + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 1); + else + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 2); /* alternate parent is active now. set the dividers */ writel(div0, base + E5433_DIV_CPU0); @@ -348,7 +351,10 @@ static int exynos5433_cpuclk_post_rate_change(struct clk_notifier_data *ndata, /* select apll as the alternate parent */ mux_reg = readl(base + E5433_MUX_SEL2); writel(mux_reg & ~1, base + E5433_MUX_SEL2); - wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 1); + if (cpuclk->flags & CLK_CPU_HAS_MODIFIED_MUX_STAT) + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 0); + else + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 1); exynos5433_set_safe_div(base, div, div_mask); spin_unlock_irqrestore(cpuclk->lock, flags); diff --git a/drivers/clk/samsung/clk-cpu.h b/drivers/clk/samsung/clk-cpu.h index d4b6b51..b4d9a4b 100644 --- a/drivers/clk/samsung/clk-cpu.h +++ b/drivers/clk/samsung/clk-cpu.h @@ -63,6 +63,8 @@ struct exynos_cpuclk { #define CLK_CPU_NEEDS_DEBUG_ALT_DIV (1 << 1) /* The CPU clock registers have Exynos5433-compatible layout */ #define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) +/* Exynos5433-compatible layout with different MUX_STAT register bits */ +#define CLK_CPU_HAS_MODIFIED_MUX_STAT (1 << 3) }; extern int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
Exynos7 has the same CPU clock registers layout as that present in Exynos5433 except for the bits in the MUX_STAT* registers. Add a new CLK_CPU_HAS_MODIFIED_MUX_STAT flag to handle this change. Signed-off-by: Abhilash Kesavan <a.kesavan@samsung.com> --- drivers/clk/samsung/clk-cpu.c | 10 ++++++++-- drivers/clk/samsung/clk-cpu.h | 2 ++ 2 files changed, 10 insertions(+), 2 deletions(-)