* [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
@ 2013-11-20 18:21 ` Daniel Kurtz
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-11-20 18:21 UTC (permalink / raw)
To: Russell King, Kukjin Kim, Ben Dooks
Cc: Tomasz Figa, Heiko Stuebner, Amit Daniel Kachhap,
Abhilash Kesavan, linux-arm-kernel, linux-samsung-soc,
linux-kernel, Doug Andersen, Daniel Kurtz
These tables are all immutable, make them const to save 4416 bytes of RAM.
size arch/arm/mach-exynos/pmu.o
text data bss
848 4420 4 // before
5264 4 4 // after
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
arch/arm/mach-exynos/pmu.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index 97d6885..6609145 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -17,9 +17,9 @@
#include "common.h"
-static struct exynos_pmu_conf *exynos_pmu_config;
+static const struct exynos_pmu_conf *exynos_pmu_config;
-static struct exynos_pmu_conf exynos4210_pmu_config[] = {
+static const struct exynos_pmu_conf exynos4210_pmu_config[] = {
/* { .reg = address, .val = { AFTR, LPA, SLEEP } */
{ S5P_ARM_CORE0_LOWPWR, { 0x0, 0x0, 0x2 } },
{ S5P_DIS_IRQ_CORE0, { 0x0, 0x0, 0x0 } },
@@ -95,7 +95,7 @@ static struct exynos_pmu_conf exynos4210_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static struct exynos_pmu_conf exynos4x12_pmu_config[] = {
+static const struct exynos_pmu_conf exynos4x12_pmu_config[] = {
{ S5P_ARM_CORE0_LOWPWR, { 0x0, 0x0, 0x2 } },
{ S5P_DIS_IRQ_CORE0, { 0x0, 0x0, 0x0 } },
{ S5P_DIS_IRQ_CENTRAL0, { 0x0, 0x0, 0x0 } },
@@ -203,7 +203,7 @@ static struct exynos_pmu_conf exynos4x12_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static struct exynos_pmu_conf exynos4412_pmu_config[] = {
+static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
{ S5P_ARM_CORE2_LOWPWR, { 0x0, 0x0, 0x2 } },
{ S5P_DIS_IRQ_CORE2, { 0x0, 0x0, 0x0 } },
{ S5P_DIS_IRQ_CENTRAL2, { 0x0, 0x0, 0x0 } },
@@ -213,7 +213,7 @@ static struct exynos_pmu_conf exynos4412_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static struct exynos_pmu_conf exynos5250_pmu_config[] = {
+static const struct exynos_pmu_conf exynos5250_pmu_config[] = {
/* { .reg = address, .val = { AFTR, LPA, SLEEP } */
{ EXYNOS5_ARM_CORE0_SYS_PWR_REG, { 0x0, 0x0, 0x2} },
{ EXYNOS5_DIS_IRQ_ARM_CORE0_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
@@ -317,7 +317,7 @@ static struct exynos_pmu_conf exynos5250_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static void __iomem *exynos5_list_both_cnt_feed[] = {
+static void __iomem * const exynos5_list_both_cnt_feed[] = {
EXYNOS5_ARM_CORE0_OPTION,
EXYNOS5_ARM_CORE1_OPTION,
EXYNOS5_ARM_COMMON_OPTION,
@@ -331,7 +331,7 @@ static void __iomem *exynos5_list_both_cnt_feed[] = {
EXYNOS5_TOP_PWR_SYSMEM_OPTION,
};
-static void __iomem *exynos5_list_diable_wfi_wfe[] = {
+static void __iomem * const exynos5_list_diable_wfi_wfe[] = {
EXYNOS5_ARM_CORE1_OPTION,
EXYNOS5_FSYS_ARM_OPTION,
EXYNOS5_ISP_ARM_OPTION,
--
1.8.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
@ 2013-11-20 18:21 ` Daniel Kurtz
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-11-20 18:21 UTC (permalink / raw)
To: linux-arm-kernel
These tables are all immutable, make them const to save 4416 bytes of RAM.
size arch/arm/mach-exynos/pmu.o
text data bss
848 4420 4 // before
5264 4 4 // after
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
arch/arm/mach-exynos/pmu.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/mach-exynos/pmu.c b/arch/arm/mach-exynos/pmu.c
index 97d6885..6609145 100644
--- a/arch/arm/mach-exynos/pmu.c
+++ b/arch/arm/mach-exynos/pmu.c
@@ -17,9 +17,9 @@
#include "common.h"
-static struct exynos_pmu_conf *exynos_pmu_config;
+static const struct exynos_pmu_conf *exynos_pmu_config;
-static struct exynos_pmu_conf exynos4210_pmu_config[] = {
+static const struct exynos_pmu_conf exynos4210_pmu_config[] = {
/* { .reg = address, .val = { AFTR, LPA, SLEEP } */
{ S5P_ARM_CORE0_LOWPWR, { 0x0, 0x0, 0x2 } },
{ S5P_DIS_IRQ_CORE0, { 0x0, 0x0, 0x0 } },
@@ -95,7 +95,7 @@ static struct exynos_pmu_conf exynos4210_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static struct exynos_pmu_conf exynos4x12_pmu_config[] = {
+static const struct exynos_pmu_conf exynos4x12_pmu_config[] = {
{ S5P_ARM_CORE0_LOWPWR, { 0x0, 0x0, 0x2 } },
{ S5P_DIS_IRQ_CORE0, { 0x0, 0x0, 0x0 } },
{ S5P_DIS_IRQ_CENTRAL0, { 0x0, 0x0, 0x0 } },
@@ -203,7 +203,7 @@ static struct exynos_pmu_conf exynos4x12_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static struct exynos_pmu_conf exynos4412_pmu_config[] = {
+static const struct exynos_pmu_conf exynos4412_pmu_config[] = {
{ S5P_ARM_CORE2_LOWPWR, { 0x0, 0x0, 0x2 } },
{ S5P_DIS_IRQ_CORE2, { 0x0, 0x0, 0x0 } },
{ S5P_DIS_IRQ_CENTRAL2, { 0x0, 0x0, 0x0 } },
@@ -213,7 +213,7 @@ static struct exynos_pmu_conf exynos4412_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static struct exynos_pmu_conf exynos5250_pmu_config[] = {
+static const struct exynos_pmu_conf exynos5250_pmu_config[] = {
/* { .reg = address, .val = { AFTR, LPA, SLEEP } */
{ EXYNOS5_ARM_CORE0_SYS_PWR_REG, { 0x0, 0x0, 0x2} },
{ EXYNOS5_DIS_IRQ_ARM_CORE0_LOCAL_SYS_PWR_REG, { 0x0, 0x0, 0x0} },
@@ -317,7 +317,7 @@ static struct exynos_pmu_conf exynos5250_pmu_config[] = {
{ PMU_TABLE_END,},
};
-static void __iomem *exynos5_list_both_cnt_feed[] = {
+static void __iomem * const exynos5_list_both_cnt_feed[] = {
EXYNOS5_ARM_CORE0_OPTION,
EXYNOS5_ARM_CORE1_OPTION,
EXYNOS5_ARM_COMMON_OPTION,
@@ -331,7 +331,7 @@ static void __iomem *exynos5_list_both_cnt_feed[] = {
EXYNOS5_TOP_PWR_SYSMEM_OPTION,
};
-static void __iomem *exynos5_list_diable_wfi_wfe[] = {
+static void __iomem * const exynos5_list_diable_wfi_wfe[] = {
EXYNOS5_ARM_CORE1_OPTION,
EXYNOS5_FSYS_ARM_OPTION,
EXYNOS5_ISP_ARM_OPTION,
--
1.8.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] ARM: SAMSUNG: Let s3c_pm_do_restore_*() take const sleep_save
2013-11-20 18:21 ` Daniel Kurtz
@ 2013-11-20 18:21 ` Daniel Kurtz
-1 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-11-20 18:21 UTC (permalink / raw)
To: Russell King, Kukjin Kim, Ben Dooks
Cc: Tomasz Figa, Heiko Stuebner, Amit Daniel Kachhap,
Abhilash Kesavan, linux-arm-kernel, linux-samsung-soc,
linux-kernel, Doug Andersen, Daniel Kurtz
The restore functions do not modify the passed in struct sleep_save,
so that parameter can be const.
This allows us to pass in const struct. This allows us to use const
structs sleep_save to define system registers that will always be
restored to a constant value.
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
arch/arm/plat-samsung/include/plat/pm.h | 4 ++--
arch/arm/plat-samsung/pm.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-samsung/include/plat/pm.h
index 6bc1a8f..ff6063f 100644
--- a/arch/arm/plat-samsung/include/plat/pm.h
+++ b/arch/arm/plat-samsung/include/plat/pm.h
@@ -101,8 +101,8 @@ struct pm_uart_save {
/* helper functions to save/restore lists of registers. */
extern void s3c_pm_do_save(struct sleep_save *ptr, int count);
-extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
-extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
+extern void s3c_pm_do_restore(const struct sleep_save *ptr, int count);
+extern void s3c_pm_do_restore_core(const struct sleep_save *ptr, int count);
#ifdef CONFIG_SAMSUNG_PM
extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
index d0c2301..416e5be 100644
--- a/arch/arm/plat-samsung/pm.c
+++ b/arch/arm/plat-samsung/pm.c
@@ -182,7 +182,7 @@ void s3c_pm_do_save(struct sleep_save *ptr, int count)
* restore the UARTs state yet
*/
-void s3c_pm_do_restore(struct sleep_save *ptr, int count)
+void s3c_pm_do_restore(const struct sleep_save *ptr, int count)
{
for (; count > 0; count--, ptr++) {
printk(KERN_DEBUG "restore %p (restore %08lx, was %08x)\n",
@@ -203,7 +203,7 @@ void s3c_pm_do_restore(struct sleep_save *ptr, int count)
* peripherals, as things may be changing!
*/
-void s3c_pm_do_restore_core(struct sleep_save *ptr, int count)
+void s3c_pm_do_restore_core(const struct sleep_save *ptr, int count)
{
for (; count > 0; count--, ptr++)
__raw_writel(ptr->val, ptr->reg);
--
1.8.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/3] ARM: SAMSUNG: Let s3c_pm_do_restore_*() take const sleep_save
@ 2013-11-20 18:21 ` Daniel Kurtz
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-11-20 18:21 UTC (permalink / raw)
To: linux-arm-kernel
The restore functions do not modify the passed in struct sleep_save,
so that parameter can be const.
This allows us to pass in const struct. This allows us to use const
structs sleep_save to define system registers that will always be
restored to a constant value.
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
arch/arm/plat-samsung/include/plat/pm.h | 4 ++--
arch/arm/plat-samsung/pm.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/arm/plat-samsung/include/plat/pm.h b/arch/arm/plat-samsung/include/plat/pm.h
index 6bc1a8f..ff6063f 100644
--- a/arch/arm/plat-samsung/include/plat/pm.h
+++ b/arch/arm/plat-samsung/include/plat/pm.h
@@ -101,8 +101,8 @@ struct pm_uart_save {
/* helper functions to save/restore lists of registers. */
extern void s3c_pm_do_save(struct sleep_save *ptr, int count);
-extern void s3c_pm_do_restore(struct sleep_save *ptr, int count);
-extern void s3c_pm_do_restore_core(struct sleep_save *ptr, int count);
+extern void s3c_pm_do_restore(const struct sleep_save *ptr, int count);
+extern void s3c_pm_do_restore_core(const struct sleep_save *ptr, int count);
#ifdef CONFIG_SAMSUNG_PM
extern int s3c_irq_wake(struct irq_data *data, unsigned int state);
diff --git a/arch/arm/plat-samsung/pm.c b/arch/arm/plat-samsung/pm.c
index d0c2301..416e5be 100644
--- a/arch/arm/plat-samsung/pm.c
+++ b/arch/arm/plat-samsung/pm.c
@@ -182,7 +182,7 @@ void s3c_pm_do_save(struct sleep_save *ptr, int count)
* restore the UARTs state yet
*/
-void s3c_pm_do_restore(struct sleep_save *ptr, int count)
+void s3c_pm_do_restore(const struct sleep_save *ptr, int count)
{
for (; count > 0; count--, ptr++) {
printk(KERN_DEBUG "restore %p (restore %08lx, was %08x)\n",
@@ -203,7 +203,7 @@ void s3c_pm_do_restore(struct sleep_save *ptr, int count)
* peripherals, as things may be changing!
*/
-void s3c_pm_do_restore_core(struct sleep_save *ptr, int count)
+void s3c_pm_do_restore_core(const struct sleep_save *ptr, int count)
{
for (; count > 0; count--, ptr++)
__raw_writel(ptr->val, ptr->reg);
--
1.8.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: Constify clksrc immutable register restore tables
2013-11-20 18:21 ` Daniel Kurtz
@ 2013-11-20 18:21 ` Daniel Kurtz
-1 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-11-20 18:21 UTC (permalink / raw)
To: Russell King, Kukjin Kim, Ben Dooks
Cc: Tomasz Figa, Heiko Stuebner, Amit Daniel Kachhap,
Abhilash Kesavan, linux-arm-kernel, linux-samsung-soc,
linux-kernel, Doug Andersen, Daniel Kurtz
The clksrc tables are constant, they are not used to store register values
at suspend.
size arch/arm/mach-exynos/pm.o
text data bss
1591 212 12 // Before
1671 132 12 // After
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
arch/arm/mach-exynos/pm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c679db5..7fb0f13 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -36,7 +36,7 @@
#include "common.h"
-static struct sleep_save exynos4_set_clksrc[] = {
+static const struct sleep_save exynos4_set_clksrc[] = {
{ .reg = EXYNOS4_CLKSRC_MASK_TOP , .val = 0x00000001, },
{ .reg = EXYNOS4_CLKSRC_MASK_CAM , .val = 0x11111111, },
{ .reg = EXYNOS4_CLKSRC_MASK_TV , .val = 0x00000111, },
@@ -48,7 +48,7 @@ static struct sleep_save exynos4_set_clksrc[] = {
{ .reg = EXYNOS4_CLKSRC_MASK_DMC , .val = 0x00010000, },
};
-static struct sleep_save exynos4210_set_clksrc[] = {
+static const struct sleep_save exynos4210_set_clksrc[] = {
{ .reg = EXYNOS4210_CLKSRC_MASK_LCD1 , .val = 0x00001111, },
};
--
1.8.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: Constify clksrc immutable register restore tables
@ 2013-11-20 18:21 ` Daniel Kurtz
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-11-20 18:21 UTC (permalink / raw)
To: linux-arm-kernel
The clksrc tables are constant, they are not used to store register values
at suspend.
size arch/arm/mach-exynos/pm.o
text data bss
1591 212 12 // Before
1671 132 12 // After
Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
arch/arm/mach-exynos/pm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index c679db5..7fb0f13 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -36,7 +36,7 @@
#include "common.h"
-static struct sleep_save exynos4_set_clksrc[] = {
+static const struct sleep_save exynos4_set_clksrc[] = {
{ .reg = EXYNOS4_CLKSRC_MASK_TOP , .val = 0x00000001, },
{ .reg = EXYNOS4_CLKSRC_MASK_CAM , .val = 0x11111111, },
{ .reg = EXYNOS4_CLKSRC_MASK_TV , .val = 0x00000111, },
@@ -48,7 +48,7 @@ static struct sleep_save exynos4_set_clksrc[] = {
{ .reg = EXYNOS4_CLKSRC_MASK_DMC , .val = 0x00010000, },
};
-static struct sleep_save exynos4210_set_clksrc[] = {
+static const struct sleep_save exynos4210_set_clksrc[] = {
{ .reg = EXYNOS4210_CLKSRC_MASK_LCD1 , .val = 0x00001111, },
};
--
1.8.4.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
2013-11-20 18:21 ` Daniel Kurtz
@ 2013-12-09 13:14 ` Tomasz Figa
-1 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2013-12-09 13:14 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Russell King, Kukjin Kim, Ben Dooks, Heiko Stuebner,
Amit Daniel Kachhap, Abhilash Kesavan, linux-arm-kernel,
linux-samsung-soc, linux-kernel, Doug Andersen
Hi Daniel,
On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
> These tables are all immutable, make them const to save 4416 bytes of RAM.
>
> size arch/arm/mach-exynos/pmu.o
> text data bss
> 848 4420 4 // before
> 5264 4 4 // after
I'm not sure where the mentioned saving of RAM is. Moving data between
sections is not supposed to make it use less memory, I believe.
Anyway, it's a good practice to mark constant data as const, to disallow
changing them at runtime by mistake, so the patch is fine. Except some
issues I commented on inline.
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
@ 2013-12-09 13:14 ` Tomasz Figa
0 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2013-12-09 13:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi Daniel,
On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
> These tables are all immutable, make them const to save 4416 bytes of RAM.
>
> size arch/arm/mach-exynos/pmu.o
> text data bss
> 848 4420 4 // before
> 5264 4 4 // after
I'm not sure where the mentioned saving of RAM is. Moving data between
sections is not supposed to make it use less memory, I believe.
Anyway, it's a good practice to mark constant data as const, to disallow
changing them at runtime by mistake, so the patch is fine. Except some
issues I commented on inline.
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 2/3] ARM: SAMSUNG: Let s3c_pm_do_restore_*() take const sleep_save
2013-11-20 18:21 ` Daniel Kurtz
@ 2013-12-09 13:27 ` Tomasz Figa
-1 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2013-12-09 13:27 UTC (permalink / raw)
To: Daniel Kurtz
Cc: Russell King, Kukjin Kim, Ben Dooks, Heiko Stuebner,
Amit Daniel Kachhap, Abhilash Kesavan, linux-arm-kernel,
linux-samsung-soc, linux-kernel, Doug Andersen
Hi Daniel,
On Thursday 21 of November 2013 02:21:25 Daniel Kurtz wrote:
> The restore functions do not modify the passed in struct sleep_save,
> so that parameter can be const.
>
> This allows us to pass in const struct. This allows us to use const
> structs sleep_save to define system registers that will always be
> restored to a constant value.
>
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
> arch/arm/plat-samsung/include/plat/pm.h | 4 ++--
> arch/arm/plat-samsung/pm.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 2/3] ARM: SAMSUNG: Let s3c_pm_do_restore_*() take const sleep_save
@ 2013-12-09 13:27 ` Tomasz Figa
0 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2013-12-09 13:27 UTC (permalink / raw)
To: linux-arm-kernel
Hi Daniel,
On Thursday 21 of November 2013 02:21:25 Daniel Kurtz wrote:
> The restore functions do not modify the passed in struct sleep_save,
> so that parameter can be const.
>
> This allows us to pass in const struct. This allows us to use const
> structs sleep_save to define system registers that will always be
> restored to a constant value.
>
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
> arch/arm/plat-samsung/include/plat/pm.h | 4 ++--
> arch/arm/plat-samsung/pm.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
2013-12-09 13:14 ` Tomasz Figa
@ 2013-12-09 16:11 ` Daniel Kurtz
-1 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-12-09 16:11 UTC (permalink / raw)
To: Tomasz Figa
Cc: kgene.kim, Heiko Stuebner, Amit Daniel Kachhap, linux-arm-kernel,
linux-samsung-soc, Doug Andersen, Abhilash Kesavan, Ben Dooks,
Russell King, linux-kernel
Hi Tomasz,
Thank you for the reviews.
On Dec 9, 2013 5:15 AM, "Tomasz Figa" <t.figa@samsung.com> wrote:
>
> Hi Daniel,
>
> On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
> > These tables are all immutable, make them const to save 4416 bytes of RAM.
> >
> > size arch/arm/mach-exynos/pmu.o
> > text data bss
> > 848 4420 4 // before
> > 5264 4 4 // after
>
> I'm not sure where the mentioned saving of RAM is. Moving data between
> sections is not supposed to make it use less memory, I believe.
You are correct. This was my misunderstanding from doing too much
work with microcontrollers, where .text sections are accessed in place
from FLASH for code and const data, but .data memory is copied from a
FLASH section to a second RAM section at init for access at runtime.
Most modern Linux systems copy/decompress their code and data sections
from external storage to RAM anyway, so there is no actual memory
savings (except potentially the compiler may be able to optimize a bit
more with the const hint).
>
> Anyway, it's a good practice to mark constant data as const, to disallow
> changing them at runtime by mistake, so the patch is fine. Except some
> issues I commented on inline.
Were there supposed to be inline comments? I don't see any.
Best Regards,
-djk
>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
>
> Best regards,
> Tomasz
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
@ 2013-12-09 16:11 ` Daniel Kurtz
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Kurtz @ 2013-12-09 16:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Tomasz,
Thank you for the reviews.
On Dec 9, 2013 5:15 AM, "Tomasz Figa" <t.figa@samsung.com> wrote:
>
> Hi Daniel,
>
> On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
> > These tables are all immutable, make them const to save 4416 bytes of RAM.
> >
> > size arch/arm/mach-exynos/pmu.o
> > text data bss
> > 848 4420 4 // before
> > 5264 4 4 // after
>
> I'm not sure where the mentioned saving of RAM is. Moving data between
> sections is not supposed to make it use less memory, I believe.
You are correct. This was my misunderstanding from doing too much
work with microcontrollers, where .text sections are accessed in place
from FLASH for code and const data, but .data memory is copied from a
FLASH section to a second RAM section at init for access at runtime.
Most modern Linux systems copy/decompress their code and data sections
from external storage to RAM anyway, so there is no actual memory
savings (except potentially the compiler may be able to optimize a bit
more with the const hint).
>
> Anyway, it's a good practice to mark constant data as const, to disallow
> changing them at runtime by mistake, so the patch is fine. Except some
> issues I commented on inline.
Were there supposed to be inline comments? I don't see any.
Best Regards,
-djk
>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
>
> Best regards,
> Tomasz
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
2013-12-09 16:11 ` Daniel Kurtz
@ 2013-12-09 16:15 ` Tomasz Figa
-1 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2013-12-09 16:15 UTC (permalink / raw)
To: Daniel Kurtz
Cc: kgene.kim, Heiko Stuebner, Amit Daniel Kachhap, linux-arm-kernel,
linux-samsung-soc, Doug Andersen, Abhilash Kesavan, Ben Dooks,
Russell King, linux-kernel
On Tuesday 10 of December 2013 00:11:40 Daniel Kurtz wrote:
> Hi Tomasz,
>
> Thank you for the reviews.
>
> On Dec 9, 2013 5:15 AM, "Tomasz Figa" <t.figa@samsung.com> wrote:
> >
> > Hi Daniel,
> >
> > On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
> > > These tables are all immutable, make them const to save 4416 bytes of RAM.
> > >
> > > size arch/arm/mach-exynos/pmu.o
> > > text data bss
> > > 848 4420 4 // before
> > > 5264 4 4 // after
> >
> > I'm not sure where the mentioned saving of RAM is. Moving data between
> > sections is not supposed to make it use less memory, I believe.
>
> You are correct. This was my misunderstanding from doing too much
> work with microcontrollers, where .text sections are accessed in place
> from FLASH for code and const data, but .data memory is copied from a
> FLASH section to a second RAM section at init for access at runtime.
> Most modern Linux systems copy/decompress their code and data sections
> from external storage to RAM anyway, so there is no actual memory
> savings (except potentially the compiler may be able to optimize a bit
> more with the const hint).
>
> >
> > Anyway, it's a good practice to mark constant data as const, to disallow
> > changing them at runtime by mistake, so the patch is fine. Except some
> > issues I commented on inline.
>
> Were there supposed to be inline comments? I don't see any.
Oops, sorry for this, forgot to remove the last sentence. I initially had
one question about the constant pointers below, but I read through the
full code again and answered it myself.
The patch is fine.
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
@ 2013-12-09 16:15 ` Tomasz Figa
0 siblings, 0 replies; 16+ messages in thread
From: Tomasz Figa @ 2013-12-09 16:15 UTC (permalink / raw)
To: linux-arm-kernel
On Tuesday 10 of December 2013 00:11:40 Daniel Kurtz wrote:
> Hi Tomasz,
>
> Thank you for the reviews.
>
> On Dec 9, 2013 5:15 AM, "Tomasz Figa" <t.figa@samsung.com> wrote:
> >
> > Hi Daniel,
> >
> > On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
> > > These tables are all immutable, make them const to save 4416 bytes of RAM.
> > >
> > > size arch/arm/mach-exynos/pmu.o
> > > text data bss
> > > 848 4420 4 // before
> > > 5264 4 4 // after
> >
> > I'm not sure where the mentioned saving of RAM is. Moving data between
> > sections is not supposed to make it use less memory, I believe.
>
> You are correct. This was my misunderstanding from doing too much
> work with microcontrollers, where .text sections are accessed in place
> from FLASH for code and const data, but .data memory is copied from a
> FLASH section to a second RAM section at init for access at runtime.
> Most modern Linux systems copy/decompress their code and data sections
> from external storage to RAM anyway, so there is no actual memory
> savings (except potentially the compiler may be able to optimize a bit
> more with the const hint).
>
> >
> > Anyway, it's a good practice to mark constant data as const, to disallow
> > changing them at runtime by mistake, so the patch is fine. Except some
> > issues I commented on inline.
>
> Were there supposed to be inline comments? I don't see any.
Oops, sorry for this, forgot to remove the last sentence. I initially had
one question about the constant pointers below, but I read through the
full code again and answered it myself.
The patch is fine.
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
2013-12-09 16:15 ` Tomasz Figa
@ 2013-12-09 21:00 ` Kukjin Kim
-1 siblings, 0 replies; 16+ messages in thread
From: Kukjin Kim @ 2013-12-09 21:00 UTC (permalink / raw)
To: Tomasz Figa
Cc: Daniel Kurtz, linux-samsung-soc, Russell King, Heiko Stuebner,
Doug Andersen, linux-kernel, Amit Daniel Kachhap, kgene.kim,
Ben Dooks, Abhilash Kesavan, linux-arm-kernel
On 12/10/13 01:15, Tomasz Figa wrote:
> On Tuesday 10 of December 2013 00:11:40 Daniel Kurtz wrote:
>> Hi Tomasz,
>>
>> Thank you for the reviews.
>>
>> On Dec 9, 2013 5:15 AM, "Tomasz Figa"<t.figa@samsung.com> wrote:
>>>
>>> Hi Daniel,
>>>
>>> On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
>>>> These tables are all immutable, make them const to save 4416 bytes of RAM.
>>>>
>>>> size arch/arm/mach-exynos/pmu.o
>>>> text data bss
>>>> 848 4420 4 // before
>>>> 5264 4 4 // after
>>>
>>> I'm not sure where the mentioned saving of RAM is. Moving data between
>>> sections is not supposed to make it use less memory, I believe.
>>
>> You are correct. This was my misunderstanding from doing too much
>> work with microcontrollers, where .text sections are accessed in place
>> from FLASH for code and const data, but .data memory is copied from a
>> FLASH section to a second RAM section at init for access at runtime.
>> Most modern Linux systems copy/decompress their code and data sections
>> from external storage to RAM anyway, so there is no actual memory
>> savings (except potentially the compiler may be able to optimize a bit
>> more with the const hint).
>>
>>>
>>> Anyway, it's a good practice to mark constant data as const, to disallow
>>> changing them at runtime by mistake, so the patch is fine. Except some
>>> issues I commented on inline.
>>
>> Were there supposed to be inline comments? I don't see any.
>
> Oops, sorry for this, forgot to remove the last sentence. I initially had
> one question about the constant pointers below, but I read through the
> full code again and answered it myself.
>
> The patch is fine.
>
> Reviewed-by: Tomasz Figa<t.figa@samsung.com>
>
OK, applied 1 to 3 patches into cleanup.
Thanks,
Kukjin
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables
@ 2013-12-09 21:00 ` Kukjin Kim
0 siblings, 0 replies; 16+ messages in thread
From: Kukjin Kim @ 2013-12-09 21:00 UTC (permalink / raw)
To: linux-arm-kernel
On 12/10/13 01:15, Tomasz Figa wrote:
> On Tuesday 10 of December 2013 00:11:40 Daniel Kurtz wrote:
>> Hi Tomasz,
>>
>> Thank you for the reviews.
>>
>> On Dec 9, 2013 5:15 AM, "Tomasz Figa"<t.figa@samsung.com> wrote:
>>>
>>> Hi Daniel,
>>>
>>> On Thursday 21 of November 2013 02:21:24 Daniel Kurtz wrote:
>>>> These tables are all immutable, make them const to save 4416 bytes of RAM.
>>>>
>>>> size arch/arm/mach-exynos/pmu.o
>>>> text data bss
>>>> 848 4420 4 // before
>>>> 5264 4 4 // after
>>>
>>> I'm not sure where the mentioned saving of RAM is. Moving data between
>>> sections is not supposed to make it use less memory, I believe.
>>
>> You are correct. This was my misunderstanding from doing too much
>> work with microcontrollers, where .text sections are accessed in place
>> from FLASH for code and const data, but .data memory is copied from a
>> FLASH section to a second RAM section at init for access at runtime.
>> Most modern Linux systems copy/decompress their code and data sections
>> from external storage to RAM anyway, so there is no actual memory
>> savings (except potentially the compiler may be able to optimize a bit
>> more with the const hint).
>>
>>>
>>> Anyway, it's a good practice to mark constant data as const, to disallow
>>> changing them at runtime by mistake, so the patch is fine. Except some
>>> issues I commented on inline.
>>
>> Were there supposed to be inline comments? I don't see any.
>
> Oops, sorry for this, forgot to remove the last sentence. I initially had
> one question about the constant pointers below, but I read through the
> full code again and answered it myself.
>
> The patch is fine.
>
> Reviewed-by: Tomasz Figa<t.figa@samsung.com>
>
OK, applied 1 to 3 patches into cleanup.
Thanks,
Kukjin
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2013-12-09 21:00 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-11-20 18:21 [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables Daniel Kurtz
2013-11-20 18:21 ` Daniel Kurtz
2013-11-20 18:21 ` [PATCH 2/3] ARM: SAMSUNG: Let s3c_pm_do_restore_*() take const sleep_save Daniel Kurtz
2013-11-20 18:21 ` Daniel Kurtz
2013-12-09 13:27 ` Tomasz Figa
2013-12-09 13:27 ` Tomasz Figa
2013-11-20 18:21 ` [PATCH 3/3] ARM: EXYNOS: Constify clksrc immutable register restore tables Daniel Kurtz
2013-11-20 18:21 ` Daniel Kurtz
2013-12-09 13:14 ` [PATCH 1/3] ARM: EXYNOS: pmu: Constify data tables Tomasz Figa
2013-12-09 13:14 ` Tomasz Figa
2013-12-09 16:11 ` Daniel Kurtz
2013-12-09 16:11 ` Daniel Kurtz
2013-12-09 16:15 ` Tomasz Figa
2013-12-09 16:15 ` Tomasz Figa
2013-12-09 21:00 ` Kukjin Kim
2013-12-09 21:00 ` Kukjin Kim
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.