| Message ID | 1416307650-15162-2-git-send-email-pankaj.dubey@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 18/11/14 11:47, Pankaj Dubey wrote: > Let's register restart handler for Exynos5440 from it's clock driver > for restart functionality. So that we can cleanup restart hooks from > machine specific file. > > CC: Sylwester Nawrocki <s.nawrocki@samsung.com> > CC: Tomasz Figa <tomasz.figa@gmail.com> > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > Acked-by: Guenter Roeck <linux@roeck-us.net> The patch looks good to me. Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> Just one comment below. > --- > arch/arm/mach-exynos/exynos.c | 19 +------------------ > drivers/clk/samsung/clk-exynos5440.c | 29 ++++++++++++++++++++++++++++- > 2 files changed, 29 insertions(+), 19 deletions(-) > > diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c > index 8f638ad..8f995b7 100644 > --- a/arch/arm/mach-exynos/exynos.c > +++ b/arch/arm/mach-exynos/exynos.c > @@ -89,24 +89,7 @@ static struct map_desc exynos5_iodesc[] __initdata = { > > static void exynos_restart(enum reboot_mode mode, const char *cmd) > { > - struct device_node *np; > - u32 val = 0x1; > - void __iomem *addr = pmu_base_addr + EXYNOS_SWRESET; > - > - if (of_machine_is_compatible("samsung,exynos5440")) { > - u32 status; > - np = of_find_compatible_node(NULL, NULL, "samsung,exynos5440-clock"); > - > - addr = of_iomap(np, 0) + 0xbc; > - status = __raw_readl(addr); > - > - addr = of_iomap(np, 0) + 0xcc; > - val = __raw_readl(addr); > - > - val = (val & 0xffff0000) | (status & 0xffff); > - } > - > - __raw_writel(val, addr); > + __raw_writel(0x1, pmu_base_addr + EXYNOS_SWRESET); > } > > +static int exynos5440_clk_restart_notify(struct notifier_block *this, > + unsigned long code, void *unused) > +{ > + u32 val, status; > + > + status = readl_relaxed(reg_base + 0xbc); > + val = readl_relaxed(reg_base + 0xcc); > + val = (val & 0xffff0000) | (status & 0xffff); > + writel_relaxed(val, reg_base + 0xcc); Can we have macro definitions for these 0xcc, 0xbc address offsets ? I must say I couldn't find them documented in any Exynos datasheet I've got though. > + return NOTIFY_DONE; > +} -- Regards, Sylwester
On Tuesday, November 18, 2014 8:47 PM, Sylwester Nawrocki Wrote: > On 18/11/14 11:47, Pankaj Dubey wrote: > > Let's register restart handler for Exynos5440 from it's clock driver > > for restart functionality. So that we can cleanup restart hooks from > > machine specific file. > > > > CC: Sylwester Nawrocki <s.nawrocki@samsung.com> > > CC: Tomasz Figa <tomasz.figa@gmail.com> > > Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com> > > Acked-by: Guenter Roeck <linux@roeck-us.net> > > The patch looks good to me. > > Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > Just one comment below. > Thanks for review. > > --- > > arch/arm/mach-exynos/exynos.c | 19 +------------------ > > drivers/clk/samsung/clk-exynos5440.c | 29 > ++++++++++++++++++++++++++++- > > 2 files changed, 29 insertions(+), 19 deletions(-) > > > > diff --git a/arch/arm/mach-exynos/exynos.c > > b/arch/arm/mach-exynos/exynos.c index 8f638ad..8f995b7 100644 > > --- a/arch/arm/mach-exynos/exynos.c > > +++ b/arch/arm/mach-exynos/exynos.c > > @@ -89,24 +89,7 @@ static struct map_desc exynos5_iodesc[] __initdata > > = { > > > > static void exynos_restart(enum reboot_mode mode, const char *cmd) { > > - struct device_node *np; > > - u32 val = 0x1; > > - void __iomem *addr = pmu_base_addr + EXYNOS_SWRESET; > > - > > - if (of_machine_is_compatible("samsung,exynos5440")) { > > - u32 status; > > - np = of_find_compatible_node(NULL, NULL, "samsung,exynos5440- > clock"); > > - > > - addr = of_iomap(np, 0) + 0xbc; > > - status = __raw_readl(addr); > > - > > - addr = of_iomap(np, 0) + 0xcc; > > - val = __raw_readl(addr); > > - > > - val = (val & 0xffff0000) | (status & 0xffff); > > - } > > - > > - __raw_writel(val, addr); > > + __raw_writel(0x1, pmu_base_addr + EXYNOS_SWRESET); > > } > > > > > +static int exynos5440_clk_restart_notify(struct notifier_block *this, > > + unsigned long code, void *unused) > > +{ > > + u32 val, status; > > + > > + status = readl_relaxed(reg_base + 0xbc); > > + val = readl_relaxed(reg_base + 0xcc); > > + val = (val & 0xffff0000) | (status & 0xffff); > > + writel_relaxed(val, reg_base + 0xcc); > > Can we have macro definitions for these 0xcc, 0xbc address offsets ? > I must say I couldn't find them documented in any Exynos datasheet I've got though. > I also wished this, but I could not find them documented. So I tried to keep logic of original code as it is, just changed location. I would also like to mention that I have not tested this on exynos5440 as I do not have one with me. I believe if it was working at its original place in exynos_restart it should work here also. Other patch (2/2) I have verified on Exynos3250 board and its working well. Thanks, Pankaj Dubey > > + return NOTIFY_DONE; > > +} > > -- > Regards, > Sylwester
On 19/11/14 04:37, Pankaj Dubey wrote: >> > >>> > > +static int exynos5440_clk_restart_notify(struct notifier_block *this, >>> > > + unsigned long code, void *unused) >>> > > +{ >>> > > + u32 val, status; >>> > > + >>> > > + status = readl_relaxed(reg_base + 0xbc); >>> > > + val = readl_relaxed(reg_base + 0xcc); >>> > > + val = (val & 0xffff0000) | (status & 0xffff); >>> > > + writel_relaxed(val, reg_base + 0xcc); >> > >> > Can we have macro definitions for these 0xcc, 0xbc address offsets ? >> > I must say I couldn't find them documented in any Exynos datasheet I've > got though. >> > > > I also wished this, but I could not find them documented. > So I tried to keep logic of original code as it is, just changed location. > I would also like to mention that I have not tested this on exynos5440 as I > do not have > one with me. I believe if it was working at its original place in > exynos_restart it should work > here also. Other patch (2/2) I have verified on Exynos3250 board and its > working well. I think it's best to merge both patches in that series through the arm-soc tree, since applying them not in order may cause some breakage. Thus I'd let Kukjin take this patch set into his tree. For both patches: Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> -- Thanks, Sylwester
Hi Kukjin, On Thursday 20 November 2014 11:18 PM, Sylwester Nawrocki wrote: > On 19/11/14 04:37, Pankaj Dubey wrote: >>>> >>>>>> +static int exynos5440_clk_restart_notify(struct notifier_block *this, >>>>>> + unsigned long code, void *unused) >>>>>> +{ >>>>>> + u32 val, status; >>>>>> + >>>>>> + status = readl_relaxed(reg_base + 0xbc); >>>>>> + val = readl_relaxed(reg_base + 0xcc); >>>>>> + val = (val & 0xffff0000) | (status & 0xffff); >>>>>> + writel_relaxed(val, reg_base + 0xcc); >>>> >>>> Can we have macro definitions for these 0xcc, 0xbc address offsets ? >>>> I must say I couldn't find them documented in any Exynos datasheet I've >> got though. >>>> >> >> I also wished this, but I could not find them documented. >> So I tried to keep logic of original code as it is, just changed location. >> I would also like to mention that I have not tested this on exynos5440 as I >> do not have >> one with me. I believe if it was working at its original place in >> exynos_restart it should work >> here also. Other patch (2/2) I have verified on Exynos3250 board and its >> working well. > > I think it's best to merge both patches in that series through > the arm-soc tree, since applying them not in order may cause some > breakage. Thus I'd let Kukjin take this patch set into his tree. > > For both patches: > Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > Will you please take this series into your tree. Thanks, Pankaj Dubey > -- > Thanks, > Sylwester >
Pankaj Dubey wrote: > > Hi Kukjin, > Hi, > On Thursday 20 November 2014 11:18 PM, Sylwester Nawrocki wrote: > > On 19/11/14 04:37, Pankaj Dubey wrote: > >>>> > >>>>>> +static int exynos5440_clk_restart_notify(struct notifier_block *this, > >>>>>> + unsigned long code, void *unused) > >>>>>> +{ > >>>>>> + u32 val, status; > >>>>>> + > >>>>>> + status = readl_relaxed(reg_base + 0xbc); > >>>>>> + val = readl_relaxed(reg_base + 0xcc); > >>>>>> + val = (val & 0xffff0000) | (status & 0xffff); > >>>>>> + writel_relaxed(val, reg_base + 0xcc); > >>>> > >>>> Can we have macro definitions for these 0xcc, 0xbc address offsets ? > >>>> I must say I couldn't find them documented in any Exynos datasheet I've > >> got though. > >>>> > >> > >> I also wished this, but I could not find them documented. > >> So I tried to keep logic of original code as it is, just changed location. > >> I would also like to mention that I have not tested this on exynos5440 as I > >> do not have > >> one with me. I believe if it was working at its original place in > >> exynos_restart it should work > >> here also. Other patch (2/2) I have verified on Exynos3250 board and its > >> working well. > > > > I think it's best to merge both patches in that series through > > the arm-soc tree, since applying them not in order may cause some > > breakage. Thus I'd let Kukjin take this patch set into his tree. > > > > For both patches: > > Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> > > Sylwester, thanks for your ack. > > Will you please take this series into your tree. > + Amit Daniel Yeah, this series looks good to me and I'm looking at this series with Amit's moving pmu.c into drivers/. Maybe I need to ask Amit to respin the series on top of this? If required, I'll reply. Thanks, Kukjin
On Sat, Nov 22, 2014 at 7:38 AM, Kukjin Kim <kgene@kernel.org> wrote: > Pankaj Dubey wrote: >> >> Hi Kukjin, >> > Hi, > >> On Thursday 20 November 2014 11:18 PM, Sylwester Nawrocki wrote: >> > On 19/11/14 04:37, Pankaj Dubey wrote: >> >>>> >> >>>>>> +static int exynos5440_clk_restart_notify(struct notifier_block *this, >> >>>>>> + unsigned long code, void *unused) >> >>>>>> +{ >> >>>>>> + u32 val, status; >> >>>>>> + >> >>>>>> + status = readl_relaxed(reg_base + 0xbc); >> >>>>>> + val = readl_relaxed(reg_base + 0xcc); >> >>>>>> + val = (val & 0xffff0000) | (status & 0xffff); >> >>>>>> + writel_relaxed(val, reg_base + 0xcc); >> >>>> >> >>>> Can we have macro definitions for these 0xcc, 0xbc address offsets ? >> >>>> I must say I couldn't find them documented in any Exynos datasheet I've >> >> got though. >> >>>> >> >> >> >> I also wished this, but I could not find them documented. >> >> So I tried to keep logic of original code as it is, just changed location. >> >> I would also like to mention that I have not tested this on exynos5440 as I >> >> do not have >> >> one with me. I believe if it was working at its original place in >> >> exynos_restart it should work >> >> here also. Other patch (2/2) I have verified on Exynos3250 board and its >> >> working well. >> > >> > I think it's best to merge both patches in that series through >> > the arm-soc tree, since applying them not in order may cause some >> > breakage. Thus I'd let Kukjin take this patch set into his tree. >> > >> > For both patches: >> > Acked-by: Sylwester Nawrocki <s.nawrocki@samsung.com> >> > > Sylwester, thanks for your ack. > >> >> Will you please take this series into your tree. >> > + Amit Daniel > > Yeah, this series looks good to me and I'm looking at this series with Amit's > moving pmu.c into drivers/. Maybe I need to ask Amit to respin the series on > top of this? If required, I'll reply. I was ready to post pmu v4 version with Russell suggested changes so i rebased and posted v4 series on top of this 2 patch. Regards, Amit D > > Thanks, > Kukjin > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c index 8f638ad..8f995b7 100644 --- a/arch/arm/mach-exynos/exynos.c +++ b/arch/arm/mach-exynos/exynos.c @@ -89,24 +89,7 @@ static struct map_desc exynos5_iodesc[] __initdata = { static void exynos_restart(enum reboot_mode mode, const char *cmd) { - struct device_node *np; - u32 val = 0x1; - void __iomem *addr = pmu_base_addr + EXYNOS_SWRESET; - - if (of_machine_is_compatible("samsung,exynos5440")) { - u32 status; - np = of_find_compatible_node(NULL, NULL, "samsung,exynos5440-clock"); - - addr = of_iomap(np, 0) + 0xbc; - status = __raw_readl(addr); - - addr = of_iomap(np, 0) + 0xcc; - val = __raw_readl(addr); - - val = (val & 0xffff0000) | (status & 0xffff); - } - - __raw_writel(val, addr); + __raw_writel(0x1, pmu_base_addr + EXYNOS_SWRESET); } static struct platform_device exynos_cpuidle = { diff --git a/drivers/clk/samsung/clk-exynos5440.c b/drivers/clk/samsung/clk-exynos5440.c index 00d1d00..979e813 100644 --- a/drivers/clk/samsung/clk-exynos5440.c +++ b/drivers/clk/samsung/clk-exynos5440.c @@ -15,6 +15,8 @@ #include <linux/clk-provider.h> #include <linux/of.h> #include <linux/of_address.h> +#include <linux/notifier.h> +#include <linux/reboot.h> #include "clk.h" #include "clk-pll.h" @@ -23,6 +25,8 @@ #define CPU_CLK_STATUS 0xfc #define MISC_DOUT1 0x558 +static void __iomem *reg_base; + /* parent clock name list */ PNAME(mout_armclk_p) = { "cplla", "cpllb" }; PNAME(mout_spi_p) = { "div125", "div200" }; @@ -89,10 +93,30 @@ static const struct of_device_id ext_clk_match[] __initconst = { {}, }; +static int exynos5440_clk_restart_notify(struct notifier_block *this, + unsigned long code, void *unused) +{ + u32 val, status; + + status = readl_relaxed(reg_base + 0xbc); + val = readl_relaxed(reg_base + 0xcc); + val = (val & 0xffff0000) | (status & 0xffff); + writel_relaxed(val, reg_base + 0xcc); + + return NOTIFY_DONE; +} + +/* + * Exynos5440 Clock restart notifier, handles restart functionality + */ +static struct notifier_block exynos5440_clk_restart_handler = { + .notifier_call = exynos5440_clk_restart_notify, + .priority = 128, +}; + /* register exynos5440 clocks */ static void __init exynos5440_clk_init(struct device_node *np) { - void __iomem *reg_base; struct samsung_clk_provider *ctx; reg_base = of_iomap(np, 0); @@ -125,6 +149,9 @@ static void __init exynos5440_clk_init(struct device_node *np) samsung_clk_of_add_provider(np, ctx); + if (register_restart_handler(&exynos5440_clk_restart_handler)) + pr_warn("exynos5440 clock can't register restart handler\n"); + pr_info("Exynos5440: arm_clk = %ldHz\n", _get_rate("arm_clk")); pr_info("exynos5440 clock initialization complete\n"); }