All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440
@ 2013-09-30  1:32 Jungseok Lee
  2013-09-30  3:03 ` Sachin Kamat
  0 siblings, 1 reply; 6+ messages in thread
From: Jungseok Lee @ 2013-09-30  1:32 UTC (permalink / raw)
  To: kgene.kim; +Cc: linux-samsung-soc


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(-)

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();
 }
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440
  2013-09-30  1:32 [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440 Jungseok Lee
@ 2013-09-30  3:03 ` Sachin Kamat
  2013-09-30  5:20   ` Jungseok Lee
  0 siblings, 1 reply; 6+ messages in thread
From: Sachin Kamat @ 2013-09-30  3:03 UTC (permalink / raw)
  To: Jungseok Lee; +Cc: Kukjin Kim, linux-samsung-soc

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.

-- 
With warm regards,
Sachin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440
  2013-09-30  3:03 ` Sachin Kamat
@ 2013-09-30  5:20   ` Jungseok Lee
  2013-09-30  7:25     ` Kukjin Kim
  0 siblings, 1 reply; 6+ messages in thread
From: Jungseok Lee @ 2013-09-30  5:20 UTC (permalink / raw)
  To: 'Sachin Kamat'; +Cc: 'Kukjin Kim', 'linux-samsung-soc'

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440
  2013-09-30  5:20   ` Jungseok Lee
@ 2013-09-30  7:25     ` Kukjin Kim
  2013-09-30  8:26       ` Sachin Kamat
  0 siblings, 1 reply; 6+ messages in thread
From: Kukjin Kim @ 2013-09-30  7:25 UTC (permalink / raw)
  To: 'Jungseok Lee', 'Sachin Kamat'
  Cc: 'linux-samsung-soc'

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440
  2013-09-30  7:25     ` Kukjin Kim
@ 2013-09-30  8:26       ` Sachin Kamat
  2013-09-30  8:41         ` Tomasz Figa
  0 siblings, 1 reply; 6+ messages in thread
From: Sachin Kamat @ 2013-09-30  8:26 UTC (permalink / raw)
  To: Kukjin Kim; +Cc: Jungseok Lee, linux-samsung-soc

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.

-- 
With warm regards,
Sachin

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440
  2013-09-30  8:26       ` Sachin Kamat
@ 2013-09-30  8:41         ` Tomasz Figa
  0 siblings, 0 replies; 6+ messages in thread
From: Tomasz Figa @ 2013-09-30  8:41 UTC (permalink / raw)
  To: Sachin Kamat; +Cc: Kukjin Kim, Jungseok Lee, linux-samsung-soc

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-09-30  8:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-30  1:32 [PATCH V2] ARM: EXYNOS: implement pm_power_off for EXYNOS5440 Jungseok Lee
2013-09-30  3:03 ` Sachin Kamat
2013-09-30  5:20   ` Jungseok Lee
2013-09-30  7:25     ` Kukjin Kim
2013-09-30  8:26       ` Sachin Kamat
2013-09-30  8:41         ` Tomasz Figa

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.