* [PATCH 00/16] Omap2 patches for post 2.6.26 @ 2008-06-07 1:47 ` Tony Lindgren 2008-06-07 1:47 ` [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks Tony Lindgren 0 siblings, 1 reply; 6+ messages in thread From: Tony Lindgren @ 2008-06-07 1:47 UTC (permalink / raw) To: linux-arm-kernel; +Cc: Tony Lindgren, linux-omap Hi all, This patch series contains various omap2 patchs from linux-omap tree. Big chunk of the patches still improve the clock code. Also more clean-up to allow compiling omap2 and omap3 support into the same kernel. The patch also adds core omap3430 support, but no board files or PM at at this point. This patch series depends on earlier patch series: 1. Omap upstream patches for post 2.6.26 2. Omap2 clock patches for post 2.6.26 This patch series also available via git at: git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6.git omap2-upstream Regards, Tony ------------------------------------------------------------------- List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 00/16] Omap2 patches for post 2.6.26 @ 2008-06-07 1:47 ` Tony Lindgren 2008-06-07 1:47 ` [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks Tony Lindgren 0 siblings, 1 reply; 6+ messages in thread From: Tony Lindgren @ 2008-06-07 1:47 UTC (permalink / raw) To: linux-arm-kernel; +Cc: Tony Lindgren, linux-omap Hi all, This patch series contains various omap2 patchs from linux-omap tree. Big chunk of the patches still improve the clock code. Also more clean-up to allow compiling omap2 and omap3 support into the same kernel. The patch also adds core omap3430 support, but no board files or PM at at this point. This patch series depends on earlier patch series: 1. Omap upstream patches for post 2.6.26 2. Omap2 clock patches for post 2.6.26 This patch series also available via git at: git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap-2.6.git omap2-upstream Regards, Tony ------------------------------------------------------------------- List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks 2008-06-07 1:47 ` Tony Lindgren @ 2008-06-07 1:47 ` Tony Lindgren 2008-08-19 16:40 ` Russell King - ARM Linux 0 siblings, 1 reply; 6+ messages in thread From: Tony Lindgren @ 2008-06-07 1:47 UTC (permalink / raw) To: linux-arm-kernel; +Cc: linux-omap, Jouni Hogander, Tony Lindgren From: Jouni Hogander <jouni.hogander@nokia.com> This patch adds common function to enable/disable omap2/3 uart clocks. Enabled uarts are passed by bootloader in atags and clocks for these enabled uarts are touched. Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com> Signed-off-by: Tony Lindgren <tony@atomide.com> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c index e9c367f..b0fa582 100644 --- a/arch/arm/mach-omap2/serial.c +++ b/arch/arm/mach-omap2/serial.c @@ -3,7 +3,7 @@ * * OMAP2 serial support. * - * Copyright (C) 2005 Nokia Corporation + * Copyright (C) 2005-2008 Nokia Corporation * Author: Paul Mundt <paul.mundt@nokia.com> * * Based off of arch/arm/mach-omap/omap1/serial.c @@ -18,43 +18,39 @@ #include <linux/serial_reg.h> #include <linux/clk.h> -#include <asm/io.h> +#include <linux/io.h> #include <asm/arch/common.h> #include <asm/arch/board.h> -static struct clk * uart1_ick = NULL; -static struct clk * uart1_fck = NULL; -static struct clk * uart2_ick = NULL; -static struct clk * uart2_fck = NULL; -static struct clk * uart3_ick = NULL; -static struct clk * uart3_fck = NULL; +static struct clk *uart_ick[OMAP_MAX_NR_PORTS]; +static struct clk *uart_fck[OMAP_MAX_NR_PORTS]; static struct plat_serial8250_port serial_platform_data[] = { { - .membase = (char *)IO_ADDRESS(OMAP_UART1_BASE), + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART1_BASE), .mapbase = (unsigned long)OMAP_UART1_BASE, .irq = 72, .flags = UPF_BOOT_AUTOCONF, .iotype = UPIO_MEM, .regshift = 2, - .uartclk = OMAP16XX_BASE_BAUD * 16, + .uartclk = OMAP24XX_BASE_BAUD * 16, }, { - .membase = (char *)IO_ADDRESS(OMAP_UART2_BASE), + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART2_BASE), .mapbase = (unsigned long)OMAP_UART2_BASE, .irq = 73, .flags = UPF_BOOT_AUTOCONF, .iotype = UPIO_MEM, .regshift = 2, - .uartclk = OMAP16XX_BASE_BAUD * 16, + .uartclk = OMAP24XX_BASE_BAUD * 16, }, { - .membase = (char *)IO_ADDRESS(OMAP_UART3_BASE), + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART3_BASE), .mapbase = (unsigned long)OMAP_UART3_BASE, .irq = 74, .flags = UPF_BOOT_AUTOCONF, .iotype = UPIO_MEM, .regshift = 2, - .uartclk = OMAP16XX_BASE_BAUD * 16, + .uartclk = OMAP24XX_BASE_BAUD * 16, }, { .flags = 0 } @@ -71,7 +67,7 @@ static inline void serial_write_reg(struct plat_serial8250_port *p, int offset, int value) { offset <<= p->regshift; - __raw_writeb(value, (unsigned long)(p->membase + offset)); + __raw_writeb(value, p->membase + offset); } /* @@ -87,10 +83,27 @@ static inline void __init omap_serial_reset(struct plat_serial8250_port *p) serial_write_reg(p, UART_OMAP_SYSC, (0x02 << 3) | (1 << 2) | (1 << 0)); } -void __init omap_serial_init() +void omap_serial_enable_clocks(int enable) +{ + int i; + for (i = 0; i < OMAP_MAX_NR_PORTS; i++) { + if (uart_ick[i] && uart_fck[i]) { + if (enable) { + clk_enable(uart_ick[i]); + clk_enable(uart_fck[i]); + } else { + clk_disable(uart_ick[i]); + clk_disable(uart_fck[i]); + } + } + } +} + +void __init omap_serial_init(void) { int i; const struct omap_uart_config *info; + char name[16]; /* * Make sure the serial ports are muxed on at this point. @@ -98,8 +111,7 @@ void __init omap_serial_init() * if not needed. */ - info = omap_get_config(OMAP_TAG_UART, - struct omap_uart_config); + info = omap_get_config(OMAP_TAG_UART, struct omap_uart_config); if (info == NULL) return; @@ -108,58 +120,26 @@ void __init omap_serial_init() struct plat_serial8250_port *p = serial_platform_data + i; if (!(info->enabled_uarts & (1 << i))) { - p->membase = 0; + p->membase = NULL; p->mapbase = 0; continue; } - switch (i) { - case 0: - uart1_ick = clk_get(NULL, "uart1_ick"); - if (IS_ERR(uart1_ick)) - printk("Could not get uart1_ick\n"); - else { - clk_enable(uart1_ick); - } - - uart1_fck = clk_get(NULL, "uart1_fck"); - if (IS_ERR(uart1_fck)) - printk("Could not get uart1_fck\n"); - else { - clk_enable(uart1_fck); - } - break; - case 1: - uart2_ick = clk_get(NULL, "uart2_ick"); - if (IS_ERR(uart2_ick)) - printk("Could not get uart2_ick\n"); - else { - clk_enable(uart2_ick); - } - - uart2_fck = clk_get(NULL, "uart2_fck"); - if (IS_ERR(uart2_fck)) - printk("Could not get uart2_fck\n"); - else { - clk_enable(uart2_fck); - } - break; - case 2: - uart3_ick = clk_get(NULL, "uart3_ick"); - if (IS_ERR(uart3_ick)) - printk("Could not get uart3_ick\n"); - else { - clk_enable(uart3_ick); - } - - uart3_fck = clk_get(NULL, "uart3_fck"); - if (IS_ERR(uart3_fck)) - printk("Could not get uart3_fck\n"); - else { - clk_enable(uart3_fck); - } - break; - } + sprintf(name, "uart%d_ick", i+1); + uart_ick[i] = clk_get(NULL, name); + if (IS_ERR(uart_ick[i])) { + printk(KERN_ERR "Could not get uart%d_ick\n", i+1); + uart_ick[i] = NULL; + } else + clk_enable(uart_ick[i]); + + sprintf(name, "uart%d_fck", i+1); + uart_fck[i] = clk_get(NULL, name); + if (IS_ERR(uart_fck[i])) { + printk(KERN_ERR "Could not get uart%d_fck\n", i+1); + uart_fck[i] = NULL; + } else + clk_enable(uart_fck[i]); omap_serial_reset(p); } diff --git a/include/asm-arm/arch-omap/common.h b/include/asm-arm/arch-omap/common.h index 8ac0307..e015e94 100644 --- a/include/asm-arm/arch-omap/common.h +++ b/include/asm-arm/arch-omap/common.h @@ -34,6 +34,7 @@ struct sys_timer; extern void omap_map_common_io(void); extern struct sys_timer omap_timer; extern void omap_serial_init(void); +extern void omap_serial_enable_clocks(int enable); #ifdef CONFIG_I2C_OMAP extern int omap_register_i2c_bus(int bus_id, u32 clkrate, struct i2c_board_info const *info, diff --git a/include/asm-arm/arch-omap/serial.h b/include/asm-arm/arch-omap/serial.h index 79a5297..b6771b4 100644 --- a/include/asm-arm/arch-omap/serial.h +++ b/include/asm-arm/arch-omap/serial.h @@ -20,11 +20,17 @@ #define OMAP_UART1_BASE 0x4806a000 #define OMAP_UART2_BASE 0x4806c000 #define OMAP_UART3_BASE 0x4806e000 +#elif defined(CONFIG_ARCH_OMAP3) +/* OMAP3 serial ports */ +#define OMAP_UART1_BASE 0x4806a000 +#define OMAP_UART2_BASE 0x4806c000 +#define OMAP_UART3_BASE 0x49020000 #endif #define OMAP_MAX_NR_PORTS 3 #define OMAP1510_BASE_BAUD (12000000/16) #define OMAP16XX_BASE_BAUD (48000000/16) +#define OMAP24XX_BASE_BAUD (48000000/16) #define is_omap_port(p) ({int __ret = 0; \ if (p == IO_ADDRESS(OMAP_UART1_BASE) || \ ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks 2008-06-07 1:47 ` [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks Tony Lindgren @ 2008-08-19 16:40 ` Russell King - ARM Linux 2008-08-20 5:49 ` Högander Jouni 0 siblings, 1 reply; 6+ messages in thread From: Russell King - ARM Linux @ 2008-08-19 16:40 UTC (permalink / raw) To: Tony Lindgren; +Cc: linux-arm-kernel, linux-omap, Jouni Hogander On Fri, Jun 06, 2008 at 06:47:42PM -0700, Tony Lindgren wrote: > This patch adds common function to enable/disable omap2/3 uart > clocks. Enabled uarts are passed by bootloader in atags and clocks for > these enabled uarts are touched. In some ways, this patch is a good thing, in others it's outrageous. > -static struct clk * uart1_ick = NULL; > -static struct clk * uart1_fck = NULL; > -static struct clk * uart2_ick = NULL; > -static struct clk * uart2_fck = NULL; > -static struct clk * uart3_ick = NULL; > -static struct clk * uart3_fck = NULL; > +static struct clk *uart_ick[OMAP_MAX_NR_PORTS]; > +static struct clk *uart_fck[OMAP_MAX_NR_PORTS]; > > static struct plat_serial8250_port serial_platform_data[] = { > { > - .membase = (char *)IO_ADDRESS(OMAP_UART1_BASE), > + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART1_BASE), __force in drivers is a big no no, immediate patch rejection. Drivers do not use __force. The reason for adding __iomem is to pick up where people are doing stupid things with pointers, and __force is added in _private_ code (eg, functions supporting IO, mapping functions, etc) to provide a way of saying "this is part of the infrastructure and its permitted to do this conversion." The correct place for that cast, in its entirety, is inside the IO_ADDRESS macro. > @@ -71,7 +67,7 @@ static inline void serial_write_reg(struct plat_serial8250_port *p, int offset, > int value) > { > offset <<= p->regshift; > - __raw_writeb(value, (unsigned long)(p->membase + offset)); > + __raw_writeb(value, p->membase + offset); Good. > @@ -87,10 +83,27 @@ static inline void __init omap_serial_reset(struct plat_serial8250_port *p) > serial_write_reg(p, UART_OMAP_SYSC, (0x02 << 3) | (1 << 2) | (1 << 0)); > } > > -void __init omap_serial_init() > +void omap_serial_enable_clocks(int enable) > +{ > + int i; > + for (i = 0; i < OMAP_MAX_NR_PORTS; i++) { > + if (uart_ick[i] && uart_fck[i]) { > + if (enable) { > + clk_enable(uart_ick[i]); > + clk_enable(uart_fck[i]); > + } else { > + clk_disable(uart_ick[i]); > + clk_disable(uart_fck[i]); > + } > + } > + } > +} Urgh. Why enable all clocks? I thought OMAP was about being as lean and mean as possible as far as power management goes, so why enable everything and disable everything? Looking at the omapzoom tree, this function isn't even referenced by another part of the kernel, so maybe this function is just cruft? > + sprintf(name, "uart%d_ick", i+1); > + uart_ick[i] = clk_get(NULL, name); > + if (IS_ERR(uart_ick[i])) { > + printk(KERN_ERR "Could not get uart%d_ick\n", i+1); > + uart_ick[i] = NULL; > + } else > + clk_enable(uart_ick[i]); > + > + sprintf(name, "uart%d_fck", i+1); > + uart_fck[i] = clk_get(NULL, name); > + if (IS_ERR(uart_fck[i])) { > + printk(KERN_ERR "Could not get uart%d_fck\n", i+1); > + uart_fck[i] = NULL; > + } else > + clk_enable(uart_fck[i]); Definitely an improvement, but still some way to go yet... but not a reason not to get this in (once the above issues have been resolved.) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks 2008-08-19 16:40 ` Russell King - ARM Linux @ 2008-08-20 5:49 ` Högander Jouni 2008-08-20 7:28 ` Tony Lindgren 0 siblings, 1 reply; 6+ messages in thread From: Högander Jouni @ 2008-08-20 5:49 UTC (permalink / raw) To: ext Russell King - ARM Linux; +Cc: Tony Lindgren, linux-arm-kernel, linux-omap "ext Russell King - ARM Linux" <linux@arm.linux.org.uk> writes: > On Fri, Jun 06, 2008 at 06:47:42PM -0700, Tony Lindgren wrote: >> This patch adds common function to enable/disable omap2/3 uart >> clocks. Enabled uarts are passed by bootloader in atags and clocks for >> these enabled uarts are touched. > > In some ways, this patch is a good thing, in others it's outrageous. > >> -static struct clk * uart1_ick = NULL; >> -static struct clk * uart1_fck = NULL; >> -static struct clk * uart2_ick = NULL; >> -static struct clk * uart2_fck = NULL; >> -static struct clk * uart3_ick = NULL; >> -static struct clk * uart3_fck = NULL; >> +static struct clk *uart_ick[OMAP_MAX_NR_PORTS]; >> +static struct clk *uart_fck[OMAP_MAX_NR_PORTS]; >> >> static struct plat_serial8250_port serial_platform_data[] = { >> { >> - .membase = (char *)IO_ADDRESS(OMAP_UART1_BASE), >> + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART1_BASE), > > __force in drivers is a big no no, immediate patch rejection. Drivers > do not use __force. > > The reason for adding __iomem is to pick up where people are doing > stupid things with pointers, and __force is added in _private_ code > (eg, functions supporting IO, mapping functions, etc) to provide a > way of saying "this is part of the infrastructure and its permitted > to do this conversion." > > The correct place for that cast, in its entirety, is inside the > IO_ADDRESS macro. > >> @@ -71,7 +67,7 @@ static inline void serial_write_reg(struct plat_serial8250_port *p, int offset, >> int value) >> { >> offset <<= p->regshift; >> - __raw_writeb(value, (unsigned long)(p->membase + offset)); >> + __raw_writeb(value, p->membase + offset); > > Good. > >> @@ -87,10 +83,27 @@ static inline void __init omap_serial_reset(struct plat_serial8250_port *p) >> serial_write_reg(p, UART_OMAP_SYSC, (0x02 << 3) | (1 << 2) | (1 << 0)); >> } >> >> -void __init omap_serial_init() >> +void omap_serial_enable_clocks(int enable) >> +{ >> + int i; >> + for (i = 0; i < OMAP_MAX_NR_PORTS; i++) { >> + if (uart_ick[i] && uart_fck[i]) { >> + if (enable) { >> + clk_enable(uart_ick[i]); >> + clk_enable(uart_fck[i]); >> + } else { >> + clk_disable(uart_ick[i]); >> + clk_disable(uart_fck[i]); >> + } >> + } >> + } >> +} > > Urgh. Why enable all clocks? I thought OMAP was about being as lean > and mean as possible as far as power management goes, so why enable > everything and disable everything? Not everything, just those that are marked as a enabled by a bootloader. This mean basically serial-console uart. If all three omap uarts are used as a serial console, then this function will disable/enable clocks for all of them. > Looking at the omapzoom tree, this function isn't even referenced by > another part of the kernel, so maybe this function is just cruft? This function is added for dynamic PM to have a mean to enable/disable serial console clocks. My intention is to be able to have PM when serial console is used without need to reboot the device and changing atags from the bootloader. There are patches on linux-omap list which are using this. They are not pushed yet. > >> + sprintf(name, "uart%d_ick", i+1); >> + uart_ick[i] = clk_get(NULL, name); >> + if (IS_ERR(uart_ick[i])) { >> + printk(KERN_ERR "Could not get uart%d_ick\n", i+1); >> + uart_ick[i] = NULL; >> + } else >> + clk_enable(uart_ick[i]); >> + >> + sprintf(name, "uart%d_fck", i+1); >> + uart_fck[i] = clk_get(NULL, name); >> + if (IS_ERR(uart_fck[i])) { >> + printk(KERN_ERR "Could not get uart%d_fck\n", i+1); >> + uart_fck[i] = NULL; >> + } else >> + clk_enable(uart_fck[i]); > > Definitely an improvement, but still some way to go yet... but not a > reason not to get this in (once the above issues have been resolved.) > > -- Jouni Högander -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks 2008-08-20 5:49 ` Högander Jouni @ 2008-08-20 7:28 ` Tony Lindgren 2008-08-23 22:21 ` Tony Lindgren 0 siblings, 1 reply; 6+ messages in thread From: Tony Lindgren @ 2008-08-20 7:28 UTC (permalink / raw) To: Högander Jouni Cc: ext Russell King - ARM Linux, linux-arm-kernel, linux-omap * Högander Jouni <jouni.hogander@nokia.com> [080820 08:50]: > "ext Russell King - ARM Linux" <linux@arm.linux.org.uk> writes: > > > On Fri, Jun 06, 2008 at 06:47:42PM -0700, Tony Lindgren wrote: > >> This patch adds common function to enable/disable omap2/3 uart > >> clocks. Enabled uarts are passed by bootloader in atags and clocks for > >> these enabled uarts are touched. > > > > In some ways, this patch is a good thing, in others it's outrageous. > > > >> -static struct clk * uart1_ick = NULL; > >> -static struct clk * uart1_fck = NULL; > >> -static struct clk * uart2_ick = NULL; > >> -static struct clk * uart2_fck = NULL; > >> -static struct clk * uart3_ick = NULL; > >> -static struct clk * uart3_fck = NULL; > >> +static struct clk *uart_ick[OMAP_MAX_NR_PORTS]; > >> +static struct clk *uart_fck[OMAP_MAX_NR_PORTS]; > >> > >> static struct plat_serial8250_port serial_platform_data[] = { > >> { > >> - .membase = (char *)IO_ADDRESS(OMAP_UART1_BASE), > >> + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART1_BASE), > > > > __force in drivers is a big no no, immediate patch rejection. Drivers > > do not use __force. > > > > The reason for adding __iomem is to pick up where people are doing > > stupid things with pointers, and __force is added in _private_ code > > (eg, functions supporting IO, mapping functions, etc) to provide a > > way of saying "this is part of the infrastructure and its permitted > > to do this conversion." > > > > The correct place for that cast, in its entirety, is inside the > > IO_ADDRESS macro. > > > >> @@ -71,7 +67,7 @@ static inline void serial_write_reg(struct plat_serial8250_port *p, int offset, > >> int value) > >> { > >> offset <<= p->regshift; > >> - __raw_writeb(value, (unsigned long)(p->membase + offset)); > >> + __raw_writeb(value, p->membase + offset); > > > > Good. > > > >> @@ -87,10 +83,27 @@ static inline void __init omap_serial_reset(struct plat_serial8250_port *p) > >> serial_write_reg(p, UART_OMAP_SYSC, (0x02 << 3) | (1 << 2) | (1 << 0)); > >> } > >> > >> -void __init omap_serial_init() > >> +void omap_serial_enable_clocks(int enable) > >> +{ > >> + int i; > >> + for (i = 0; i < OMAP_MAX_NR_PORTS; i++) { > >> + if (uart_ick[i] && uart_fck[i]) { > >> + if (enable) { > >> + clk_enable(uart_ick[i]); > >> + clk_enable(uart_fck[i]); > >> + } else { > >> + clk_disable(uart_ick[i]); > >> + clk_disable(uart_fck[i]); > >> + } > >> + } > >> + } > >> +} > > > > Urgh. Why enable all clocks? I thought OMAP was about being as lean > > and mean as possible as far as power management goes, so why enable > > everything and disable everything? > > Not everything, just those that are marked as a enabled by a > bootloader. This mean basically serial-console uart. If all three omap > uarts are used as a serial console, then this function will > disable/enable clocks for all of them. Or marked as enabled in board-*.c file. The omap ATAGs are not going to mainline tree as discussed earlier and will get removed from l-o tree eventually also. Any bootloader tags must be generic, such as ATAG_UART or something similar. > > Looking at the omapzoom tree, this function isn't even referenced by > > another part of the kernel, so maybe this function is just cruft? > > This function is added for dynamic PM to have a mean to enable/disable > serial console clocks. My intention is to be able to have PM when > serial console is used without need to reboot the device and changing > atags from the bootloader. There are patches on linux-omap list which are > using this. They are not pushed yet. The long term solution is to switch to omap-uart instead of 8250.c so we can support DMA and dynamic power and clocking. > > > > >> + sprintf(name, "uart%d_ick", i+1); > >> + uart_ick[i] = clk_get(NULL, name); > >> + if (IS_ERR(uart_ick[i])) { > >> + printk(KERN_ERR "Could not get uart%d_ick\n", i+1); > >> + uart_ick[i] = NULL; > >> + } else > >> + clk_enable(uart_ick[i]); > >> + > >> + sprintf(name, "uart%d_fck", i+1); > >> + uart_fck[i] = clk_get(NULL, name); > >> + if (IS_ERR(uart_fck[i])) { > >> + printk(KERN_ERR "Could not get uart%d_fck\n", i+1); > >> + uart_fck[i] = NULL; > >> + } else > >> + clk_enable(uart_fck[i]); > > > > Definitely an improvement, but still some way to go yet... but not a > > reason not to get this in (once the above issues have been resolved.) > > > > > > -- > Jouni Högander > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks 2008-08-20 7:28 ` Tony Lindgren @ 2008-08-23 22:21 ` Tony Lindgren 0 siblings, 0 replies; 6+ messages in thread From: Tony Lindgren @ 2008-08-23 22:21 UTC (permalink / raw) To: Högander Jouni Cc: ext Russell King - ARM Linux, linux-arm-kernel, linux-omap [-- Attachment #1: Type: text/plain, Size: 4771 bytes --] * Tony Lindgren <tony@atomide.com> [080820 00:28]: > * Högander Jouni <jouni.hogander@nokia.com> [080820 08:50]: > > "ext Russell King - ARM Linux" <linux@arm.linux.org.uk> writes: > > > > > On Fri, Jun 06, 2008 at 06:47:42PM -0700, Tony Lindgren wrote: > > >> This patch adds common function to enable/disable omap2/3 uart > > >> clocks. Enabled uarts are passed by bootloader in atags and clocks for > > >> these enabled uarts are touched. > > > > > > In some ways, this patch is a good thing, in others it's outrageous. > > > > > >> -static struct clk * uart1_ick = NULL; > > >> -static struct clk * uart1_fck = NULL; > > >> -static struct clk * uart2_ick = NULL; > > >> -static struct clk * uart2_fck = NULL; > > >> -static struct clk * uart3_ick = NULL; > > >> -static struct clk * uart3_fck = NULL; > > >> +static struct clk *uart_ick[OMAP_MAX_NR_PORTS]; > > >> +static struct clk *uart_fck[OMAP_MAX_NR_PORTS]; > > >> > > >> static struct plat_serial8250_port serial_platform_data[] = { > > >> { > > >> - .membase = (char *)IO_ADDRESS(OMAP_UART1_BASE), > > >> + .membase = (__force void __iomem *)IO_ADDRESS(OMAP_UART1_BASE), > > > > > > __force in drivers is a big no no, immediate patch rejection. Drivers > > > do not use __force. > > > > > > The reason for adding __iomem is to pick up where people are doing > > > stupid things with pointers, and __force is added in _private_ code > > > (eg, functions supporting IO, mapping functions, etc) to provide a > > > way of saying "this is part of the infrastructure and its permitted > > > to do this conversion." > > > > > > The correct place for that cast, in its entirety, is inside the > > > IO_ADDRESS macro. > > > > > >> @@ -71,7 +67,7 @@ static inline void serial_write_reg(struct plat_serial8250_port *p, int offset, > > >> int value) > > >> { > > >> offset <<= p->regshift; > > >> - __raw_writeb(value, (unsigned long)(p->membase + offset)); > > >> + __raw_writeb(value, p->membase + offset); > > > > > > Good. > > > > > >> @@ -87,10 +83,27 @@ static inline void __init omap_serial_reset(struct plat_serial8250_port *p) > > >> serial_write_reg(p, UART_OMAP_SYSC, (0x02 << 3) | (1 << 2) | (1 << 0)); > > >> } > > >> > > >> -void __init omap_serial_init() > > >> +void omap_serial_enable_clocks(int enable) > > >> +{ > > >> + int i; > > >> + for (i = 0; i < OMAP_MAX_NR_PORTS; i++) { > > >> + if (uart_ick[i] && uart_fck[i]) { > > >> + if (enable) { > > >> + clk_enable(uart_ick[i]); > > >> + clk_enable(uart_fck[i]); > > >> + } else { > > >> + clk_disable(uart_ick[i]); > > >> + clk_disable(uart_fck[i]); > > >> + } > > >> + } > > >> + } > > >> +} > > > > > > Urgh. Why enable all clocks? I thought OMAP was about being as lean > > > and mean as possible as far as power management goes, so why enable > > > everything and disable everything? > > > > Not everything, just those that are marked as a enabled by a > > bootloader. This mean basically serial-console uart. If all three omap > > uarts are used as a serial console, then this function will > > disable/enable clocks for all of them. > > Or marked as enabled in board-*.c file. The omap ATAGs are not going > to mainline tree as discussed earlier and will get removed from l-o > tree eventually also. Any bootloader tags must be generic, such as > ATAG_UART or something similar. > > > > Looking at the omapzoom tree, this function isn't even referenced by > > > another part of the kernel, so maybe this function is just cruft? > > > > This function is added for dynamic PM to have a mean to enable/disable > > serial console clocks. My intention is to be able to have PM when > > serial console is used without need to reboot the device and changing > > atags from the bootloader. There are patches on linux-omap list which are > > using this. They are not pushed yet. > > The long term solution is to switch to omap-uart instead of 8250.c so we > can support DMA and dynamic power and clocking. > > > > > > > > >> + sprintf(name, "uart%d_ick", i+1); > > >> + uart_ick[i] = clk_get(NULL, name); > > >> + if (IS_ERR(uart_ick[i])) { > > >> + printk(KERN_ERR "Could not get uart%d_ick\n", i+1); > > >> + uart_ick[i] = NULL; > > >> + } else > > >> + clk_enable(uart_ick[i]); > > >> + > > >> + sprintf(name, "uart%d_fck", i+1); > > >> + uart_fck[i] = clk_get(NULL, name); > > >> + if (IS_ERR(uart_fck[i])) { > > >> + printk(KERN_ERR "Could not get uart%d_fck\n", i+1); > > >> + uart_fck[i] = NULL; > > >> + } else > > >> + clk_enable(uart_fck[i]); > > > > > > Definitely an improvement, but still some way to go yet... but not a > > > reason not to get this in (once the above issues have been resolved.) > > > > > > > > Here's this one updated with __force removed. Tony [-- Attachment #2: 0001-serial --] [-- Type: text/plain, Size: 6803 bytes --] >From bb49b3cd1da2f23b344fcaf32607d280731d2653 Mon Sep 17 00:00:00 2001 From: Jouni Hogander <jouni.hogander@nokia.com> Date: Sat, 23 Aug 2008 15:19:46 -0700 Subject: [PATCH] ARM: OMAP2 Provide function to enable/disable uart clocks This patch adds common function to enable/disable omap2/3 uart clocks. Enabled uarts are passed by bootloader in atags and clocks for these enabled uarts are touched. Signed-off-by: Jouni Hogander <jouni.hogander@nokia.com> Signed-off-by: Tony Lindgren <tony@atomide.com> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c index adc8a26..bd6f8c9 100644 --- a/arch/arm/mach-omap2/serial.c +++ b/arch/arm/mach-omap2/serial.c @@ -3,7 +3,7 @@ * * OMAP2 serial support. * - * Copyright (C) 2005 Nokia Corporation + * Copyright (C) 2005-2008 Nokia Corporation * Author: Paul Mundt <paul.mundt@nokia.com> * * Based off of arch/arm/mach-omap/omap1/serial.c @@ -18,43 +18,39 @@ #include <linux/serial_reg.h> #include <linux/clk.h> -#include <asm/io.h> +#include <linux/io.h> #include <mach/common.h> #include <mach/board.h> -static struct clk * uart1_ick = NULL; -static struct clk * uart1_fck = NULL; -static struct clk * uart2_ick = NULL; -static struct clk * uart2_fck = NULL; -static struct clk * uart3_ick = NULL; -static struct clk * uart3_fck = NULL; +static struct clk *uart_ick[OMAP_MAX_NR_PORTS]; +static struct clk *uart_fck[OMAP_MAX_NR_PORTS]; static struct plat_serial8250_port serial_platform_data[] = { { - .membase = (char *)IO_ADDRESS(OMAP_UART1_BASE), + .membase = (void __iomem *)IO_ADDRESS(OMAP_UART1_BASE), .mapbase = (unsigned long)OMAP_UART1_BASE, .irq = 72, .flags = UPF_BOOT_AUTOCONF, .iotype = UPIO_MEM, .regshift = 2, - .uartclk = OMAP16XX_BASE_BAUD * 16, + .uartclk = OMAP24XX_BASE_BAUD * 16, }, { - .membase = (char *)IO_ADDRESS(OMAP_UART2_BASE), + .membase = (void __iomem *)IO_ADDRESS(OMAP_UART2_BASE), .mapbase = (unsigned long)OMAP_UART2_BASE, .irq = 73, .flags = UPF_BOOT_AUTOCONF, .iotype = UPIO_MEM, .regshift = 2, - .uartclk = OMAP16XX_BASE_BAUD * 16, + .uartclk = OMAP24XX_BASE_BAUD * 16, }, { - .membase = (char *)IO_ADDRESS(OMAP_UART3_BASE), + .membase = (void __iomem *)IO_ADDRESS(OMAP_UART3_BASE), .mapbase = (unsigned long)OMAP_UART3_BASE, .irq = 74, .flags = UPF_BOOT_AUTOCONF, .iotype = UPIO_MEM, .regshift = 2, - .uartclk = OMAP16XX_BASE_BAUD * 16, + .uartclk = OMAP24XX_BASE_BAUD * 16, }, { .flags = 0 } @@ -71,7 +67,7 @@ static inline void serial_write_reg(struct plat_serial8250_port *p, int offset, int value) { offset <<= p->regshift; - __raw_writeb(value, (unsigned long)(p->membase + offset)); + __raw_writeb(value, p->membase + offset); } /* @@ -87,10 +83,27 @@ static inline void __init omap_serial_reset(struct plat_serial8250_port *p) serial_write_reg(p, UART_OMAP_SYSC, (0x02 << 3) | (1 << 2) | (1 << 0)); } -void __init omap_serial_init() +void omap_serial_enable_clocks(int enable) +{ + int i; + for (i = 0; i < OMAP_MAX_NR_PORTS; i++) { + if (uart_ick[i] && uart_fck[i]) { + if (enable) { + clk_enable(uart_ick[i]); + clk_enable(uart_fck[i]); + } else { + clk_disable(uart_ick[i]); + clk_disable(uart_fck[i]); + } + } + } +} + +void __init omap_serial_init(void) { int i; const struct omap_uart_config *info; + char name[16]; /* * Make sure the serial ports are muxed on at this point. @@ -98,8 +111,7 @@ void __init omap_serial_init() * if not needed. */ - info = omap_get_config(OMAP_TAG_UART, - struct omap_uart_config); + info = omap_get_config(OMAP_TAG_UART, struct omap_uart_config); if (info == NULL) return; @@ -108,58 +120,26 @@ void __init omap_serial_init() struct plat_serial8250_port *p = serial_platform_data + i; if (!(info->enabled_uarts & (1 << i))) { - p->membase = 0; + p->membase = NULL; p->mapbase = 0; continue; } - switch (i) { - case 0: - uart1_ick = clk_get(NULL, "uart1_ick"); - if (IS_ERR(uart1_ick)) - printk("Could not get uart1_ick\n"); - else { - clk_enable(uart1_ick); - } - - uart1_fck = clk_get(NULL, "uart1_fck"); - if (IS_ERR(uart1_fck)) - printk("Could not get uart1_fck\n"); - else { - clk_enable(uart1_fck); - } - break; - case 1: - uart2_ick = clk_get(NULL, "uart2_ick"); - if (IS_ERR(uart2_ick)) - printk("Could not get uart2_ick\n"); - else { - clk_enable(uart2_ick); - } - - uart2_fck = clk_get(NULL, "uart2_fck"); - if (IS_ERR(uart2_fck)) - printk("Could not get uart2_fck\n"); - else { - clk_enable(uart2_fck); - } - break; - case 2: - uart3_ick = clk_get(NULL, "uart3_ick"); - if (IS_ERR(uart3_ick)) - printk("Could not get uart3_ick\n"); - else { - clk_enable(uart3_ick); - } - - uart3_fck = clk_get(NULL, "uart3_fck"); - if (IS_ERR(uart3_fck)) - printk("Could not get uart3_fck\n"); - else { - clk_enable(uart3_fck); - } - break; - } + sprintf(name, "uart%d_ick", i+1); + uart_ick[i] = clk_get(NULL, name); + if (IS_ERR(uart_ick[i])) { + printk(KERN_ERR "Could not get uart%d_ick\n", i+1); + uart_ick[i] = NULL; + } else + clk_enable(uart_ick[i]); + + sprintf(name, "uart%d_fck", i+1); + uart_fck[i] = clk_get(NULL, name); + if (IS_ERR(uart_fck[i])) { + printk(KERN_ERR "Could not get uart%d_fck\n", i+1); + uart_fck[i] = NULL; + } else + clk_enable(uart_fck[i]); omap_serial_reset(p); } diff --git a/arch/arm/plat-omap/include/mach/common.h b/arch/arm/plat-omap/include/mach/common.h index 0609311..515c628 100644 --- a/arch/arm/plat-omap/include/mach/common.h +++ b/arch/arm/plat-omap/include/mach/common.h @@ -34,6 +34,7 @@ struct sys_timer; extern void omap_map_common_io(void); extern struct sys_timer omap_timer; extern void omap_serial_init(void); +extern void omap_serial_enable_clocks(int enable); #ifdef CONFIG_I2C_OMAP extern int omap_register_i2c_bus(int bus_id, u32 clkrate, struct i2c_board_info const *info, diff --git a/arch/arm/plat-omap/include/mach/serial.h b/arch/arm/plat-omap/include/mach/serial.h index cc6bfa5..abea6ef 100644 --- a/arch/arm/plat-omap/include/mach/serial.h +++ b/arch/arm/plat-omap/include/mach/serial.h @@ -20,11 +20,17 @@ #define OMAP_UART1_BASE 0x4806a000 #define OMAP_UART2_BASE 0x4806c000 #define OMAP_UART3_BASE 0x4806e000 +#elif defined(CONFIG_ARCH_OMAP3) +/* OMAP3 serial ports */ +#define OMAP_UART1_BASE 0x4806a000 +#define OMAP_UART2_BASE 0x4806c000 +#define OMAP_UART3_BASE 0x49020000 #endif #define OMAP_MAX_NR_PORTS 3 #define OMAP1510_BASE_BAUD (12000000/16) #define OMAP16XX_BASE_BAUD (48000000/16) +#define OMAP24XX_BASE_BAUD (48000000/16) #define is_omap_port(p) ({int __ret = 0; \ if (p == IO_ADDRESS(OMAP_UART1_BASE) || \ ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-08-23 22:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-06-07 1:47 [PATCH 00/16] Omap2 patches for post 2.6.26 Tony Lindgren 2008-06-07 1:47 ` Tony Lindgren 2008-06-07 1:47 ` [PATCH 01/16] ARM: OMAP2 Provide function to enable/disable uart clocks Tony Lindgren 2008-08-19 16:40 ` Russell King - ARM Linux 2008-08-20 5:49 ` Högander Jouni 2008-08-20 7:28 ` Tony Lindgren 2008-08-23 22:21 ` Tony Lindgren
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.