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@xxxxxxxxxx>
> Signed-off-by: Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx>
> [on S3C2440 SoC based Mini2440 board]
> Tested-by: Sylwester Nawrocki <sylvester.nawrocki@xxxxxxxxx>
> ---
>
> 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@xxxxxxxxxxxxxxx. 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
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
