* [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART
@ 2013-07-21 19:42 Sylwester Nawrocki
  2013-07-22  4:39 ` Kukjin Kim
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Sylwester Nawrocki @ 2013-07-21 19:42 UTC (permalink / raw)
  To: kgene.kim
  Cc: linux-samsung-soc, linux-arm-kernel, linux-serial, oselas,
	Sylwester Nawrocki, Chander Kashyap
This patch restores serial port operation which has been broken since
commit 60e93575476f90a72146b51283f514da655410a7
serial: samsung: enable clock before clearing pending interrupts during init
That commit only uncovered the real issue which was missing clkdev
entries for the "uart" clocks on S3C2440. It went unnoticed so far
because return value of clk API calls were not being checked at all
in the samsung serial port driver.
This patch should be backported to at least 3.10 stable kernel, since
the serial port has not been working on s3c2440 since 3.10-rc5.
Cc: Chander Kashyap <chander.kashyap@linaro.org>
Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
[on S3C2440 SoC based Mini2440 board]
Tested-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
---
There seems to be something wrong with the process, as things are
getting broken late in the -rc cycle.  It seems author of the above
mentioned commit didn't test all 6 SoC series that the samsung
serial driver handles.  While I can understand they might not have
all required hardware it's a bit irritating that the patch that
in practice caused regression appeared only on linux-serial mailing
list. And it's not evident it was even tried to test it on all
potentially affected platforms. It not clear on what platforms
the patch has been tested.
My humble suggestion is, _please_ do send patches that affect multiple
Samsung SoCs to linux-samsung@vger.kernel.org.  So there is better
review and test exposure.  And we can reduce mess like this happening
in the future.
Thanks!
Sylwester
---
 arch/arm/mach-s3c24xx/clock-s3c2410.c      |  161 +++++++++++++++++-----------
 arch/arm/mach-s3c24xx/clock-s3c2440.c      |    3 +
 arch/arm/plat-samsung/include/plat/clock.h |    5 +
 3 files changed, 106 insertions(+), 63 deletions(-)
diff --git a/arch/arm/mach-s3c24xx/clock-s3c2410.c b/arch/arm/mach-s3c24xx/clock-s3c2410.c
index afa0267..d39d3c7 100644
--- a/arch/arm/mach-s3c24xx/clock-s3c2410.c
+++ b/arch/arm/mach-s3c24xx/clock-s3c2410.c
@@ -119,66 +119,101 @@ static struct clk init_clocks_off[] = {
 	}
 };
 
-static struct clk init_clocks[] = {
-	{
-		.name		= "lcd",
-		.parent		= &clk_h,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_LCDC,
-	}, {
-		.name		= "gpio",
-		.parent		= &clk_p,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_GPIO,
-	}, {
-		.name		= "usb-host",
-		.parent		= &clk_h,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_USBH,
-	}, {
-		.name		= "usb-device",
-		.parent		= &clk_h,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_USBD,
-	}, {
-		.name		= "timers",
-		.parent		= &clk_p,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_PWMT,
-	}, {
-		.name		= "uart",
-		.devname	= "s3c2410-uart.0",
-		.parent		= &clk_p,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_UART0,
-	}, {
-		.name		= "uart",
-		.devname	= "s3c2410-uart.1",
-		.parent		= &clk_p,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_UART1,
-	}, {
-		.name		= "uart",
-		.devname	= "s3c2410-uart.2",
-		.parent		= &clk_p,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_UART2,
-	}, {
-		.name		= "rtc",
-		.parent		= &clk_p,
-		.enable		= s3c2410_clkcon_enable,
-		.ctrlbit	= S3C2410_CLKCON_RTC,
-	}, {
-		.name		= "watchdog",
-		.parent		= &clk_p,
-		.ctrlbit	= 0,
-	}, {
-		.name		= "usb-bus-host",
-		.parent		= &clk_usb_bus,
-	}, {
-		.name		= "usb-bus-gadget",
-		.parent		= &clk_usb_bus,
-	},
+static struct clk clk_lcd = {
+	.name		= "lcd",
+	.parent		= &clk_h,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_LCDC,
+};
+
+static struct clk clk_gpio = {
+	.name		= "gpio",
+	.parent		= &clk_p,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_GPIO,
+};
+
+static struct clk clk_usb_host = {
+	.name		= "usb-host",
+	.parent		= &clk_h,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_USBH,
+};
+
+static struct clk clk_usb_device = {
+	.name		= "usb-device",
+	.parent		= &clk_h,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_USBD,
+};
+
+static struct clk clk_timers = {
+	.name		= "timers",
+	.parent		= &clk_p,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_PWMT,
+};
+
+struct clk s3c24xx_clk_uart0 = {
+	.name		= "uart",
+	.devname	= "s3c2410-uart.0",
+	.parent		= &clk_p,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_UART0,
+};
+
+struct clk s3c24xx_clk_uart1 = {
+	.name		= "uart",
+	.devname	= "s3c2410-uart.1",
+	.parent		= &clk_p,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_UART1,
+};
+
+struct clk s3c24xx_clk_uart2 = {
+	.name		= "uart",
+	.devname	= "s3c2410-uart.2",
+	.parent		= &clk_p,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_UART2,
+};
+
+static struct clk clk_rtc = {
+	.name		= "rtc",
+	.parent		= &clk_p,
+	.enable		= s3c2410_clkcon_enable,
+	.ctrlbit	= S3C2410_CLKCON_RTC,
+};
+
+static struct clk clk_watchdog = {
+	.name		= "watchdog",
+	.parent		= &clk_p,
+	.ctrlbit	= 0,
+};
+
+static struct clk clk_usb_bus_host = {
+	.name		= "usb-bus-host",
+	.parent		= &clk_usb_bus,
+};
+
+static struct clk clk_usb_bus_gadget = {
+	.name		= "usb-bus-gadget",
+	.parent		= &clk_usb_bus,
+};
+
+static struct clk *init_clocks[] = {
+	&clk_lcd,
+	&clk_gpio,
+	&clk_usb_host,
+	&clk_usb_device,
+	&clk_timers,
+	&s3c24xx_clk_uart0,
+	&s3c24xx_clk_uart1,
+	&s3c24xx_clk_uart2,
+	&clk_rtc,
+	&clk_watchdog,
+	&clk_usb_bus_host,
+	&clk_usb_bus_gadget,
 };
 
 /* s3c2410_baseclk_add()
@@ -195,7 +230,6 @@ int __init s3c2410_baseclk_add(void)
 {
 	unsigned long clkslow = __raw_readl(S3C2410_CLKSLOW);
 	unsigned long clkcon  = __raw_readl(S3C2410_CLKCON);
-	struct clk *clkp;
 	struct clk *xtal;
 	int ret;
 	int ptr;
@@ -207,8 +241,9 @@ int __init s3c2410_baseclk_add(void)
 
 	/* register clocks from clock array */
 
-	clkp = init_clocks;
-	for (ptr = 0; ptr < ARRAY_SIZE(init_clocks); ptr++, clkp++) {
+	for (ptr = 0; ptr < ARRAY_SIZE(init_clocks); ptr++) {
+		struct clk *clkp = init_clocks[ptr];
+
 		/* ensure that we note the clock state */
 
 		clkp->usage = clkcon & clkp->ctrlbit ? 1 : 0;
diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c b/arch/arm/mach-s3c24xx/clock-s3c2440.c
index 1069b56..aaf006d 100644
--- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
+++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
@@ -166,6 +166,9 @@ static struct clk_lookup s3c2440_clk_lookup[] = {
 	CLKDEV_INIT(NULL, "clk_uart_baud1", &s3c24xx_uclk),
 	CLKDEV_INIT(NULL, "clk_uart_baud2", &clk_p),
 	CLKDEV_INIT(NULL, "clk_uart_baud3", &s3c2440_clk_fclk_n),
+	CLKDEV_INIT("s3c2440-uart.0", "uart", &s3c24xx_clk_uart0),
+	CLKDEV_INIT("s3c2440-uart.1", "uart", &s3c24xx_clk_uart1),
+	CLKDEV_INIT("s3c2440-uart.2", "uart", &s3c24xx_clk_uart2),
 	CLKDEV_INIT("s3c2440-camif", "camera", &s3c2440_clk_cam_upll),
 };
 
diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat-samsung/include/plat/clock.h
index 254c3dd..63239f4 100644
--- a/arch/arm/plat-samsung/include/plat/clock.h
+++ b/arch/arm/plat-samsung/include/plat/clock.h
@@ -83,6 +83,11 @@ extern struct clk clk_ext;
 extern struct clksrc_clk clk_epllref;
 extern struct clksrc_clk clk_esysclk;
 
+/* S3C24XX UART clocks */
+extern struct clk s3c24xx_clk_uart0;
+extern struct clk s3c24xx_clk_uart1;
+extern struct clk s3c24xx_clk_uart2;
+
 /* S3C64XX specific clocks */
 extern struct clk clk_h2;
 extern struct clk clk_27m;
-- 
1.7.4.1
^ permalink raw reply related	[flat|nested] 4+ messages in thread
* RE: [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART
  2013-07-21 19:42 [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART Sylwester Nawrocki
@ 2013-07-22  4:39 ` Kukjin Kim
  2013-07-22 17:14 ` Tomasz Figa
       [not found] ` <1374435765-6747-1-git-send-email-sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2 siblings, 0 replies; 4+ messages in thread
From: Kukjin Kim @ 2013-07-22  4:39 UTC (permalink / raw)
  To: 'Sylwester Nawrocki', 'Heiko Stübner'
  Cc: linux-samsung-soc, linux-arm-kernel, linux-serial, oselas,
	'Chander Kashyap'
Sylwester Nawrocki wrote:
> 
> This patch restores serial port operation which has been broken since
> commit 60e93575476f90a72146b51283f514da655410a7
> serial: samsung: enable clock before clearing pending interrupts during
> init
> 
> That commit only uncovered the real issue which was missing clkdev
> entries for the "uart" clocks on S3C2440. It went unnoticed so far
> because return value of clk API calls were not being checked at all
> in the samsung serial port driver.
> 
> This patch should be backported to at least 3.10 stable kernel, since
> the serial port has not been working on s3c2440 since 3.10-rc5.
> 
OK, will apply into -fixes and let me add 'stable' tree in Cc, thanks for
your suggestion :-)
> Cc: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> [on S3C2440 SoC based Mini2440 board]
> Tested-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> ---
> 
> There seems to be something wrong with the process, as things are
> getting broken late in the -rc cycle.  It seems author of the above
> mentioned commit didn't test all 6 SoC series that the samsung
> serial driver handles.  While I can understand they might not have
> all required hardware it's a bit irritating that the patch that
> in practice caused regression appeared only on linux-serial mailing
> list. And it's not evident it was even tried to test it on all
> potentially affected platforms. It not clear on what platforms
> the patch has been tested.
> 
> My humble suggestion is, _please_ do send patches that affect multiple
> Samsung SoCs to linux-samsung@vger.kernel.org.  So there is better
> review and test exposure.  And we can reduce mess like this happening
> in the future.
> 
Absolutely!
Thanks,
Kukjin
> Thanks!
> Sylwester
> ---
>  arch/arm/mach-s3c24xx/clock-s3c2410.c      |  161
+++++++++++++++++--------
> ---
>  arch/arm/mach-s3c24xx/clock-s3c2440.c      |    3 +
>  arch/arm/plat-samsung/include/plat/clock.h |    5 +
>  3 files changed, 106 insertions(+), 63 deletions(-)
> 
> diff --git a/arch/arm/mach-s3c24xx/clock-s3c2410.c b/arch/arm/mach-
> s3c24xx/clock-s3c2410.c
> index afa0267..d39d3c7 100644
> --- a/arch/arm/mach-s3c24xx/clock-s3c2410.c
> +++ b/arch/arm/mach-s3c24xx/clock-s3c2410.c
> @@ -119,66 +119,101 @@ static struct clk init_clocks_off[] = {
>  	}
>  };
> 
> -static struct clk init_clocks[] = {
> -	{
> -		.name		= "lcd",
> -		.parent		= &clk_h,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_LCDC,
> -	}, {
> -		.name		= "gpio",
> -		.parent		= &clk_p,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_GPIO,
> -	}, {
> -		.name		= "usb-host",
> -		.parent		= &clk_h,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_USBH,
> -	}, {
> -		.name		= "usb-device",
> -		.parent		= &clk_h,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_USBD,
> -	}, {
> -		.name		= "timers",
> -		.parent		= &clk_p,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_PWMT,
> -	}, {
> -		.name		= "uart",
> -		.devname	= "s3c2410-uart.0",
> -		.parent		= &clk_p,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_UART0,
> -	}, {
> -		.name		= "uart",
> -		.devname	= "s3c2410-uart.1",
> -		.parent		= &clk_p,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_UART1,
> -	}, {
> -		.name		= "uart",
> -		.devname	= "s3c2410-uart.2",
> -		.parent		= &clk_p,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_UART2,
> -	}, {
> -		.name		= "rtc",
> -		.parent		= &clk_p,
> -		.enable		= s3c2410_clkcon_enable,
> -		.ctrlbit	= S3C2410_CLKCON_RTC,
> -	}, {
> -		.name		= "watchdog",
> -		.parent		= &clk_p,
> -		.ctrlbit	= 0,
> -	}, {
> -		.name		= "usb-bus-host",
> -		.parent		= &clk_usb_bus,
> -	}, {
> -		.name		= "usb-bus-gadget",
> -		.parent		= &clk_usb_bus,
> -	},
> +static struct clk clk_lcd = {
> +	.name		= "lcd",
> +	.parent		= &clk_h,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_LCDC,
> +};
> +
> +static struct clk clk_gpio = {
> +	.name		= "gpio",
> +	.parent		= &clk_p,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_GPIO,
> +};
> +
> +static struct clk clk_usb_host = {
> +	.name		= "usb-host",
> +	.parent		= &clk_h,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_USBH,
> +};
> +
> +static struct clk clk_usb_device = {
> +	.name		= "usb-device",
> +	.parent		= &clk_h,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_USBD,
> +};
> +
> +static struct clk clk_timers = {
> +	.name		= "timers",
> +	.parent		= &clk_p,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_PWMT,
> +};
> +
> +struct clk s3c24xx_clk_uart0 = {
> +	.name		= "uart",
> +	.devname	= "s3c2410-uart.0",
> +	.parent		= &clk_p,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_UART0,
> +};
> +
> +struct clk s3c24xx_clk_uart1 = {
> +	.name		= "uart",
> +	.devname	= "s3c2410-uart.1",
> +	.parent		= &clk_p,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_UART1,
> +};
> +
> +struct clk s3c24xx_clk_uart2 = {
> +	.name		= "uart",
> +	.devname	= "s3c2410-uart.2",
> +	.parent		= &clk_p,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_UART2,
> +};
> +
> +static struct clk clk_rtc = {
> +	.name		= "rtc",
> +	.parent		= &clk_p,
> +	.enable		= s3c2410_clkcon_enable,
> +	.ctrlbit	= S3C2410_CLKCON_RTC,
> +};
> +
> +static struct clk clk_watchdog = {
> +	.name		= "watchdog",
> +	.parent		= &clk_p,
> +	.ctrlbit	= 0,
> +};
> +
> +static struct clk clk_usb_bus_host = {
> +	.name		= "usb-bus-host",
> +	.parent		= &clk_usb_bus,
> +};
> +
> +static struct clk clk_usb_bus_gadget = {
> +	.name		= "usb-bus-gadget",
> +	.parent		= &clk_usb_bus,
> +};
> +
> +static struct clk *init_clocks[] = {
> +	&clk_lcd,
> +	&clk_gpio,
> +	&clk_usb_host,
> +	&clk_usb_device,
> +	&clk_timers,
> +	&s3c24xx_clk_uart0,
> +	&s3c24xx_clk_uart1,
> +	&s3c24xx_clk_uart2,
> +	&clk_rtc,
> +	&clk_watchdog,
> +	&clk_usb_bus_host,
> +	&clk_usb_bus_gadget,
>  };
> 
>  /* s3c2410_baseclk_add()
> @@ -195,7 +230,6 @@ int __init s3c2410_baseclk_add(void)
>  {
>  	unsigned long clkslow = __raw_readl(S3C2410_CLKSLOW);
>  	unsigned long clkcon  = __raw_readl(S3C2410_CLKCON);
> -	struct clk *clkp;
>  	struct clk *xtal;
>  	int ret;
>  	int ptr;
> @@ -207,8 +241,9 @@ int __init s3c2410_baseclk_add(void)
> 
>  	/* register clocks from clock array */
> 
> -	clkp = init_clocks;
> -	for (ptr = 0; ptr < ARRAY_SIZE(init_clocks); ptr++, clkp++) {
> +	for (ptr = 0; ptr < ARRAY_SIZE(init_clocks); ptr++) {
> +		struct clk *clkp = init_clocks[ptr];
> +
>  		/* ensure that we note the clock state */
> 
>  		clkp->usage = clkcon & clkp->ctrlbit ? 1 : 0;
> diff --git a/arch/arm/mach-s3c24xx/clock-s3c2440.c b/arch/arm/mach-
> s3c24xx/clock-s3c2440.c
> index 1069b56..aaf006d 100644
> --- a/arch/arm/mach-s3c24xx/clock-s3c2440.c
> +++ b/arch/arm/mach-s3c24xx/clock-s3c2440.c
> @@ -166,6 +166,9 @@ static struct clk_lookup s3c2440_clk_lookup[] = {
>  	CLKDEV_INIT(NULL, "clk_uart_baud1", &s3c24xx_uclk),
>  	CLKDEV_INIT(NULL, "clk_uart_baud2", &clk_p),
>  	CLKDEV_INIT(NULL, "clk_uart_baud3", &s3c2440_clk_fclk_n),
> +	CLKDEV_INIT("s3c2440-uart.0", "uart", &s3c24xx_clk_uart0),
> +	CLKDEV_INIT("s3c2440-uart.1", "uart", &s3c24xx_clk_uart1),
> +	CLKDEV_INIT("s3c2440-uart.2", "uart", &s3c24xx_clk_uart2),
>  	CLKDEV_INIT("s3c2440-camif", "camera", &s3c2440_clk_cam_upll),
>  };
> 
> diff --git a/arch/arm/plat-samsung/include/plat/clock.h b/arch/arm/plat-
> samsung/include/plat/clock.h
> index 254c3dd..63239f4 100644
> --- a/arch/arm/plat-samsung/include/plat/clock.h
> +++ b/arch/arm/plat-samsung/include/plat/clock.h
> @@ -83,6 +83,11 @@ extern struct clk clk_ext;
>  extern struct clksrc_clk clk_epllref;
>  extern struct clksrc_clk clk_esysclk;
> 
> +/* S3C24XX UART clocks */
> +extern struct clk s3c24xx_clk_uart0;
> +extern struct clk s3c24xx_clk_uart1;
> +extern struct clk s3c24xx_clk_uart2;
> +
>  /* S3C64XX specific clocks */
>  extern struct clk clk_h2;
>  extern struct clk clk_27m;
> --
> 1.7.4.1
^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART
  2013-07-21 19:42 [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART Sylwester Nawrocki
  2013-07-22  4:39 ` Kukjin Kim
@ 2013-07-22 17:14 ` Tomasz Figa
       [not found] ` <1374435765-6747-1-git-send-email-sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
  2 siblings, 0 replies; 4+ messages in thread
From: Tomasz Figa @ 2013-07-22 17:14 UTC (permalink / raw)
  To: Sylwester Nawrocki
  Cc: kgene.kim, linux-samsung-soc, linux-arm-kernel, linux-serial,
	oselas, Chander Kashyap
On Sunday 21 of July 2013 21:42:45 Sylwester Nawrocki wrote:
> This patch restores serial port operation which has been broken since
> commit 60e93575476f90a72146b51283f514da655410a7
> serial: samsung: enable clock before clearing pending interrupts during
> init
> 
> That commit only uncovered the real issue which was missing clkdev
> entries for the "uart" clocks on S3C2440. It went unnoticed so far
> because return value of clk API calls were not being checked at all
> in the samsung serial port driver.
> 
> This patch should be backported to at least 3.10 stable kernel, since
> the serial port has not been working on s3c2440 since 3.10-rc5.
> 
> Cc: Chander Kashyap <chander.kashyap@linaro.org>
> Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> [on S3C2440 SoC based Mini2440 board]
> Tested-by: Sylwester Nawrocki <sylvester.nawrocki@gmail.com>
> ---
> 
> There seems to be something wrong with the process, as things are
> getting broken late in the -rc cycle.  It seems author of the above
> mentioned commit didn't test all 6 SoC series that the samsung
> serial driver handles.  While I can understand they might not have
> all required hardware it's a bit irritating that the patch that
> in practice caused regression appeared only on linux-serial mailing
> list. And it's not evident it was even tried to test it on all
> potentially affected platforms. It not clear on what platforms
> the patch has been tested.
> 
> My humble suggestion is, _please_ do send patches that affect multiple
> Samsung SoCs to linux-samsung@vger.kernel.org.  So there is better
> review and test exposure.  And we can reduce mess like this happening
> in the future.
> 
> Thanks!
> Sylwester
> ---
>  arch/arm/mach-s3c24xx/clock-s3c2410.c      |  161
> +++++++++++++++++----------- arch/arm/mach-s3c24xx/clock-s3c2440.c     
> |    3 +
>  arch/arm/plat-samsung/include/plat/clock.h |    5 +
>  3 files changed, 106 insertions(+), 63 deletions(-)
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Best regards,
Tomasz
^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART
       [not found] ` <1374435765-6747-1-git-send-email-sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
@ 2013-07-23  7:57   ` Jürgen Beisert
  0 siblings, 0 replies; 4+ messages in thread
From: Jürgen Beisert @ 2013-07-23  7:57 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	oselas-/6JGXy0y6WNX4V3MAk3apN53zsg1cpMQ, Chander Kashyap,
	kgene.kim-Sze3O3UU22JBDgjK7y7TUQ,
	linux-serial-u79uwXL29TY76Z2rM5mHXA, Sylwester Nawrocki
On Sunday 21 July 2013 21:42:45 Sylwester Nawrocki wrote:
> This patch restores serial port operation which has been broken since
> commit 60e93575476f90a72146b51283f514da655410a7
> serial: samsung: enable clock before clearing pending interrupts during
> init
>
> That commit only uncovered the real issue which was missing clkdev
> entries for the "uart" clocks on S3C2440. It went unnoticed so far
> because return value of clk API calls were not being checked at all
> in the samsung serial port driver.
>
> This patch should be backported to at least 3.10 stable kernel, since
> the serial port has not been working on s3c2440 since 3.10-rc5.
>
> Cc: Chander Kashyap <chander.kashyap-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> [on S3C2440 SoC based Mini2440 board]
> Tested-by: Sylwester Nawrocki <sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> [...]
Tested-by: Juergen Beisert <jbe-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Peiner Str. 6-8, 31137 Hildesheim, Germany    | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |
^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-07-23  7:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-21 19:42 [PATCH] ARM: S3C24XX: Add missing clkdev entries for s3c2440 UART Sylwester Nawrocki
2013-07-22  4:39 ` Kukjin Kim
2013-07-22 17:14 ` Tomasz Figa
     [not found] ` <1374435765-6747-1-git-send-email-sylvester.nawrocki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2013-07-23  7:57   ` Jürgen Beisert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).