[PATCH] pch_uart: Change default UART clock setting 192MHz

[PATCH] pch_uart: Change default UART clock setting 192MHz

[Tomoya MORINAGA]

Currently, PCH_UART uses 1843200Hz as default clock.
However, in case of using high baud rate, users need to modify
clock setting.

This patch uses 192MHz setting as default UART clock setting.
Using this clock, users can use almost high baud rate without modifying
clock settings.

This setting is the same as quirk for CM-iTC board.
So, delete the quirk.

Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
---
Related patch is 
http://marc.info/?l=linux-kernel&m=132979974907774&w=2
---
 drivers/tty/serial/pch_uart.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..d068c34 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -203,7 +203,7 @@ enum {
 
 #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
 
-#define DEFAULT_BAUD_RATE 1843200 /* 1.8432MHz */
+#define DEFAULT_UART_CLOCK 192000000 /* 192.0MHz */
 
 struct pch_uart_buffer {
 	unsigned char *buf;
@@ -287,6 +287,7 @@ static struct pch_uart_driver_data drv_dat[] = {
 static struct eg20t_port *pch_uart_ports[PCH_UART_NR];
 #endif
 static unsigned int default_baud = 9600;
+static unsigned int clock_param;
 static const int trigger_level_256[4] = { 1, 64, 128, 224 };
 static const int trigger_level_64[4] = { 1, 16, 32, 56 };
 static const int trigger_level_16[4] = { 1, 4, 8, 14 };
@@ -1507,7 +1508,7 @@ static int __init pch_console_setup(struct console *co, char *options)
 		return -ENODEV;
 
 	/* setup uartclock */
-	port->uartclk = DEFAULT_BAUD_RATE;
+	port->uartclk = clock_param ? clock_param : DEFAULT_UART_CLOCK;
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
@@ -1553,7 +1554,6 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	int fifosize, base_baud;
 	int port_type;
 	struct pch_uart_driver_data *board;
-	const char *board_name;
 
 	board = &drv_dat[id->driver_data];
 	port_type = board->port_type;
@@ -1566,12 +1566,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	if (!rxbuf)
 		goto init_port_free_txbuf;
 
-	base_baud = DEFAULT_BAUD_RATE;
-
-	/* quirk for CM-iTC board */
-	board_name = dmi_get_system_info(DMI_BOARD_NAME);
-	if (board_name && strstr(board_name, "CM-iTC"))
-		base_baud = 192000000; /* 192.0MHz */
+	/* The module parameter overrides default. */
+	base_baud = clock_param ? clock_param : DEFAULT_UART_CLOCK;
 
 	switch (port_type) {
 	case PORT_UNKNOWN:
@@ -1785,3 +1781,4 @@ module_exit(pch_uart_module_exit);
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Intel EG20T PCH UART PCI Driver");
 module_param(default_baud, uint, S_IRUGO);
+module_param(clock_param, uint, (S_IRUSR | S_IWUSR));
-- 
1.7.7.6

Re: [PATCH] pch_uart: Change default UART clock setting 192MHz

[Darren Hart]

On 02/20/2012 08:55 PM, Tomoya MORINAGA wrote:
> Currently, PCH_UART uses 1843200Hz as default clock.
> However, in case of using high baud rate, users need to modify
> clock setting.
> 
> This patch uses 192MHz setting as default UART clock setting.
> Using this clock, users can use almost high baud rate without modifying
> clock settings.
> 
> This setting is the same as quirk for CM-iTC board.
> So, delete the quirk.

This also adds my module parameter "clock_param" to the patch. This
should be added separately. My current version renames this to
user_uartclk. I can send this one separately.

I'll test this today to ensure it works for my development board. I'm
concerned that it won't work for early serial console and I'll still
need to modify the BAUD on the kernel command line to compensate for the
delta between the firmware clock setting (64MHz) and the phub setting to
192MHz. When does the phub driver update the clock registers?

--
Darren


> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
> ---
> Related patch is 
> http://marc.info/?l=linux-kernel&m=132979974907774&w=2
> ---
>  drivers/tty/serial/pch_uart.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
> index 17ae657..d068c34 100644
> --- a/drivers/tty/serial/pch_uart.c
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -203,7 +203,7 @@ enum {
>  
>  #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
>  
> -#define DEFAULT_BAUD_RATE 1843200 /* 1.8432MHz */
> +#define DEFAULT_UART_CLOCK 192000000 /* 192.0MHz */
>  
>  struct pch_uart_buffer {
>  	unsigned char *buf;
> @@ -287,6 +287,7 @@ static struct pch_uart_driver_data drv_dat[] = {
>  static struct eg20t_port *pch_uart_ports[PCH_UART_NR];
>  #endif
>  static unsigned int default_baud = 9600;
> +static unsigned int clock_param;
>  static const int trigger_level_256[4] = { 1, 64, 128, 224 };
>  static const int trigger_level_64[4] = { 1, 16, 32, 56 };
>  static const int trigger_level_16[4] = { 1, 4, 8, 14 };
> @@ -1507,7 +1508,7 @@ static int __init pch_console_setup(struct console *co, char *options)
>  		return -ENODEV;
>  
>  	/* setup uartclock */
> -	port->uartclk = DEFAULT_BAUD_RATE;
> +	port->uartclk = clock_param ? clock_param : DEFAULT_UART_CLOCK;
>  
>  	if (options)
>  		uart_parse_options(options, &baud, &parity, &bits, &flow);
> @@ -1553,7 +1554,6 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>  	int fifosize, base_baud;
>  	int port_type;
>  	struct pch_uart_driver_data *board;
> -	const char *board_name;
>  
>  	board = &drv_dat[id->driver_data];
>  	port_type = board->port_type;
> @@ -1566,12 +1566,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>  	if (!rxbuf)
>  		goto init_port_free_txbuf;
>  
> -	base_baud = DEFAULT_BAUD_RATE;
> -
> -	/* quirk for CM-iTC board */
> -	board_name = dmi_get_system_info(DMI_BOARD_NAME);
> -	if (board_name && strstr(board_name, "CM-iTC"))
> -		base_baud = 192000000; /* 192.0MHz */
> +	/* The module parameter overrides default. */
> +	base_baud = clock_param ? clock_param : DEFAULT_UART_CLOCK;
>  
>  	switch (port_type) {
>  	case PORT_UNKNOWN:
> @@ -1785,3 +1781,4 @@ module_exit(pch_uart_module_exit);
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Intel EG20T PCH UART PCI Driver");
>  module_param(default_baud, uint, S_IRUGO);
> +module_param(clock_param, uint, (S_IRUSR | S_IWUSR));

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH] pch_uart: Change default UART clock setting 192MHz

[Darren Hart]

On 02/20/2012 08:55 PM, Tomoya MORINAGA wrote:
> Currently, PCH_UART uses 1843200Hz as default clock.
> However, in case of using high baud rate, users need to modify
> clock setting.
> 
> This patch uses 192MHz setting as default UART clock setting.
> Using this clock, users can use almost high baud rate without modifying
> clock settings.
> 
> This setting is the same as quirk for CM-iTC board.
> So, delete the quirk.

With this change, the serial console will work, but only after the
device is initialized. We get garbage out of the early serial console
(as it still uses the 115200 as the BASE_BAUD and the firmware in my
case has set this to 64000000/16):

xĀx�x��x�x��xx�x��xxx�xxx�x��x�x�xxxx���xxx�x�x��x�x�x�����x��xx���x�x�x�xx�x��x�x6
ahci 0000:03:06.0: AHCI 0001.0100 32 slots 2 ports 3 Gbps 0x3 impl SATA mode
ahci 0000:03:06.0: flags: ncq sntf pm led clo only pmp pio slum part ccc
scsi0 : ahci
scsi1 : ahci
...

Perhaps a reasonable solution would be to add a KCONFIG option for the
early_console BASE_BAUD. I prefer the fixed 192MHz solution here to
special casing every EG20T PCH board.

I'll prepare and test a patch for early_serial_console BASE_BAUD.

Please resend these patches with my clock_param changes removed as they
shouldn't be necessary now that we set all boards to 192MHz.

--
Darren

> 
> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
> ---
> Related patch is 
> http://marc.info/?l=linux-kernel&m=132979974907774&w=2
> ---
>  drivers/tty/serial/pch_uart.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
> index 17ae657..d068c34 100644
> --- a/drivers/tty/serial/pch_uart.c
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -203,7 +203,7 @@ enum {
>  
>  #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
>  
> -#define DEFAULT_BAUD_RATE 1843200 /* 1.8432MHz */
> +#define DEFAULT_UART_CLOCK 192000000 /* 192.0MHz */
>  
>  struct pch_uart_buffer {
>  	unsigned char *buf;
> @@ -287,6 +287,7 @@ static struct pch_uart_driver_data drv_dat[] = {
>  static struct eg20t_port *pch_uart_ports[PCH_UART_NR];
>  #endif
>  static unsigned int default_baud = 9600;
> +static unsigned int clock_param;
>  static const int trigger_level_256[4] = { 1, 64, 128, 224 };
>  static const int trigger_level_64[4] = { 1, 16, 32, 56 };
>  static const int trigger_level_16[4] = { 1, 4, 8, 14 };
> @@ -1507,7 +1508,7 @@ static int __init pch_console_setup(struct console *co, char *options)
>  		return -ENODEV;
>  
>  	/* setup uartclock */
> -	port->uartclk = DEFAULT_BAUD_RATE;
> +	port->uartclk = clock_param ? clock_param : DEFAULT_UART_CLOCK;
>  
>  	if (options)
>  		uart_parse_options(options, &baud, &parity, &bits, &flow);
> @@ -1553,7 +1554,6 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>  	int fifosize, base_baud;
>  	int port_type;
>  	struct pch_uart_driver_data *board;
> -	const char *board_name;
>  
>  	board = &drv_dat[id->driver_data];
>  	port_type = board->port_type;
> @@ -1566,12 +1566,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>  	if (!rxbuf)
>  		goto init_port_free_txbuf;
>  
> -	base_baud = DEFAULT_BAUD_RATE;
> -
> -	/* quirk for CM-iTC board */
> -	board_name = dmi_get_system_info(DMI_BOARD_NAME);
> -	if (board_name && strstr(board_name, "CM-iTC"))
> -		base_baud = 192000000; /* 192.0MHz */
> +	/* The module parameter overrides default. */
> +	base_baud = clock_param ? clock_param : DEFAULT_UART_CLOCK;
>  
>  	switch (port_type) {
>  	case PORT_UNKNOWN:
> @@ -1785,3 +1781,4 @@ module_exit(pch_uart_module_exit);
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Intel EG20T PCH UART PCI Driver");
>  module_param(default_baud, uint, S_IRUGO);
> +module_param(clock_param, uint, (S_IRUSR | S_IWUSR));

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH] pch_uart: Change default UART clock setting 192MHz

[Darren Hart]

On 02/20/2012 08:55 PM, Tomoya MORINAGA wrote:
> Currently, PCH_UART uses 1843200Hz as default clock.
> However, in case of using high baud rate, users need to modify
> clock setting.
> 
> This patch uses 192MHz setting as default UART clock setting.
> Using this clock, users can use almost high baud rate without modifying
> clock settings.
> 
> This setting is the same as quirk for CM-iTC board.
> So, delete the quirk.

With this change, the serial console will work, but only after the
device is initialized. We get garbage out of the early serial console
(as it still uses the 115200 as the BASE_BAUD and the firmware in my
case has set this to 64000000/16):

xĀx�x��x�x��xx�x��xxx�xxx�x��x�x�xxxx���xxx�x�x��x�x�x�����x��xx���x�x�x�xx�x��x�x6
ahci 0000:03:06.0: AHCI 0001.0100 32 slots 2 ports 3 Gbps 0x3 impl SATA mode
ahci 0000:03:06.0: flags: ncq sntf pm led clo only pmp pio slum part ccc
scsi0 : ahci
scsi1 : ahci
...

Perhaps a reasonable solution would be to add a KCONFIG option for the
early_console BASE_BAUD. I prefer the fixed 192MHz solution here to
special casing every EG20T PCH board.

I'll prepare and test a patch for early_serial_console BASE_BAUD.

Please resend these patches with my clock_param changes removed as they
shouldn't be necessary now that we set all boards to 192MHz.

--
Darren

> 
> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
> ---
> Related patch is 
> http://marc.info/?l=linux-kernel&m=132979974907774&w=2
> ---
>  drivers/tty/serial/pch_uart.c |   15 ++++++---------
>  1 files changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
> index 17ae657..d068c34 100644
> --- a/drivers/tty/serial/pch_uart.c
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -203,7 +203,7 @@ enum {
>  
>  #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
>  
> -#define DEFAULT_BAUD_RATE 1843200 /* 1.8432MHz */
> +#define DEFAULT_UART_CLOCK 192000000 /* 192.0MHz */
>  
>  struct pch_uart_buffer {
>  	unsigned char *buf;
> @@ -287,6 +287,7 @@ static struct pch_uart_driver_data drv_dat[] = {
>  static struct eg20t_port *pch_uart_ports[PCH_UART_NR];
>  #endif
>  static unsigned int default_baud = 9600;
> +static unsigned int clock_param;
>  static const int trigger_level_256[4] = { 1, 64, 128, 224 };
>  static const int trigger_level_64[4] = { 1, 16, 32, 56 };
>  static const int trigger_level_16[4] = { 1, 4, 8, 14 };
> @@ -1507,7 +1508,7 @@ static int __init pch_console_setup(struct console *co, char *options)
>  		return -ENODEV;
>  
>  	/* setup uartclock */
> -	port->uartclk = DEFAULT_BAUD_RATE;
> +	port->uartclk = clock_param ? clock_param : DEFAULT_UART_CLOCK;
>  
>  	if (options)
>  		uart_parse_options(options, &baud, &parity, &bits, &flow);
> @@ -1553,7 +1554,6 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>  	int fifosize, base_baud;
>  	int port_type;
>  	struct pch_uart_driver_data *board;
> -	const char *board_name;
>  
>  	board = &drv_dat[id->driver_data];
>  	port_type = board->port_type;
> @@ -1566,12 +1566,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>  	if (!rxbuf)
>  		goto init_port_free_txbuf;
>  
> -	base_baud = DEFAULT_BAUD_RATE;
> -
> -	/* quirk for CM-iTC board */
> -	board_name = dmi_get_system_info(DMI_BOARD_NAME);
> -	if (board_name && strstr(board_name, "CM-iTC"))
> -		base_baud = 192000000; /* 192.0MHz */
> +	/* The module parameter overrides default. */
> +	base_baud = clock_param ? clock_param : DEFAULT_UART_CLOCK;
>  
>  	switch (port_type) {
>  	case PORT_UNKNOWN:
> @@ -1785,3 +1781,4 @@ module_exit(pch_uart_module_exit);
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Intel EG20T PCH UART PCI Driver");
>  module_param(default_baud, uint, S_IRUGO);
> +module_param(clock_param, uint, (S_IRUSR | S_IWUSR));

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] pch_uart: Change default UART clock setting 192MHz

[Tomoya MORINAGA]

2012年2月22日1:07 Darren Hart <dvhart@linux.intel.com>:
>  When does the phub driver update the clock registers?

I've already posted pch_phub patch for 192MHz setting.
You can see the patch form below.
>> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
>> ---
>> Related patch is
>> http://marc.info/?l=linux-kernel&m=132979974907774&w=2

---
ROHM Co., Ltd.
tomoya

Re: [PATCH] pch_uart: Change default UART clock setting 192MHz

[Darren Hart]

On 02/21/2012 03:43 PM, Tomoya MORINAGA wrote:
> 2012年2月22日1:07 Darren Hart <dvhart@linux.intel.com>:
>>  When does the phub driver update the clock registers?
> 
> I've already posted pch_phub patch for 192MHz setting.
> You can see the patch form below.

Yes, I applied the patch. The problem seems to be this:

DVHART: parse_options (8250_early.c)
DVHART: pch_console_setup                                                           
DVHART: pch_console_setup                                                           
DVHART: pch_phub_probe: set CLKCFG UART to 192MHz      

As you can see, the pch_phub_probe happens much too late.

I can get it to work with a boot command line like this:
earlycon=uart8250,io,0x2050,115200n8 console=ttyPCH1,115200n8

And hacking all the BASE_BAUD references to be 48000000
(my current hardware sets the clock to 48MHz in firmware).
This gets things working until pch_phub gets around to setting
the CLKCFG register for the UART clock.

I'd prefer to not have to use the earlycon parameter/code,
but we need a way for the pch_uart to understand the difference
between early boot and post-phub setup. Can we read the CLKCFG
register in pch_console_setup to dynamically configure the
port->uartclk? (not sure that's even the right place to do it).

--
Darren

>>> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
>>> ---
>>> Related patch is
>>> http://marc.info/?l=linux-kernel&m=132979974907774&w=2
> 
> ---
> ROHM Co., Ltd.
> tomoya

-- 
Darren Hart
Intel Open Source Technology Center
Yocto Project - Linux Kernel

Re: [PATCH] pch_uart: Change default UART clock setting 192MHz

[Tomoya MORINAGA]

2012年2月22日9:20 Darren Hart <dvhart@linux.intel.com>:
> Can we read the CLKCFG
> register in pch_console_setup to dynamically configure the
> port->uartclk? (not sure that's even the right place to do it).

In general implementation, as long as pch_phub is not installed,
nobody accesses the CLKCFG register.
So I think it cannot do.

Beyond general implementation,  you would access the register.

thanks
---
ROHM Co., Ltd.
tomoy