Miscellaneous s3c64xx fixes.

From 9fd700af5b0c2289a09736a877f6047d1dcd3268 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Tue, 19 Jul 2011 22:21:41 +0200
Subject: [PATCH 1/5] ARM: s3c64xx: PM: Mask and acknowledge all UART
interrupts during core resume.

On some boards (in my case Tiny6410 from FriendlyARM), after waking up from sleep mode, UART controllers are left in an unclean state with interrupt status bits set. After reenabling interrupts the system starts to get hammered by infinite UART interrupts, which cannot be acknowledged, because of disabled UART bus clock. You can imagine the outcome.

This patch deals with the issue by reenabling the bus clock in PCLK mask temporarily, acknowledging and masking all the UART interrupts and then restoring the original PCLK mask value, before interrupts get enabled.

Alternatively, the issue could be avoided by moving all the UART interrupt handling to the S3C UART driver and disabling the IRQ on port suspend. Could anyone explain me what is the benefit of having the UART IRQ managed by the generic IRQ infrastructure, while it is used only by a single driver?

---
 arch/arm/mach-s3c64xx/pm.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

arch/arm/mach-s3c64xx/pm.c +19 -0

index bc1c470..799a212 100644
@@ -16,12 +16,14 @@
 #include <linux/suspend.h>
 #include <linux/serial_core.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include <mach/map.h>
 #include <mach/irqs.h>
 
 #include <plat/pm.h>
 #include <plat/wakeup-mask.h>
+#include <plat/regs-serial.h>
 
 #include <mach/regs-sys.h>
 #include <mach/regs-gpio.h>

