* [PATCH 0/3] ARM: EXYNOS: Trivial cleanup in mach-exynos
@ 2016-02-22 10:03 ` Pankaj Dubey
0 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, k.kozlowski, b.zolnierkie, daniel.lezcano, thomas.ab,
linux, Pankaj Dubey
This patch series cleans up mach-exynos files with resepct to unused header file inclusion,
leftover redundant register offset definition. Also corrects comment section at few places.
Pankaj Dubey (3):
ARM: EXYNOS: correct header comment in Kconfig file
ARM: EXYNOS: remove unused register offset definition
ARM: EXYNOS: cleanup header files inclusion
arch/arm/mach-exynos/Kconfig | 2 +-
arch/arm/mach-exynos/common.h | 1 -
arch/arm/mach-exynos/exynos.c | 5 -----
arch/arm/mach-exynos/firmware.c | 2 --
arch/arm/mach-exynos/include/mach/map.h | 9 +--------
arch/arm/mach-exynos/platsmp.c | 1 -
arch/arm/mach-exynos/pm.c | 4 +---
arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
8 files changed, 3 insertions(+), 22 deletions(-)
--
2.4.5
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 0/3] ARM: EXYNOS: Trivial cleanup in mach-exynos
@ 2016-02-22 10:03 ` Pankaj Dubey
0 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-arm-kernel
This patch series cleans up mach-exynos files with resepct to unused header file inclusion,
leftover redundant register offset definition. Also corrects comment section at few places.
Pankaj Dubey (3):
ARM: EXYNOS: correct header comment in Kconfig file
ARM: EXYNOS: remove unused register offset definition
ARM: EXYNOS: cleanup header files inclusion
arch/arm/mach-exynos/Kconfig | 2 +-
arch/arm/mach-exynos/common.h | 1 -
arch/arm/mach-exynos/exynos.c | 5 -----
arch/arm/mach-exynos/firmware.c | 2 --
arch/arm/mach-exynos/include/mach/map.h | 9 +--------
arch/arm/mach-exynos/platsmp.c | 1 -
arch/arm/mach-exynos/pm.c | 4 +---
arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
8 files changed, 3 insertions(+), 22 deletions(-)
--
2.4.5
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: correct header comment in Kconfig file
2016-02-22 10:03 ` Pankaj Dubey
@ 2016-02-22 10:03 ` Pankaj Dubey
-1 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, k.kozlowski, b.zolnierkie, daniel.lezcano, thomas.ab,
linux, Pankaj Dubey
This patch corrects header comment of Kconfig file by changing EXYNOS4 to
EXYNOS.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index eb26986..71ef48f 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -5,7 +5,7 @@
#
# Licensed under GPLv2
-# Configuration options for the EXYNOS4
+# Configuration options for the EXYNOS
menuconfig ARCH_EXYNOS
bool "Samsung EXYNOS"
--
2.4.5
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: correct header comment in Kconfig file
@ 2016-02-22 10:03 ` Pankaj Dubey
0 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-arm-kernel
This patch corrects header comment of Kconfig file by changing EXYNOS4 to
EXYNOS.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
index eb26986..71ef48f 100644
--- a/arch/arm/mach-exynos/Kconfig
+++ b/arch/arm/mach-exynos/Kconfig
@@ -5,7 +5,7 @@
#
# Licensed under GPLv2
-# Configuration options for the EXYNOS4
+# Configuration options for the EXYNOS
menuconfig ARCH_EXYNOS
bool "Samsung EXYNOS"
--
2.4.5
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
2016-02-22 10:03 ` Pankaj Dubey
@ 2016-02-22 10:03 ` Pankaj Dubey
-1 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, k.kozlowski, b.zolnierkie, daniel.lezcano, thomas.ab,
linux, Pankaj Dubey
This patch cleans up map.h by removing unused register offset
and changing EXYNOS4 to EXYNOS in header comment section.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/include/mach/map.h | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 351e839..c48ba4f 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -2,7 +2,7 @@
* Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
*
- * EXYNOS4 - Memory map definitions
+ * EXYNOS - Memory map definitions
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -14,12 +14,6 @@
#include <plat/map-base.h>
-/*
- * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
- * So need to define it, and here is to avoid redefinition warning.
- */
-#define S3C_UART_OFFSET (0x10000)
-
#include <plat/map-s5p.h>
#define EXYNOS_PA_CHIPID 0x10000000
@@ -30,6 +24,5 @@
#define EXYNOS4_PA_DMC1 0x10410000
#define EXYNOS4_PA_COREPERI 0x10500000
-#define EXYNOS4_PA_L2CC 0x10502000
#endif /* __ASM_ARCH_MAP_H */
--
2.4.5
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
@ 2016-02-22 10:03 ` Pankaj Dubey
0 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-arm-kernel
This patch cleans up map.h by removing unused register offset
and changing EXYNOS4 to EXYNOS in header comment section.
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/include/mach/map.h | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
index 351e839..c48ba4f 100644
--- a/arch/arm/mach-exynos/include/mach/map.h
+++ b/arch/arm/mach-exynos/include/mach/map.h
@@ -2,7 +2,7 @@
* Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
* http://www.samsung.com/
*
- * EXYNOS4 - Memory map definitions
+ * EXYNOS - Memory map definitions
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -14,12 +14,6 @@
#include <plat/map-base.h>
-/*
- * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
- * So need to define it, and here is to avoid redefinition warning.
- */
-#define S3C_UART_OFFSET (0x10000)
-
#include <plat/map-s5p.h>
#define EXYNOS_PA_CHIPID 0x10000000
@@ -30,6 +24,5 @@
#define EXYNOS4_PA_DMC1 0x10410000
#define EXYNOS4_PA_COREPERI 0x10500000
-#define EXYNOS4_PA_L2CC 0x10502000
#endif /* __ASM_ARCH_MAP_H */
--
2.4.5
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
2016-02-22 10:03 ` Pankaj Dubey
@ 2016-02-22 10:03 ` Pankaj Dubey
-1 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, k.kozlowski, b.zolnierkie, daniel.lezcano, thomas.ab,
linux, Pankaj Dubey
This includes trivial cleanup in exynos files such as
- remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
firmware.c, pm.c.
- move inclusion of of.h from common.h to pm.c where it is really
required
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/common.h | 1 -
arch/arm/mach-exynos/exynos.c | 5 -----
arch/arm/mach-exynos/firmware.c | 2 --
arch/arm/mach-exynos/platsmp.c | 1 -
arch/arm/mach-exynos/pm.c | 4 +---
arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
6 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index e349a03..5365bf1 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -12,7 +12,6 @@
#ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
#define __ARCH_ARM_MACH_EXYNOS_COMMON_H
-#include <linux/of.h>
#include <linux/platform_data/cpuidle-exynos.h>
#define EXYNOS3250_SOC_ID 0xE3472000
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 99947ad..54262a1 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -11,14 +11,10 @@
#include <linux/init.h>
#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/serial_s3c.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_fdt.h>
#include <linux/of_platform.h>
-#include <linux/platform_device.h>
-#include <linux/pm_domain.h>
#include <linux/irqchip.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
@@ -26,7 +22,6 @@
#include <asm/hardware/cache-l2x0.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
-#include <asm/memory.h>
#include <mach/map.h>
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 111cfbf..1bfd1b0 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -20,8 +20,6 @@
#include <asm/hardware/cache-l2x0.h>
#include <asm/suspend.h>
-#include <mach/map.h>
-
#include "common.h"
#include "smc.h"
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index da46c63..85c3be6 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -15,7 +15,6 @@
#include <linux/init.h>
#include <linux/errno.h>
#include <linux/delay.h>
-#include <linux/device.h>
#include <linux/jiffies.h>
#include <linux/smp.h>
#include <linux/io.h>
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index b9b9186..c43b776 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -17,7 +17,7 @@
#include <linux/suspend.h>
#include <linux/cpu_pm.h>
#include <linux/io.h>
-#include <linux/err.h>
+#include <linux/of.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
#include <linux/soc/samsung/exynos-pmu.h>
@@ -28,8 +28,6 @@
#include <mach/map.h>
-#include <plat/pm-common.h>
-
#include "common.h"
static inline void __iomem *exynos_boot_vector_addr(void)
diff --git a/arch/arm/mach-exynos/s5p-dev-mfc.c b/arch/arm/mach-exynos/s5p-dev-mfc.c
index 0b04b6b..8ef1f3e 100644
--- a/arch/arm/mach-exynos/s5p-dev-mfc.c
+++ b/arch/arm/mach-exynos/s5p-dev-mfc.c
@@ -9,7 +9,6 @@
*/
#include <linux/kernel.h>
-#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
#include <linux/memblock.h>
--
2.4.5
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
@ 2016-02-22 10:03 ` Pankaj Dubey
0 siblings, 0 replies; 28+ messages in thread
From: Pankaj Dubey @ 2016-02-22 10:03 UTC (permalink / raw)
To: linux-arm-kernel
This includes trivial cleanup in exynos files such as
- remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
firmware.c, pm.c.
- move inclusion of of.h from common.h to pm.c where it is really
required
Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
---
arch/arm/mach-exynos/common.h | 1 -
arch/arm/mach-exynos/exynos.c | 5 -----
arch/arm/mach-exynos/firmware.c | 2 --
arch/arm/mach-exynos/platsmp.c | 1 -
arch/arm/mach-exynos/pm.c | 4 +---
arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
6 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
index e349a03..5365bf1 100644
--- a/arch/arm/mach-exynos/common.h
+++ b/arch/arm/mach-exynos/common.h
@@ -12,7 +12,6 @@
#ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
#define __ARCH_ARM_MACH_EXYNOS_COMMON_H
-#include <linux/of.h>
#include <linux/platform_data/cpuidle-exynos.h>
#define EXYNOS3250_SOC_ID 0xE3472000
diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
index 99947ad..54262a1 100644
--- a/arch/arm/mach-exynos/exynos.c
+++ b/arch/arm/mach-exynos/exynos.c
@@ -11,14 +11,10 @@
#include <linux/init.h>
#include <linux/io.h>
-#include <linux/kernel.h>
-#include <linux/serial_s3c.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_fdt.h>
#include <linux/of_platform.h>
-#include <linux/platform_device.h>
-#include <linux/pm_domain.h>
#include <linux/irqchip.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
@@ -26,7 +22,6 @@
#include <asm/hardware/cache-l2x0.h>
#include <asm/mach/arch.h>
#include <asm/mach/map.h>
-#include <asm/memory.h>
#include <mach/map.h>
diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
index 111cfbf..1bfd1b0 100644
--- a/arch/arm/mach-exynos/firmware.c
+++ b/arch/arm/mach-exynos/firmware.c
@@ -20,8 +20,6 @@
#include <asm/hardware/cache-l2x0.h>
#include <asm/suspend.h>
-#include <mach/map.h>
-
#include "common.h"
#include "smc.h"
diff --git a/arch/arm/mach-exynos/platsmp.c b/arch/arm/mach-exynos/platsmp.c
index da46c63..85c3be6 100644
--- a/arch/arm/mach-exynos/platsmp.c
+++ b/arch/arm/mach-exynos/platsmp.c
@@ -15,7 +15,6 @@
#include <linux/init.h>
#include <linux/errno.h>
#include <linux/delay.h>
-#include <linux/device.h>
#include <linux/jiffies.h>
#include <linux/smp.h>
#include <linux/io.h>
diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
index b9b9186..c43b776 100644
--- a/arch/arm/mach-exynos/pm.c
+++ b/arch/arm/mach-exynos/pm.c
@@ -17,7 +17,7 @@
#include <linux/suspend.h>
#include <linux/cpu_pm.h>
#include <linux/io.h>
-#include <linux/err.h>
+#include <linux/of.h>
#include <linux/soc/samsung/exynos-regs-pmu.h>
#include <linux/soc/samsung/exynos-pmu.h>
@@ -28,8 +28,6 @@
#include <mach/map.h>
-#include <plat/pm-common.h>
-
#include "common.h"
static inline void __iomem *exynos_boot_vector_addr(void)
diff --git a/arch/arm/mach-exynos/s5p-dev-mfc.c b/arch/arm/mach-exynos/s5p-dev-mfc.c
index 0b04b6b..8ef1f3e 100644
--- a/arch/arm/mach-exynos/s5p-dev-mfc.c
+++ b/arch/arm/mach-exynos/s5p-dev-mfc.c
@@ -9,7 +9,6 @@
*/
#include <linux/kernel.h>
-#include <linux/interrupt.h>
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
#include <linux/memblock.h>
--
2.4.5
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 1/3] ARM: EXYNOS: correct header comment in Kconfig file
2016-02-22 10:03 ` Pankaj Dubey
@ 2016-02-23 0:01 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 0:01 UTC (permalink / raw)
To: Pankaj Dubey, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On 22.02.2016 19:03, Pankaj Dubey wrote:
> This patch corrects header comment of Kconfig file by changing EXYNOS4 to
> EXYNOS.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks, applied.
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/3] ARM: EXYNOS: correct header comment in Kconfig file
@ 2016-02-23 0:01 ` Krzysztof Kozlowski
0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 0:01 UTC (permalink / raw)
To: linux-arm-kernel
On 22.02.2016 19:03, Pankaj Dubey wrote:
> This patch corrects header comment of Kconfig file by changing EXYNOS4 to
> EXYNOS.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks, applied.
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
2016-02-22 10:03 ` Pankaj Dubey
@ 2016-02-23 0:03 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 0:03 UTC (permalink / raw)
To: Pankaj Dubey, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On 22.02.2016 19:03, Pankaj Dubey wrote:
> This patch cleans up map.h by removing unused register offset
> and changing EXYNOS4 to EXYNOS in header comment section.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index 351e839..c48ba4f 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -2,7 +2,7 @@
> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> * http://www.samsung.com/
> *
> - * EXYNOS4 - Memory map definitions
> + * EXYNOS - Memory map definitions
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -14,12 +14,6 @@
>
> #include <plat/map-base.h>
>
> -/*
> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
> - * So need to define it, and here is to avoid redefinition warning.
> - */
> -#define S3C_UART_OFFSET (0x10000)
> -
> #include <plat/map-s5p.h>
This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
@ 2016-02-23 0:03 ` Krzysztof Kozlowski
0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 0:03 UTC (permalink / raw)
To: linux-arm-kernel
On 22.02.2016 19:03, Pankaj Dubey wrote:
> This patch cleans up map.h by removing unused register offset
> and changing EXYNOS4 to EXYNOS in header comment section.
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
> 1 file changed, 1 insertion(+), 8 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
> index 351e839..c48ba4f 100644
> --- a/arch/arm/mach-exynos/include/mach/map.h
> +++ b/arch/arm/mach-exynos/include/mach/map.h
> @@ -2,7 +2,7 @@
> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
> * http://www.samsung.com/
> *
> - * EXYNOS4 - Memory map definitions
> + * EXYNOS - Memory map definitions
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License version 2 as
> @@ -14,12 +14,6 @@
>
> #include <plat/map-base.h>
>
> -/*
> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
> - * So need to define it, and here is to avoid redefinition warning.
> - */
> -#define S3C_UART_OFFSET (0x10000)
> -
> #include <plat/map-s5p.h>
This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
2016-02-22 10:03 ` Pankaj Dubey
@ 2016-02-23 0:09 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 0:09 UTC (permalink / raw)
To: Pankaj Dubey, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On 22.02.2016 19:03, Pankaj Dubey wrote:
> This includes trivial cleanup in exynos files such as
> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
> firmware.c, pm.c.
> - move inclusion of of.h from common.h to pm.c where it is really
> required
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/common.h | 1 -
> arch/arm/mach-exynos/exynos.c | 5 -----
> arch/arm/mach-exynos/firmware.c | 2 --
> arch/arm/mach-exynos/platsmp.c | 1 -
> arch/arm/mach-exynos/pm.c | 4 +---
> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
> 6 files changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index e349a03..5365bf1 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -12,7 +12,6 @@
> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>
> -#include <linux/of.h>
> #include <linux/platform_data/cpuidle-exynos.h>
>
> #define EXYNOS3250_SOC_ID 0xE3472000
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 99947ad..54262a1 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -11,14 +11,10 @@
>
> #include <linux/init.h>
> #include <linux/io.h>
> -#include <linux/kernel.h>
> -#include <linux/serial_s3c.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_fdt.h>
> #include <linux/of_platform.h>
> -#include <linux/platform_device.h>
platform_device is actually used in that file. It looks unused because
it is pulled by of_platform, but it makes sense to include it here.
Rest looks good.
BR,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
@ 2016-02-23 0:09 ` Krzysztof Kozlowski
0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 0:09 UTC (permalink / raw)
To: linux-arm-kernel
On 22.02.2016 19:03, Pankaj Dubey wrote:
> This includes trivial cleanup in exynos files such as
> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
> firmware.c, pm.c.
> - move inclusion of of.h from common.h to pm.c where it is really
> required
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/common.h | 1 -
> arch/arm/mach-exynos/exynos.c | 5 -----
> arch/arm/mach-exynos/firmware.c | 2 --
> arch/arm/mach-exynos/platsmp.c | 1 -
> arch/arm/mach-exynos/pm.c | 4 +---
> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
> 6 files changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
> index e349a03..5365bf1 100644
> --- a/arch/arm/mach-exynos/common.h
> +++ b/arch/arm/mach-exynos/common.h
> @@ -12,7 +12,6 @@
> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>
> -#include <linux/of.h>
> #include <linux/platform_data/cpuidle-exynos.h>
>
> #define EXYNOS3250_SOC_ID 0xE3472000
> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
> index 99947ad..54262a1 100644
> --- a/arch/arm/mach-exynos/exynos.c
> +++ b/arch/arm/mach-exynos/exynos.c
> @@ -11,14 +11,10 @@
>
> #include <linux/init.h>
> #include <linux/io.h>
> -#include <linux/kernel.h>
> -#include <linux/serial_s3c.h>
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_fdt.h>
> #include <linux/of_platform.h>
> -#include <linux/platform_device.h>
platform_device is actually used in that file. It looks unused because
it is pulled by of_platform, but it makes sense to include it here.
Rest looks good.
BR,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
2016-02-23 0:03 ` Krzysztof Kozlowski
@ 2016-02-23 9:11 ` pankaj.dubey
-1 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-23 9:11 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-samsung-soc, linux-arm-kernel
Cc: thomas.ab, kgene.kim, linux, daniel.lezcano, b.zolnierkie
Hi Krzysztof,
On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
> On 22.02.2016 19:03, Pankaj Dubey wrote:
>> This patch cleans up map.h by removing unused register offset
>> and changing EXYNOS4 to EXYNOS in header comment section.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>> index 351e839..c48ba4f 100644
>> --- a/arch/arm/mach-exynos/include/mach/map.h
>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>> @@ -2,7 +2,7 @@
>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>> * http://www.samsung.com/
>> *
>> - * EXYNOS4 - Memory map definitions
>> + * EXYNOS - Memory map definitions
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -14,12 +14,6 @@
>>
>> #include <plat/map-base.h>
>>
>> -/*
>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>> - * So need to define it, and here is to avoid redefinition warning.
>> - */
>> -#define S3C_UART_OFFSET (0x10000)
>> -
>> #include <plat/map-s5p.h>
>
> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>
Actually it's just defined there but not getting used anywhere. In fact
we can remove it from there also. So I submitted another patch to
cleanup such redundant register definitions from plat/map-s5p.h.
Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
from map-s3c.h. I have compiled all related platform defconfigs and
found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
map.h.
Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
@ 2016-02-23 9:11 ` pankaj.dubey
0 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-23 9:11 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
> On 22.02.2016 19:03, Pankaj Dubey wrote:
>> This patch cleans up map.h by removing unused register offset
>> and changing EXYNOS4 to EXYNOS in header comment section.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>> index 351e839..c48ba4f 100644
>> --- a/arch/arm/mach-exynos/include/mach/map.h
>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>> @@ -2,7 +2,7 @@
>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>> * http://www.samsung.com/
>> *
>> - * EXYNOS4 - Memory map definitions
>> + * EXYNOS - Memory map definitions
>> *
>> * This program is free software; you can redistribute it and/or modify
>> * it under the terms of the GNU General Public License version 2 as
>> @@ -14,12 +14,6 @@
>>
>> #include <plat/map-base.h>
>>
>> -/*
>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>> - * So need to define it, and here is to avoid redefinition warning.
>> - */
>> -#define S3C_UART_OFFSET (0x10000)
>> -
>> #include <plat/map-s5p.h>
>
> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>
Actually it's just defined there but not getting used anywhere. In fact
we can remove it from there also. So I submitted another patch to
cleanup such redundant register definitions from plat/map-s5p.h.
Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
from map-s3c.h. I have compiled all related platform defconfigs and
found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
map.h.
Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
2016-02-23 0:09 ` Krzysztof Kozlowski
@ 2016-02-23 9:17 ` pankaj.dubey
-1 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-23 9:17 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
Hi Krzysztof,
On Tuesday 23 February 2016 05:39 AM, Krzysztof Kozlowski wrote:
> On 22.02.2016 19:03, Pankaj Dubey wrote:
>> This includes trivial cleanup in exynos files such as
>> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
>> firmware.c, pm.c.
>> - move inclusion of of.h from common.h to pm.c where it is really
>> required
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/common.h | 1 -
>> arch/arm/mach-exynos/exynos.c | 5 -----
>> arch/arm/mach-exynos/firmware.c | 2 --
>> arch/arm/mach-exynos/platsmp.c | 1 -
>> arch/arm/mach-exynos/pm.c | 4 +---
>> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
>> 6 files changed, 1 insertion(+), 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>> index e349a03..5365bf1 100644
>> --- a/arch/arm/mach-exynos/common.h
>> +++ b/arch/arm/mach-exynos/common.h
>> @@ -12,7 +12,6 @@
>> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
>> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>
>> -#include <linux/of.h>
>> #include <linux/platform_data/cpuidle-exynos.h>
>>
>> #define EXYNOS3250_SOC_ID 0xE3472000
>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>> index 99947ad..54262a1 100644
>> --- a/arch/arm/mach-exynos/exynos.c
>> +++ b/arch/arm/mach-exynos/exynos.c
>> @@ -11,14 +11,10 @@
>>
>> #include <linux/init.h>
>> #include <linux/io.h>
>> -#include <linux/kernel.h>
>> -#include <linux/serial_s3c.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> #include <linux/of_fdt.h>
>> #include <linux/of_platform.h>
>> -#include <linux/platform_device.h>
>
> platform_device is actually used in that file. It looks unused because
> it is pulled by of_platform, but it makes sense to include it here.
>
Thanks for review.
Well I tried to grep for of_platform.h and platform_device.h in
arch/arm/ and out of 88 files including of_platform.h only 16 files are
including platform_device.h also. So majority do not include both of
them even do they need. So majority of files both are not included. So
If we got with majority it's not required. What's your opinion?
Thanks,
Pankaj Dubey
> Rest looks good.
>
> BR,
> Krzysztof
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
@ 2016-02-23 9:17 ` pankaj.dubey
0 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-23 9:17 UTC (permalink / raw)
To: linux-arm-kernel
Hi Krzysztof,
On Tuesday 23 February 2016 05:39 AM, Krzysztof Kozlowski wrote:
> On 22.02.2016 19:03, Pankaj Dubey wrote:
>> This includes trivial cleanup in exynos files such as
>> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
>> firmware.c, pm.c.
>> - move inclusion of of.h from common.h to pm.c where it is really
>> required
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/common.h | 1 -
>> arch/arm/mach-exynos/exynos.c | 5 -----
>> arch/arm/mach-exynos/firmware.c | 2 --
>> arch/arm/mach-exynos/platsmp.c | 1 -
>> arch/arm/mach-exynos/pm.c | 4 +---
>> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
>> 6 files changed, 1 insertion(+), 13 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>> index e349a03..5365bf1 100644
>> --- a/arch/arm/mach-exynos/common.h
>> +++ b/arch/arm/mach-exynos/common.h
>> @@ -12,7 +12,6 @@
>> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
>> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>
>> -#include <linux/of.h>
>> #include <linux/platform_data/cpuidle-exynos.h>
>>
>> #define EXYNOS3250_SOC_ID 0xE3472000
>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>> index 99947ad..54262a1 100644
>> --- a/arch/arm/mach-exynos/exynos.c
>> +++ b/arch/arm/mach-exynos/exynos.c
>> @@ -11,14 +11,10 @@
>>
>> #include <linux/init.h>
>> #include <linux/io.h>
>> -#include <linux/kernel.h>
>> -#include <linux/serial_s3c.h>
>> #include <linux/of.h>
>> #include <linux/of_address.h>
>> #include <linux/of_fdt.h>
>> #include <linux/of_platform.h>
>> -#include <linux/platform_device.h>
>
> platform_device is actually used in that file. It looks unused because
> it is pulled by of_platform, but it makes sense to include it here.
>
Thanks for review.
Well I tried to grep for of_platform.h and platform_device.h in
arch/arm/ and out of 88 files including of_platform.h only 16 files are
including platform_device.h also. So majority do not include both of
them even do they need. So majority of files both are not included. So
If we got with majority it's not required. What's your opinion?
Thanks,
Pankaj Dubey
> Rest looks good.
>
> BR,
> Krzysztof
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
2016-02-23 9:11 ` pankaj.dubey
@ 2016-02-23 23:47 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 23:47 UTC (permalink / raw)
To: pankaj.dubey, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On 23.02.2016 18:11, pankaj.dubey wrote:
> Hi Krzysztof,
>
> On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>> This patch cleans up map.h by removing unused register offset
>>> and changing EXYNOS4 to EXYNOS in header comment section.
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>>> index 351e839..c48ba4f 100644
>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>> @@ -2,7 +2,7 @@
>>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>>> * http://www.samsung.com/
>>> *
>>> - * EXYNOS4 - Memory map definitions
>>> + * EXYNOS - Memory map definitions
>>> *
>>> * This program is free software; you can redistribute it and/or modify
>>> * it under the terms of the GNU General Public License version 2 as
>>> @@ -14,12 +14,6 @@
>>>
>>> #include <plat/map-base.h>
>>>
>>> -/*
>>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>>> - * So need to define it, and here is to avoid redefinition warning.
>>> - */
>>> -#define S3C_UART_OFFSET (0x10000)
>>> -
>>> #include <plat/map-s5p.h>
>>
>> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>>
>
> Actually it's just defined there but not getting used anywhere. In fact
> we can remove it from there also.
The point is that your patch introduce changes. Before S3C_UART_OFFSET
was defined to 0x10000, after (because of #ifndef) it will be 0x400.
Description of "unused" is not accurate. Unused stuff does not impact
anything.
> So I submitted another patch to
> cleanup such redundant register definitions from plat/map-s5p.h.
That would be good.
> Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
> from map-s3c.h. I have compiled all related platform defconfigs and
> found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
> map.h.
The compilation is not a test. You compiled something like this:
-#define S3C_UART_OFFSET (0x10000)
#ifndef S3C_UART_OFFSET
#define S3C_UART_OFFSET (0x400)
#endif
... and found no errors. Of course there are no compilation errors! But
the value has changed!
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
@ 2016-02-23 23:47 ` Krzysztof Kozlowski
0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-23 23:47 UTC (permalink / raw)
To: linux-arm-kernel
On 23.02.2016 18:11, pankaj.dubey wrote:
> Hi Krzysztof,
>
> On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>> This patch cleans up map.h by removing unused register offset
>>> and changing EXYNOS4 to EXYNOS in header comment section.
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>>> index 351e839..c48ba4f 100644
>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>> @@ -2,7 +2,7 @@
>>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>>> * http://www.samsung.com/
>>> *
>>> - * EXYNOS4 - Memory map definitions
>>> + * EXYNOS - Memory map definitions
>>> *
>>> * This program is free software; you can redistribute it and/or modify
>>> * it under the terms of the GNU General Public License version 2 as
>>> @@ -14,12 +14,6 @@
>>>
>>> #include <plat/map-base.h>
>>>
>>> -/*
>>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>>> - * So need to define it, and here is to avoid redefinition warning.
>>> - */
>>> -#define S3C_UART_OFFSET (0x10000)
>>> -
>>> #include <plat/map-s5p.h>
>>
>> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>>
>
> Actually it's just defined there but not getting used anywhere. In fact
> we can remove it from there also.
The point is that your patch introduce changes. Before S3C_UART_OFFSET
was defined to 0x10000, after (because of #ifndef) it will be 0x400.
Description of "unused" is not accurate. Unused stuff does not impact
anything.
> So I submitted another patch to
> cleanup such redundant register definitions from plat/map-s5p.h.
That would be good.
> Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
> from map-s3c.h. I have compiled all related platform defconfigs and
> found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
> map.h.
The compilation is not a test. You compiled something like this:
-#define S3C_UART_OFFSET (0x10000)
#ifndef S3C_UART_OFFSET
#define S3C_UART_OFFSET (0x400)
#endif
... and found no errors. Of course there are no compilation errors! But
the value has changed!
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
2016-02-23 9:17 ` pankaj.dubey
@ 2016-02-24 1:02 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-24 1:02 UTC (permalink / raw)
To: pankaj.dubey, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On 23.02.2016 18:17, pankaj.dubey wrote:
> Hi Krzysztof,
>
> On Tuesday 23 February 2016 05:39 AM, Krzysztof Kozlowski wrote:
>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>> This includes trivial cleanup in exynos files such as
>>> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
>>> firmware.c, pm.c.
>>> - move inclusion of of.h from common.h to pm.c where it is really
>>> required
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>> arch/arm/mach-exynos/common.h | 1 -
>>> arch/arm/mach-exynos/exynos.c | 5 -----
>>> arch/arm/mach-exynos/firmware.c | 2 --
>>> arch/arm/mach-exynos/platsmp.c | 1 -
>>> arch/arm/mach-exynos/pm.c | 4 +---
>>> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
>>> 6 files changed, 1 insertion(+), 13 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>>> index e349a03..5365bf1 100644
>>> --- a/arch/arm/mach-exynos/common.h
>>> +++ b/arch/arm/mach-exynos/common.h
>>> @@ -12,7 +12,6 @@
>>> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>>
>>> -#include <linux/of.h>
>>> #include <linux/platform_data/cpuidle-exynos.h>
>>>
>>> #define EXYNOS3250_SOC_ID 0xE3472000
>>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>>> index 99947ad..54262a1 100644
>>> --- a/arch/arm/mach-exynos/exynos.c
>>> +++ b/arch/arm/mach-exynos/exynos.c
>>> @@ -11,14 +11,10 @@
>>>
>>> #include <linux/init.h>
>>> #include <linux/io.h>
>>> -#include <linux/kernel.h>
>>> -#include <linux/serial_s3c.h>
>>> #include <linux/of.h>
>>> #include <linux/of_address.h>
>>> #include <linux/of_fdt.h>
>>> #include <linux/of_platform.h>
>>> -#include <linux/platform_device.h>
>>
>> platform_device is actually used in that file. It looks unused because
>> it is pulled by of_platform, but it makes sense to include it here.
>>
> Thanks for review.
>
> Well I tried to grep for of_platform.h and platform_device.h in
> arch/arm/ and out of 88 files including of_platform.h only 16 files are
> including platform_device.h also. So majority do not include both of
> them even do they need. So majority of files both are not included. So
> If we got with majority it's not required. What's your opinion?
Actually most of files including of_platform.h and not including
platform_device.h just do not require the second one (I checked like 20
of them and only one was referring to platform device function... of
course maybe my random selection of 20 files was not random enough :) ).
They do not directly reference platform device stuff.
In that file the platform_device is used so the header may stay.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
@ 2016-02-24 1:02 ` Krzysztof Kozlowski
0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-24 1:02 UTC (permalink / raw)
To: linux-arm-kernel
On 23.02.2016 18:17, pankaj.dubey wrote:
> Hi Krzysztof,
>
> On Tuesday 23 February 2016 05:39 AM, Krzysztof Kozlowski wrote:
>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>> This includes trivial cleanup in exynos files such as
>>> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
>>> firmware.c, pm.c.
>>> - move inclusion of of.h from common.h to pm.c where it is really
>>> required
>>>
>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>> ---
>>> arch/arm/mach-exynos/common.h | 1 -
>>> arch/arm/mach-exynos/exynos.c | 5 -----
>>> arch/arm/mach-exynos/firmware.c | 2 --
>>> arch/arm/mach-exynos/platsmp.c | 1 -
>>> arch/arm/mach-exynos/pm.c | 4 +---
>>> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
>>> 6 files changed, 1 insertion(+), 13 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>>> index e349a03..5365bf1 100644
>>> --- a/arch/arm/mach-exynos/common.h
>>> +++ b/arch/arm/mach-exynos/common.h
>>> @@ -12,7 +12,6 @@
>>> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>>
>>> -#include <linux/of.h>
>>> #include <linux/platform_data/cpuidle-exynos.h>
>>>
>>> #define EXYNOS3250_SOC_ID 0xE3472000
>>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>>> index 99947ad..54262a1 100644
>>> --- a/arch/arm/mach-exynos/exynos.c
>>> +++ b/arch/arm/mach-exynos/exynos.c
>>> @@ -11,14 +11,10 @@
>>>
>>> #include <linux/init.h>
>>> #include <linux/io.h>
>>> -#include <linux/kernel.h>
>>> -#include <linux/serial_s3c.h>
>>> #include <linux/of.h>
>>> #include <linux/of_address.h>
>>> #include <linux/of_fdt.h>
>>> #include <linux/of_platform.h>
>>> -#include <linux/platform_device.h>
>>
>> platform_device is actually used in that file. It looks unused because
>> it is pulled by of_platform, but it makes sense to include it here.
>>
> Thanks for review.
>
> Well I tried to grep for of_platform.h and platform_device.h in
> arch/arm/ and out of 88 files including of_platform.h only 16 files are
> including platform_device.h also. So majority do not include both of
> them even do they need. So majority of files both are not included. So
> If we got with majority it's not required. What's your opinion?
Actually most of files including of_platform.h and not including
platform_device.h just do not require the second one (I checked like 20
of them and only one was referring to platform device function... of
course maybe my random selection of 20 files was not random enough :) ).
They do not directly reference platform device stuff.
In that file the platform_device is used so the header may stay.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
2016-02-24 1:02 ` Krzysztof Kozlowski
@ 2016-02-24 4:01 ` pankaj.dubey
-1 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-24 4:01 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On Wednesday 24 February 2016 06:32 AM, Krzysztof Kozlowski wrote:
> On 23.02.2016 18:17, pankaj.dubey wrote:
>> Hi Krzysztof,
>>
>> On Tuesday 23 February 2016 05:39 AM, Krzysztof Kozlowski wrote:
>>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>>> This includes trivial cleanup in exynos files such as
>>>> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
>>>> firmware.c, pm.c.
>>>> - move inclusion of of.h from common.h to pm.c where it is really
>>>> required
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>> arch/arm/mach-exynos/common.h | 1 -
>>>> arch/arm/mach-exynos/exynos.c | 5 -----
>>>> arch/arm/mach-exynos/firmware.c | 2 --
>>>> arch/arm/mach-exynos/platsmp.c | 1 -
>>>> arch/arm/mach-exynos/pm.c | 4 +---
>>>> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
>>>> 6 files changed, 1 insertion(+), 13 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>>>> index e349a03..5365bf1 100644
>>>> --- a/arch/arm/mach-exynos/common.h
>>>> +++ b/arch/arm/mach-exynos/common.h
>>>> @@ -12,7 +12,6 @@
>>>> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>>> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>>>
>>>> -#include <linux/of.h>
>>>> #include <linux/platform_data/cpuidle-exynos.h>
>>>>
>>>> #define EXYNOS3250_SOC_ID 0xE3472000
>>>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>>>> index 99947ad..54262a1 100644
>>>> --- a/arch/arm/mach-exynos/exynos.c
>>>> +++ b/arch/arm/mach-exynos/exynos.c
>>>> @@ -11,14 +11,10 @@
>>>>
>>>> #include <linux/init.h>
>>>> #include <linux/io.h>
>>>> -#include <linux/kernel.h>
>>>> -#include <linux/serial_s3c.h>
>>>> #include <linux/of.h>
>>>> #include <linux/of_address.h>
>>>> #include <linux/of_fdt.h>
>>>> #include <linux/of_platform.h>
>>>> -#include <linux/platform_device.h>
>>>
>>> platform_device is actually used in that file. It looks unused because
>>> it is pulled by of_platform, but it makes sense to include it here.
>>>
>> Thanks for review.
>>
>> Well I tried to grep for of_platform.h and platform_device.h in
>> arch/arm/ and out of 88 files including of_platform.h only 16 files are
>> including platform_device.h also. So majority do not include both of
>> them even do they need. So majority of files both are not included. So
>> If we got with majority it's not required. What's your opinion?
>
> Actually most of files including of_platform.h and not including
> platform_device.h just do not require the second one (I checked like 20
> of them and only one was referring to platform device function... of
> course maybe my random selection of 20 files was not random enough :) ).
> They do not directly reference platform device stuff.
>
> In that file the platform_device is used so the header may stay.
OK. will resend this again with keeping back platform_device.h in
exynos.c file.
Thanks,
Pankaj Dubey
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion
@ 2016-02-24 4:01 ` pankaj.dubey
0 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-24 4:01 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 24 February 2016 06:32 AM, Krzysztof Kozlowski wrote:
> On 23.02.2016 18:17, pankaj.dubey wrote:
>> Hi Krzysztof,
>>
>> On Tuesday 23 February 2016 05:39 AM, Krzysztof Kozlowski wrote:
>>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>>> This includes trivial cleanup in exynos files such as
>>>> - remove unused header files inclusion from exynos.c, s5p-dev-mfc.c,
>>>> firmware.c, pm.c.
>>>> - move inclusion of of.h from common.h to pm.c where it is really
>>>> required
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>> arch/arm/mach-exynos/common.h | 1 -
>>>> arch/arm/mach-exynos/exynos.c | 5 -----
>>>> arch/arm/mach-exynos/firmware.c | 2 --
>>>> arch/arm/mach-exynos/platsmp.c | 1 -
>>>> arch/arm/mach-exynos/pm.c | 4 +---
>>>> arch/arm/mach-exynos/s5p-dev-mfc.c | 1 -
>>>> 6 files changed, 1 insertion(+), 13 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos/common.h b/arch/arm/mach-exynos/common.h
>>>> index e349a03..5365bf1 100644
>>>> --- a/arch/arm/mach-exynos/common.h
>>>> +++ b/arch/arm/mach-exynos/common.h
>>>> @@ -12,7 +12,6 @@
>>>> #ifndef __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>>> #define __ARCH_ARM_MACH_EXYNOS_COMMON_H
>>>>
>>>> -#include <linux/of.h>
>>>> #include <linux/platform_data/cpuidle-exynos.h>
>>>>
>>>> #define EXYNOS3250_SOC_ID 0xE3472000
>>>> diff --git a/arch/arm/mach-exynos/exynos.c b/arch/arm/mach-exynos/exynos.c
>>>> index 99947ad..54262a1 100644
>>>> --- a/arch/arm/mach-exynos/exynos.c
>>>> +++ b/arch/arm/mach-exynos/exynos.c
>>>> @@ -11,14 +11,10 @@
>>>>
>>>> #include <linux/init.h>
>>>> #include <linux/io.h>
>>>> -#include <linux/kernel.h>
>>>> -#include <linux/serial_s3c.h>
>>>> #include <linux/of.h>
>>>> #include <linux/of_address.h>
>>>> #include <linux/of_fdt.h>
>>>> #include <linux/of_platform.h>
>>>> -#include <linux/platform_device.h>
>>>
>>> platform_device is actually used in that file. It looks unused because
>>> it is pulled by of_platform, but it makes sense to include it here.
>>>
>> Thanks for review.
>>
>> Well I tried to grep for of_platform.h and platform_device.h in
>> arch/arm/ and out of 88 files including of_platform.h only 16 files are
>> including platform_device.h also. So majority do not include both of
>> them even do they need. So majority of files both are not included. So
>> If we got with majority it's not required. What's your opinion?
>
> Actually most of files including of_platform.h and not including
> platform_device.h just do not require the second one (I checked like 20
> of them and only one was referring to platform device function... of
> course maybe my random selection of 20 files was not random enough :) ).
> They do not directly reference platform device stuff.
>
> In that file the platform_device is used so the header may stay.
OK. will resend this again with keeping back platform_device.h in
exynos.c file.
Thanks,
Pankaj Dubey
>
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
2016-02-23 23:47 ` Krzysztof Kozlowski
@ 2016-02-24 8:23 ` pankaj.dubey
-1 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-24 8:23 UTC (permalink / raw)
To: Krzysztof Kozlowski, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On Wednesday 24 February 2016 05:17 AM, Krzysztof Kozlowski wrote:
> On 23.02.2016 18:11, pankaj.dubey wrote:
>> Hi Krzysztof,
>>
>> On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
>>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>>> This patch cleans up map.h by removing unused register offset
>>>> and changing EXYNOS4 to EXYNOS in header comment section.
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>>>> index 351e839..c48ba4f 100644
>>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>>> @@ -2,7 +2,7 @@
>>>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>>>> * http://www.samsung.com/
>>>> *
>>>> - * EXYNOS4 - Memory map definitions
>>>> + * EXYNOS - Memory map definitions
>>>> *
>>>> * This program is free software; you can redistribute it and/or modify
>>>> * it under the terms of the GNU General Public License version 2 as
>>>> @@ -14,12 +14,6 @@
>>>>
>>>> #include <plat/map-base.h>
>>>>
>>>> -/*
>>>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>>>> - * So need to define it, and here is to avoid redefinition warning.
>>>> - */
>>>> -#define S3C_UART_OFFSET (0x10000)
>>>> -
>>>> #include <plat/map-s5p.h>
>>>
>>> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>>>
>>
>> Actually it's just defined there but not getting used anywhere. In fact
>> we can remove it from there also.
>
> The point is that your patch introduce changes. Before S3C_UART_OFFSET
> was defined to 0x10000, after (because of #ifndef) it will be 0x400.
> Description of "unused" is not accurate. Unused stuff does not impact
> anything.
>
>> So I submitted another patch to
>> cleanup such redundant register definitions from plat/map-s5p.h.
>
> That would be good.
>
>> Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
>> from map-s3c.h. I have compiled all related platform defconfigs and
>> found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
>> map.h.
>
> The compilation is not a test. You compiled something like this:
> -#define S3C_UART_OFFSET (0x10000)
> #ifndef S3C_UART_OFFSET
> #define S3C_UART_OFFSET (0x400)
> #endif
>
> ... and found no errors. Of course there are no compilation errors! But
> the value has changed!
>
OK. I see, there are quite a few places these definitions are scattered
even though not used (not used in the sense what ever values these
macros are taking in code it's not used). Such as
mach-exynos/include/map.h, plat-samsung/include/plat/{map-s3c.h,
map-s5p.h} and mach-s3c24xx/include/mach/map.h. So I will remove all
these in one go.
Is this fine?
Also I have one doubt in case I am touching files across various mach
files and preparing a single patch what should be commit message
heading? Is commit starting with "ARM: samsung" fine? or anything more
appropriate you would suggest.
Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
@ 2016-02-24 8:23 ` pankaj.dubey
0 siblings, 0 replies; 28+ messages in thread
From: pankaj.dubey @ 2016-02-24 8:23 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 24 February 2016 05:17 AM, Krzysztof Kozlowski wrote:
> On 23.02.2016 18:11, pankaj.dubey wrote:
>> Hi Krzysztof,
>>
>> On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
>>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>>> This patch cleans up map.h by removing unused register offset
>>>> and changing EXYNOS4 to EXYNOS in header comment section.
>>>>
>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>> ---
>>>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>>>> index 351e839..c48ba4f 100644
>>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>>> @@ -2,7 +2,7 @@
>>>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>>>> * http://www.samsung.com/
>>>> *
>>>> - * EXYNOS4 - Memory map definitions
>>>> + * EXYNOS - Memory map definitions
>>>> *
>>>> * This program is free software; you can redistribute it and/or modify
>>>> * it under the terms of the GNU General Public License version 2 as
>>>> @@ -14,12 +14,6 @@
>>>>
>>>> #include <plat/map-base.h>
>>>>
>>>> -/*
>>>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>>>> - * So need to define it, and here is to avoid redefinition warning.
>>>> - */
>>>> -#define S3C_UART_OFFSET (0x10000)
>>>> -
>>>> #include <plat/map-s5p.h>
>>>
>>> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>>>
>>
>> Actually it's just defined there but not getting used anywhere. In fact
>> we can remove it from there also.
>
> The point is that your patch introduce changes. Before S3C_UART_OFFSET
> was defined to 0x10000, after (because of #ifndef) it will be 0x400.
> Description of "unused" is not accurate. Unused stuff does not impact
> anything.
>
>> So I submitted another patch to
>> cleanup such redundant register definitions from plat/map-s5p.h.
>
> That would be good.
>
>> Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
>> from map-s3c.h. I have compiled all related platform defconfigs and
>> found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
>> map.h.
>
> The compilation is not a test. You compiled something like this:
> -#define S3C_UART_OFFSET (0x10000)
> #ifndef S3C_UART_OFFSET
> #define S3C_UART_OFFSET (0x400)
> #endif
>
> ... and found no errors. Of course there are no compilation errors! But
> the value has changed!
>
OK. I see, there are quite a few places these definitions are scattered
even though not used (not used in the sense what ever values these
macros are taking in code it's not used). Such as
mach-exynos/include/map.h, plat-samsung/include/plat/{map-s3c.h,
map-s5p.h} and mach-s3c24xx/include/mach/map.h. So I will remove all
these in one go.
Is this fine?
Also I have one doubt in case I am touching files across various mach
files and preparing a single patch what should be commit message
heading? Is commit starting with "ARM: samsung" fine? or anything more
appropriate you would suggest.
Thanks,
Pankaj Dubey
> Best regards,
> Krzysztof
>
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
2016-02-24 8:23 ` pankaj.dubey
@ 2016-02-24 9:14 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-24 9:14 UTC (permalink / raw)
To: pankaj.dubey, linux-samsung-soc, linux-arm-kernel
Cc: kgene.kim, b.zolnierkie, daniel.lezcano, thomas.ab, linux
On 24.02.2016 17:23, pankaj.dubey wrote:
>
>
> On Wednesday 24 February 2016 05:17 AM, Krzysztof Kozlowski wrote:
>> On 23.02.2016 18:11, pankaj.dubey wrote:
>>> Hi Krzysztof,
>>>
>>> On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
>>>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>>>> This patch cleans up map.h by removing unused register offset
>>>>> and changing EXYNOS4 to EXYNOS in header comment section.
>>>>>
>>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>>> ---
>>>>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>>>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>>>>> index 351e839..c48ba4f 100644
>>>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>>>> @@ -2,7 +2,7 @@
>>>>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>>>>> * http://www.samsung.com/
>>>>> *
>>>>> - * EXYNOS4 - Memory map definitions
>>>>> + * EXYNOS - Memory map definitions
>>>>> *
>>>>> * This program is free software; you can redistribute it and/or modify
>>>>> * it under the terms of the GNU General Public License version 2 as
>>>>> @@ -14,12 +14,6 @@
>>>>>
>>>>> #include <plat/map-base.h>
>>>>>
>>>>> -/*
>>>>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>>>>> - * So need to define it, and here is to avoid redefinition warning.
>>>>> - */
>>>>> -#define S3C_UART_OFFSET (0x10000)
>>>>> -
>>>>> #include <plat/map-s5p.h>
>>>>
>>>> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>>>>
>>>
>>> Actually it's just defined there but not getting used anywhere. In fact
>>> we can remove it from there also.
>>
>> The point is that your patch introduce changes. Before S3C_UART_OFFSET
>> was defined to 0x10000, after (because of #ifndef) it will be 0x400.
>> Description of "unused" is not accurate. Unused stuff does not impact
>> anything.
>>
>>> So I submitted another patch to
>>> cleanup such redundant register definitions from plat/map-s5p.h.
>>
>> That would be good.
>>
>>> Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
>>> from map-s3c.h. I have compiled all related platform defconfigs and
>>> found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
>>> map.h.
>>
>> The compilation is not a test. You compiled something like this:
>> -#define S3C_UART_OFFSET (0x10000)
>> #ifndef S3C_UART_OFFSET
>> #define S3C_UART_OFFSET (0x400)
>> #endif
>>
>> ... and found no errors. Of course there are no compilation errors! But
>> the value has changed!
>>
>
> OK. I see, there are quite a few places these definitions are scattered
> even though not used (not used in the sense what ever values these
> macros are taking in code it's not used). Such as
> mach-exynos/include/map.h, plat-samsung/include/plat/{map-s3c.h,
> map-s5p.h} and mach-s3c24xx/include/mach/map.h. So I will remove all
> these in one go.
> Is this fine?
Except the used ones (s3c64xx) - yes, all at once. Some of them depend
on previous ones (ifndef) so "unused" when patching this one-by-one is
not accurate.
> Also I have one doubt in case I am touching files across various mach
> files and preparing a single patch what should be commit message
> heading? Is commit starting with "ARM: samsung" fine? or anything more
> appropriate you would suggest.
It is still the same Samsung tree and either way it would end in the
same branch. ARM: SAMSUNG: is fine (I saw it in the log).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition
@ 2016-02-24 9:14 ` Krzysztof Kozlowski
0 siblings, 0 replies; 28+ messages in thread
From: Krzysztof Kozlowski @ 2016-02-24 9:14 UTC (permalink / raw)
To: linux-arm-kernel
On 24.02.2016 17:23, pankaj.dubey wrote:
>
>
> On Wednesday 24 February 2016 05:17 AM, Krzysztof Kozlowski wrote:
>> On 23.02.2016 18:11, pankaj.dubey wrote:
>>> Hi Krzysztof,
>>>
>>> On Tuesday 23 February 2016 05:33 AM, Krzysztof Kozlowski wrote:
>>>> On 22.02.2016 19:03, Pankaj Dubey wrote:
>>>>> This patch cleans up map.h by removing unused register offset
>>>>> and changing EXYNOS4 to EXYNOS in header comment section.
>>>>>
>>>>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>>>>> ---
>>>>> arch/arm/mach-exynos/include/mach/map.h | 9 +--------
>>>>> 1 file changed, 1 insertion(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-exynos/include/mach/map.h b/arch/arm/mach-exynos/include/mach/map.h
>>>>> index 351e839..c48ba4f 100644
>>>>> --- a/arch/arm/mach-exynos/include/mach/map.h
>>>>> +++ b/arch/arm/mach-exynos/include/mach/map.h
>>>>> @@ -2,7 +2,7 @@
>>>>> * Copyright (c) 2010-2011 Samsung Electronics Co., Ltd.
>>>>> * http://www.samsung.com/
>>>>> *
>>>>> - * EXYNOS4 - Memory map definitions
>>>>> + * EXYNOS - Memory map definitions
>>>>> *
>>>>> * This program is free software; you can redistribute it and/or modify
>>>>> * it under the terms of the GNU General Public License version 2 as
>>>>> @@ -14,12 +14,6 @@
>>>>>
>>>>> #include <plat/map-base.h>
>>>>>
>>>>> -/*
>>>>> - * EXYNOS4 UART offset is 0x10000 but the older S5P SoCs are 0x400.
>>>>> - * So need to define it, and here is to avoid redefinition warning.
>>>>> - */
>>>>> -#define S3C_UART_OFFSET (0x10000)
>>>>> -
>>>>> #include <plat/map-s5p.h>
>>>>
>>>> This does not look good. The S3C_UART_OFFSET is used in plat/map-s5p.h.
>>>>
>>>
>>> Actually it's just defined there but not getting used anywhere. In fact
>>> we can remove it from there also.
>>
>> The point is that your patch introduce changes. Before S3C_UART_OFFSET
>> was defined to 0x10000, after (because of #ifndef) it will be 0x400.
>> Description of "unused" is not accurate. Unused stuff does not impact
>> anything.
>>
>>> So I submitted another patch to
>>> cleanup such redundant register definitions from plat/map-s5p.h.
>>
>> That would be good.
>>
>>> Only platform using S3C_UART_OFFSET is s3c64xx and its using definition
>>> from map-s3c.h. I have compiled all related platform defconfigs and
>>> found no issues because of removal of S3C_UART_OFFSET from map-s5p.c and
>>> map.h.
>>
>> The compilation is not a test. You compiled something like this:
>> -#define S3C_UART_OFFSET (0x10000)
>> #ifndef S3C_UART_OFFSET
>> #define S3C_UART_OFFSET (0x400)
>> #endif
>>
>> ... and found no errors. Of course there are no compilation errors! But
>> the value has changed!
>>
>
> OK. I see, there are quite a few places these definitions are scattered
> even though not used (not used in the sense what ever values these
> macros are taking in code it's not used). Such as
> mach-exynos/include/map.h, plat-samsung/include/plat/{map-s3c.h,
> map-s5p.h} and mach-s3c24xx/include/mach/map.h. So I will remove all
> these in one go.
> Is this fine?
Except the used ones (s3c64xx) - yes, all at once. Some of them depend
on previous ones (ifndef) so "unused" when patching this one-by-one is
not accurate.
> Also I have one doubt in case I am touching files across various mach
> files and preparing a single patch what should be commit message
> heading? Is commit starting with "ARM: samsung" fine? or anything more
> appropriate you would suggest.
It is still the same Samsung tree and either way it would end in the
same branch. ARM: SAMSUNG: is fine (I saw it in the log).
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2016-02-24 9:15 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-22 10:03 [PATCH 0/3] ARM: EXYNOS: Trivial cleanup in mach-exynos Pankaj Dubey
2016-02-22 10:03 ` Pankaj Dubey
2016-02-22 10:03 ` [PATCH 1/3] ARM: EXYNOS: correct header comment in Kconfig file Pankaj Dubey
2016-02-22 10:03 ` Pankaj Dubey
2016-02-23 0:01 ` Krzysztof Kozlowski
2016-02-23 0:01 ` Krzysztof Kozlowski
2016-02-22 10:03 ` [PATCH 2/3] ARM: EXYNOS: remove unused register offset definition Pankaj Dubey
2016-02-22 10:03 ` Pankaj Dubey
2016-02-23 0:03 ` Krzysztof Kozlowski
2016-02-23 0:03 ` Krzysztof Kozlowski
2016-02-23 9:11 ` pankaj.dubey
2016-02-23 9:11 ` pankaj.dubey
2016-02-23 23:47 ` Krzysztof Kozlowski
2016-02-23 23:47 ` Krzysztof Kozlowski
2016-02-24 8:23 ` pankaj.dubey
2016-02-24 8:23 ` pankaj.dubey
2016-02-24 9:14 ` Krzysztof Kozlowski
2016-02-24 9:14 ` Krzysztof Kozlowski
2016-02-22 10:03 ` [PATCH 3/3] ARM: EXYNOS: cleanup header files inclusion Pankaj Dubey
2016-02-22 10:03 ` Pankaj Dubey
2016-02-23 0:09 ` Krzysztof Kozlowski
2016-02-23 0:09 ` Krzysztof Kozlowski
2016-02-23 9:17 ` pankaj.dubey
2016-02-23 9:17 ` pankaj.dubey
2016-02-24 1:02 ` Krzysztof Kozlowski
2016-02-24 1:02 ` Krzysztof Kozlowski
2016-02-24 4:01 ` pankaj.dubey
2016-02-24 4:01 ` pankaj.dubey
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.