| Message ID | 003101cebd7c$ded3a610$9c7af230$%lee@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30 September 2013 07:02, Jungseok Lee <jays.lee@samsung.com> wrote: > + /* turn off all power domains */ > + addr = of_iomap(np, 0) + 0x14; > + __raw_writel(0x1, addr); Actually my comment was more about mentioning what these above values especially 0x14 represented? Either using a macro (preferred way) or atleast a comment.
On Monday, September 30, 2013 12:04 PM, Sachin Kamat wrote: > On 30 September 2013 07:02, Jungseok Lee <jays.lee@samsung.com> wrote: > > > + /* turn off all power domains */ > > + addr = of_iomap(np, 0) + 0x14; > > + __raw_writel(0x1, addr); > > Actually my comment was more about mentioning what these above values > especially 0x14 represented? Either using a macro (preferred way) or > atleast a comment. How about changing a variable name "addr" to "power_down_reg"? 0x14 is only available for exynos5440, not exynos5. Currently, there are no macros for specific SoCs in exynos5, such as exynos5250 or exynos5420. That is why I hesitate to add a macro for 0x14. Best Regards Jungseok Lee > > -- > With warm regards, > Sachin > -- > 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 -- 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
Jungseok Lee wrote: > > On Monday, September 30, 2013 12:04 PM, Sachin Kamat wrote: > > On 30 September 2013 07:02, Jungseok Lee <jays.lee@samsung.com> wrote: > > > > > + /* turn off all power domains */ > > > + addr = of_iomap(np, 0) + 0x14; > > > + __raw_writel(0x1, addr); > > > > Actually my comment was more about mentioning what these above values > > especially 0x14 represented? Either using a macro (preferred way) or > > atleast a comment. > > How about changing a variable name "addr" to "power_down_reg"? > > 0x14 is only available for exynos5440, not exynos5. Currently, there are > no > macros for specific SoCs in exynos5, such as exynos5250 or exynos5420. > That is > why I hesitate to add a macro for 0x14. > I think current patch looks good to me, and in this case I don't have any idea why we should macro for just one time usage. Applied, 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
On 30 September 2013 12:55, Kukjin Kim <kgene@kernel.org> wrote: >> > I think current patch looks good to me, and in this case I don't have any > idea why we should macro for just one time usage. It is not the question of one time usage, it is just to make the code more readable.
On Monday 30 of September 2013 13:56:39 Sachin Kamat wrote: > On 30 September 2013 12:55, Kukjin Kim <kgene@kernel.org> wrote: > > I think current patch looks good to me, and in this case I don't have > > any idea why we should macro for just one time usage. > > It is not the question of one time usage, it is just to make the code > more readable. I tend to agree with Sachin on this. Code as in current version of the patch reads as "write 0x1 to register offset by 0x14 from clock controller base", while it should read "write RESET_VAL to RESET_CTRL register of clock controller" (names made up) for people doing further work on this file to quickly find what the code does or allow referring to SoC documentation. Best regards, Tomasz -- 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/common.c b/arch/arm/mach-exynos/common.c index ba95e5d..24eaa65 100644 --- a/arch/arm/mach-exynos/common.c +++ b/arch/arm/mach-exynos/common.c @@ -294,11 +294,24 @@ void exynos5_restart(enum reboot_mode mode, const char *cmd) __raw_writel(val, addr); } +static void exynos5440_power_off(void) +{ + struct device_node *np; + void __iomem *addr; + + np = of_find_compatible_node(NULL, NULL, "samsung,exynos5440-clock"); + + /* turn off all power domains */ + addr = of_iomap(np, 0) + 0x14; + __raw_writel(0x1, addr); +} + void __init exynos_init_late(void) { - if (of_machine_is_compatible("samsung,exynos5440")) - /* to be supported later */ + if (of_machine_is_compatible("samsung,exynos5440")) { + pm_power_off = exynos5440_power_off; return; + } exynos_pm_late_initcall(); }
This patch implements pm_power_off function since a power-down control register should be set in order to turn off EXYNOS5440. Otherwise, power domains remain alive despite "poweroff" action. Signed-off-by: Jungseok Lee <jays.lee@samsung.com> --- Changes since v1: - Added a comment to the effect of register configuration. arch/arm/mach-exynos/common.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)