@@ -93,12 +95,36 @@ void s3c_pm_configure_extint(void)
 
 void s3c_pm_restore_core(void)
 {
+	u32 pclkgate, tmp;
+	int i;
+
 	__raw_writel(0, S3C64XX_EINT_MASK);
 
 	s3c_pm_debug_smdkled(1 << 2, 0);
 
 	s3c_pm_do_restore_core(core_save, ARRAY_SIZE(core_save));
 	s3c_pm_do_restore(misc_save, ARRAY_SIZE(misc_save));
+
+	tmp = pclkgate = __raw_readl(S3C_PCLK_GATE);
+
+	/* re-start uart clocks */
+	tmp |= S3C_CLKCON_PCLK_UART0;
+	tmp |= S3C_CLKCON_PCLK_UART1;
+	tmp |= S3C_CLKCON_PCLK_UART2;
+	tmp |= S3C_CLKCON_PCLK_UART3;
+
+	__raw_writel(tmp, S3C_PCLK_GATE);
+
+	udelay(10);
+
+	for (i = 0; i < 4; ++i) {

Quoted 7 lines · Tomasz Figa

From 9fd700af5b0c2289a09736a877f6047d1dcd3268 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Tue, 19 Jul 2011 22:21:41 +0200
Subject: [PATCH 1/5] ARM: s3c64xx: PM: Mask and acknowledge all UART
interrupts during core resume.

On some boards (in my case Tiny6410 from FriendlyARM), after waking up

Quoted 3 lines

sleep mode, UART controllers are left in an unclean state with interrupt status bits set. After reenabling interrupts the system starts to get hammered by infinite UART interrupts, which cannot be acknowledged,

Quoted 7 lines

of disabled UART bus clock. You can imagine the outcome.

This patch deals with the issue by reenabling the bus clock in PCLK mask temporarily, acknowledging and masking all the UART interrupts and then restoring the original PCLK mask value, before interrupts get enabled.

Alternatively, the issue could be avoided by moving all the UART

Quoted 2 lines

handling to the S3C UART driver and disabling the IRQ on port suspend. Could anyone explain me what is the benefit of having the UART IRQ

Quoted 1 line

by the generic IRQ infrastructure, while it is used only by a single

Quoted 52 lines

---
 arch/arm/mach-s3c64xx/pm.c |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

arch/arm/mach-s3c64xx/pm.c +19 -0

index bc1c470..799a212 100644
@@ -16,12 +16,14 @@
 #include <linux/suspend.h>
 #include <linux/serial_core.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include <mach/map.h>
 #include <mach/irqs.h>
 
 #include <plat/pm.h>
 #include <plat/wakeup-mask.h>
+#include <plat/regs-serial.h>
 
 #include <mach/regs-sys.h>
 #include <mach/regs-gpio.h>

@@ -93,12 +95,36 @@ void s3c_pm_configure_extint(void)
 
 void s3c_pm_restore_core(void)
 {
+	u32 pclkgate, tmp;
+	int i;
+
 	__raw_writel(0, S3C64XX_EINT_MASK);
 
 	s3c_pm_debug_smdkled(1 << 2, 0);
 
 	s3c_pm_do_restore_core(core_save, ARRAY_SIZE(core_save));
 	s3c_pm_do_restore(misc_save, ARRAY_SIZE(misc_save));
+
+	tmp = pclkgate = __raw_readl(S3C_PCLK_GATE);
+
+	/* re-start uart clocks */
+	tmp |= S3C_CLKCON_PCLK_UART0;
+	tmp |= S3C_CLKCON_PCLK_UART1;
+	tmp |= S3C_CLKCON_PCLK_UART2;
+	tmp |= S3C_CLKCON_PCLK_UART3;
+
+	__raw_writel(tmp, S3C_PCLK_GATE);
+
+	udelay(10);
+
+	for (i = 0; i < 4; ++i) {

arch/arm/mach-s3c64xx/pm.c void s3c_pm_configure_extint(void) · line 130

Quoted 10 lines

arch/arm/mach-s3c64xx/pm.c +7 -0

+		__raw_writel(15, S3C_VA_UARTx(i) + S3C64XX_UINTM);
+		__raw_writel(15, S3C_VA_UARTx(i) + S3C64XX_UINTP);
+	}
+
+	udelay(10);
+
+	__raw_writel(pclkgate, S3C_PCLK_GATE);
 }
 
 void s3c_pm_save_core(void)

This should be fixed by

[PATCH 0/2] Update uart irq handling for s3c64xx and later SoC's by Thomas Abraham,

so I guess it is safe to drop my patch.

From c985e14a4fc5c7970d7f6c2fde6d214217214688 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Thu, 28 Jul 2011 14:34:51 +0200
Subject: [PATCH 5/5] ARM: s3c64xx: Add support for synchronous clock
operation.

Some boards based on S3C6410 use synchronous clocking, which means that HCLKx2
and other system clocks are generated from APLL instead of MPLL.

This patch adds support for such boards, by calculating hclk2 depending on the status of S3C_OTHERS_SYNCMUXSEL_SYNC bit in S3C_OTHERS registers.

arch/arm/mach-s3c64xx/include/mach/regs-clock.h +1 -1

+
 #endif /* _PLAT_REGS_CLOCK_H */
--

1.7.6

Quoted 7 lines · Tomasz Figa

From c985e14a4fc5c7970d7f6c2fde6d214217214688 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Thu, 28 Jul 2011 14:34:51 +0200
Subject: [PATCH 5/5] ARM: s3c64xx: Add support for synchronous clock
operation.

Quoted 7 lines

Some boards based on S3C6410 use synchronous clocking, which means that HCLKx2
and other system clocks are generated from APLL instead of MPLL.

This patch adds support for such boards, by calculating hclk2 depending on the status of S3C_OTHERS_SYNCMUXSEL_SYNC bit in S3C_OTHERS registers.

Where is 'Signed-off-by' ?

Review

Quoted 25 lines

---
 arch/arm/mach-s3c64xx/clock.c                   |    8 +++++++-
 arch/arm/mach-s3c64xx/include/mach/regs-clock.h |    4 ++++
 2 files changed, 11 insertions(+), 1 deletions(-)

arch/arm/mach-s3c64xx/clock.c +7 -1

index fdfc4d5..56421ab 100644
@@ -780,7 +780,13 @@ void __init_or_cpufreq s3c6400_setup_clocks(void)
 	printk(KERN_INFO "S3C64XX: PLL settings, A=%ld, M=%ld, E=%ld\n",
 	       apll, mpll, epll);
 
-	hclk2 = mpll / GET_DIV(clkdiv0, S3C6400_CLKDIV0_HCLK2);
+	if(__raw_readl(S3C_OTHERS) & S3C_OTHERS_SYNCMUXSEL_SYNC)
+		/* Synchronous mode */
+		hclk2 = apll / GET_DIV(clkdiv0, S3C6400_CLKDIV0_HCLK2);
+	else
+		/* Asynchronous mode */
+		hclk2 = mpll / GET_DIV(clkdiv0, S3C6400_CLKDIV0_HCLK2);
+
 	hclk = hclk2 / GET_DIV(clkdiv0, S3C6400_CLKDIV0_HCLK);
 	pclk = hclk2 / GET_DIV(clkdiv0, S3C6400_CLKDIV0_PCLK);
 

(quoted diff) +0 -0

b/arch/arm/mach-

arch/arm/mach-s3c64xx/include/mach/regs-clock.h line 38

Quoted 9 lines

s3c64xx/include/mach/regs-clock.h
index 05332b9..ac2202f 100644

arch/arm/mach-s3c64xx/include/mach/regs-clock.h +1 -0

@@ -35,6 +35,7 @@
 #define S3C_MEM0_GATE		S3C_CLKREG(0x3C)
 #define S3C6410_CLK_SRC2	S3C_CLKREG(0x10C)
 #define S3C_MEM_SYS_CFG		S3C_CLKREG(0x120)
+#define S3C_OTHERS		S3C_CLKREG(0x900)

In mach/regs-sys.h, the "S3C64XX_OTHERS" which can be used instead of above is defined.

arch/arm/mach-s3c64xx/include/mach/regs-clock.h line 164

Quoted 9 lines

arch/arm/mach-s3c64xx/include/mach/regs-clock.h +2 -0

 /* CLKDIV0 */
 #define S3C6400_CLKDIV0_PCLK_MASK	(0xf << 12)

@@ -159,4 +160,7 @@
 #define MEM_SYS_CFG_INDEP_CF		0x4000
 #define MEM_SYS_CFG_EBI_FIX_PRI_CFCON	0x30
 
+/* OTHERS */
+#define S3C_OTHERS_SYNCMUXSEL_SYNC	(1<<6)

1st, according to codingstyle, the blank should added around << like (1 <<
6) even though other old codes don't keep it in same file. 2nd, according to datasheet, please use 'SYNCMUXSEL' not 'SYNCMUXSEL_SYNC' 3rd, should be moved in mach/regs-sys.h

Quoted 4 lines

arch/arm/mach-s3c64xx/include/mach/regs-clock.h +1 -1

+
 #endif /* _PLAT_REGS_CLOCK_H */
--

1.7.6

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

From c985e14a4fc5c7970d7f6c2fde6d214217214688 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Fri, 29 Jul 2011 16:50:16 +0200
Subject: [PATCH 0/5] Miscellaneous s3c64xx fixes.

This series attempts to fix some issues I approached when working on the s3c64xx platform. See descriptions of individual patches for more details on each one.

All the patches are based on v3.0 tag of Linus' tree.

Tomasz Figa (5):
ARM: s3c64xx: PM: Mask and acknowledge all UART interrupts during
core resume.
ARM: s3c64xx: DMA: Use S3C64XX_SDMA_SEL register name instead of
numeric address.
ARM: s3c64xx: Save/restore S3C64XX_SDMA_SEL on suspend/resume.
ARM: s3c64xx: Save/restore S3C64XX_MODEM_MIFPCON on suspend/resume to
preserve LCD bypass state.
ARM: s3c64xx: Add support for synchronous clock operation.

 arch/arm/mach-s3c64xx/clock.c                   |    8 +++++-
 arch/arm/mach-s3c64xx/dma.c                     |    2 +-
 arch/arm/mach-s3c64xx/include/mach/regs-clock.h |    4 +++
 arch/arm/mach-s3c64xx/include/mach/regs-sys.h   |    2 +
 arch/arm/mach-s3c64xx/pm.c                      |   30 
+++++++++++++++++++++++
 5 files changed, 44 insertions(+), 2 deletions(-)

Anyone?

Quoted 11 lines

From c985e14a4fc5c7970d7f6c2fde6d214217214688 Mon Sep 17 00:00:00 2001
From: Tomasz Figa <[email protected]>
Date: Fri, 29 Jul 2011 16:50:16 +0200
Subject: [PATCH 0/5] Miscellaneous s3c64xx fixes.

This series attempts to fix some issues I approached when working on the s3c64xx platform. See descriptions of individual patches for more details on each one.

All the patches are based on v3.0 tag of Linus' tree.

Quoted 7 lines

Tomasz Figa (5):
ARM: s3c64xx: PM: Mask and acknowledge all UART interrupts during
core resume.
ARM: s3c64xx: DMA: Use S3C64XX_SDMA_SEL register name instead of
numeric address.
ARM: s3c64xx: Save/restore S3C64XX_SDMA_SEL on suspend/resume.
ARM: s3c64xx: Save/restore S3C64XX_MODEM_MIFPCON on

suspend/resume to

Quoted 10 lines

preserve LCD bypass state.
ARM: s3c64xx: Add support for synchronous clock operation.

 arch/arm/mach-s3c64xx/clock.c                   |    8 +++++-
 arch/arm/mach-s3c64xx/dma.c                     |    2 +-
 arch/arm/mach-s3c64xx/include/mach/regs-clock.h |    4 +++
 arch/arm/mach-s3c64xx/include/mach/regs-sys.h   |    2 +
 arch/arm/mach-s3c64xx/pm.c                      |   30
+++++++++++++++++++++++
 5 files changed, 44 insertions(+), 2 deletions(-)

Quoted 3 lines · Tomasz Figa

Anyone?

Hi Tomasz Figa,

Will be on this in the next week...

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Quoted 10 lines · Kukjin Kim

Quoted 3 lines · Tomasz Figa

Anyone?

Hi Tomasz Figa,

Will be on this in the next week...

Thanks.

Quoted 6 lines

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Thanks a lot. I have plenty of other S3C64xx patches to be submitted, but

are more feature additions not fixes, so I am waiting for these fixes to

reviewed first.

Quoted 12 lines · Tomasz Figa

Quoted 10 lines · Kukjin Kim

Quoted 3 lines · Tomasz Figa

Anyone?

Hi Tomasz Figa,

Will be on this in the next week...

Thanks.

Quoted 8 lines

Quoted 6 lines

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.

Thanks a lot. I have plenty of other S3C64xx patches to be submitted, but

they

Quoted 1 line

are more feature additions not fixes, so I am waiting for these fixes to

be

Quoted 2 lines

reviewed first.

Hi,

Firstly, you have to re-submit whole series with your 'sign'. It is very important.

And 2nd, 3rd and 4th patches look ok to me except missing your sign even though I think, secure DMA mode should be setting in boot loader with trust zone...

Please refer to my review on 5th patch.

Thanks.

Best regards,
Kgene.
--
Kukjin Kim <[email protected]>, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd.