2012-02-17 01:04:26

by Darren Hart

[permalink] [raw]
Subject: pch_uart and pch_phub clock selection

I'm working on a tunnel creek (atom e6xx + topcliff PCH) development platform
that uses a 48MHz or 64MHz clock to drive the pch_uart. I've found that if I
force the uart_clock to 48MHz (or 64MHz on the latest rev) I can get the kernel
messages and getty on the serial port.

I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
This is done in pch_uart.c code, but there is some register manipulation done in
the pch_phub.c driver and I don't understand the connection. How are the two
related?

Is the pch_phub.c register manipulation required for proper related? It seems to
work without touching those registers if I just force the clock.

The device I'm working with is EFI, and the dmi_get_system_info(DMI_BOARD_NAME)
returns "(null)", so I can't use the same test to identify this board. Is there
another common mechanism I might be able to use?

The following patch (by way of example, not meant for inclusion) gets things
working for this particular board with this command line:

console=ttyPCH1,115200 pch_uart.clock_param=48000000

I believe the right thing to do here is to discover a way to identify the board
and special case the clock as is done for the CM-iTC, but I would like to
understand the purpose of the pch_phub register manipulation code. Does this
make sense?

Thanks,

Darren


>From f83fa6cb575844d8e37f136890fe32258eb88dd2 Mon Sep 17 00:00:00 2001
Message-Id: <f83fa6cb575844d8e37f136890fe32258eb88dd2.1329439822.git.dvhart@linux.intel.com>
From: Darren Hart <[email protected]>
Date: Wed, 15 Feb 2012 15:44:18 -0800
Subject: [PATCH] pch_uart: Add clock parameter

Allow for the specification of the clock as a module parameter. This is useful
when a board uses a non-standard clock, or when different versions of a board
use different clocks, and that board name or the version are not available to
the kernel.

Signed-off-by: Darren Hart <[email protected]>
---
drivers/tty/serial/pch_uart.c | 29 +++++++++++++++++------------
1 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..8f192b9 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 1843200 /* 1.8432MHz */

struct pch_uart_buffer {
unsigned char *buf;
@@ -218,7 +218,7 @@ struct eg20t_port {
unsigned int iobase;
struct pci_dev *pdev;
int fifo_size;
- int base_baud;
+ int uart_clock;
int start_tx;
int start_rx;
int tx_empty;
@@ -287,13 +287,14 @@ 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 = 0;
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 };
static const int trigger_level_1[4] = { 1, 1, 1, 1 };

static void pch_uart_hal_request(struct pci_dev *pdev, int fifosize,
- int base_baud)
+ int uart_clock)
{
struct eg20t_port *priv = pci_get_drvdata(pdev);

@@ -332,7 +333,7 @@ static int pch_uart_hal_set_line(struct eg20t_port *priv, int baud,
unsigned int dll, dlm, lcr;
int div;

- div = DIV_ROUND_CLOSEST(priv->base_baud / 16, baud);
+ div = DIV_ROUND_CLOSEST(priv->uart_clock / 16, baud);
if (div < 0 || USHRT_MAX <= div) {
dev_err(priv->port.dev, "Invalid Baud(div=0x%x)\n", div);
return -EINVAL;
@@ -1153,9 +1154,9 @@ static int pch_uart_startup(struct uart_port *port)
priv->tx_empty = 1;

if (port->uartclk)
- priv->base_baud = port->uartclk;
+ priv->uart_clock = port->uartclk;
else
- port->uartclk = priv->base_baud;
+ port->uartclk = priv->uart_clock;

pch_uart_hal_disable_interrupt(priv, PCH_UART_HAL_ALL_INT);
ret = pch_uart_hal_set_line(priv, default_baud,
@@ -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);
@@ -1550,7 +1551,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
unsigned int iobase;
unsigned int mapbase;
unsigned char *rxbuf;
- int fifosize, base_baud;
+ int fifosize, uart_clock;
int port_type;
struct pch_uart_driver_data *board;
const char *board_name;
@@ -1566,12 +1567,15 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
if (!rxbuf)
goto init_port_free_txbuf;

- base_baud = DEFAULT_BAUD_RATE;
+ uart_clock = DEFAULT_UART_CLOCK;

/* 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 */
+ uart_clock = 192000000; /* 192.0MHz */
+
+ /* The module parameter overrides default and system quirks. */
+ uart_clock = clock_param ? clock_param : uart_clock;

switch (port_type) {
case PORT_UNKNOWN:
@@ -1597,7 +1601,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
priv->rxbuf.size = PAGE_SIZE;

priv->fifo_size = fifosize;
- priv->base_baud = base_baud;
+ priv->uart_clock = uart_clock;
priv->port_type = PORT_MAX_8250 + port_type + 1;
priv->port.dev = &pdev->dev;
priv->port.iobase = iobase;
@@ -1614,7 +1618,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
spin_lock_init(&priv->port.lock);

pci_set_drvdata(pdev, priv);
- pch_uart_hal_request(pdev, fifosize, base_baud);
+ pch_uart_hal_request(pdev, fifosize, uart_clock);

#ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
pch_uart_ports[board->line_no] = priv;
@@ -1785,3 +1789,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_IRUGO);
--
1.7.6.5


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


2012-02-17 01:35:30

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

On Thu, Feb 16, 2012 at 04:57:59PM -0800, Darren Hart wrote:
> I'm working on a tunnel creek (atom e6xx + topcliff PCH) development platform
> that uses a 48MHz or 64MHz clock to drive the pch_uart. I've found that if I
> force the uart_clock to 48MHz (or 64MHz on the latest rev) I can get the kernel
> messages and getty on the serial port.
>
> I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
> This is done in pch_uart.c code, but there is some register manipulation done in
> the pch_phub.c driver and I don't understand the connection. How are the two
> related?
>
> Is the pch_phub.c register manipulation required for proper related? It seems to
> work without touching those registers if I just force the clock.
>
> The device I'm working with is EFI, and the dmi_get_system_info(DMI_BOARD_NAME)
> returns "(null)", so I can't use the same test to identify this board. Is there
> another common mechanism I might be able to use?

There's no relevant DMI information for the board at all? What does:
grep . /sys/class/dmi/id/*
show?

>
> The following patch (by way of example, not meant for inclusion) gets things
> working for this particular board with this command line:
>
> console=ttyPCH1,115200 pch_uart.clock_param=48000000
>
> I believe the right thing to do here is to discover a way to identify the board
> and special case the clock as is done for the CM-iTC, but I would like to
> understand the purpose of the pch_phub register manipulation code. Does this
> make sense?
>
> Thanks,
>
> Darren
>
>
> >From f83fa6cb575844d8e37f136890fe32258eb88dd2 Mon Sep 17 00:00:00 2001
> Message-Id: <f83fa6cb575844d8e37f136890fe32258eb88dd2.1329439822.git.dvhart@linux.intel.com>
> From: Darren Hart <[email protected]>
> Date: Wed, 15 Feb 2012 15:44:18 -0800
> Subject: [PATCH] pch_uart: Add clock parameter
>
> Allow for the specification of the clock as a module parameter. This is useful
> when a board uses a non-standard clock, or when different versions of a board
> use different clocks, and that board name or the version are not available to
> the kernel.

You also rename base_baud to uart_clock here, so you might want to
mention it in the changelog entry :)

greg k-h

2012-02-17 07:29:21

by Feng Tang

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

Hi Darren,

On Thu, Feb 16, 2012 at 04:57:59PM -0800, Darren Hart wrote:
> I'm working on a tunnel creek (atom e6xx + topcliff PCH) development platform
> that uses a 48MHz or 64MHz clock to drive the pch_uart. I've found that if I
> force the uart_clock to 48MHz (or 64MHz on the latest rev) I can get the kernel
> messages and getty on the serial port.
>
> I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
> This is done in pch_uart.c code, but there is some register manipulation done in
> the pch_phub.c driver and I don't understand the connection. How are the two
> related?

I've met similar problem about the base clock rate for pch_uart on EG20T/ML7213,
which is controller by the phub clock configuration register.

My thought is to use a unified base clock rate 192MHz for all pch_uart devices
which could be found on EG20T/ML7213/ML72xx chipsets, so that we don't need to
worry about the different clock rate on all kinds of boards, nor do we need
the CM-iTC quirk any more. I've tested the below patch on one EG20T board and
one ML7213 board, and both work fine with it.

Adding Tomoya for his comments as he is most familiar with pch_uart driver.

Tomoya, do you know if we can also set it to 192MHz for ML7223 IOH Bus-m/n?

Thanks,
Feng

----------------------------
>From 091952dc27d3bf76e28b2afbc2fb36f0d6caada1 Mon Sep 17 00:00:00 2001
From: Feng Tang <[email protected]>
Date: Fri, 17 Feb 2012 13:59:43 +0800
Subject: [PATCH] pch_phub: unify the inital UART clock rate for EG20T and ML7213

Set the base clk rate to 192MHz, so that the pch_uart driver can
have the unique uartclk value, and don't need to add quirks for
different boards and configurations.

Signed-off-by: Feng Tang <[email protected]>
---
drivers/misc/pch_phub.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/pch_phub.c b/drivers/misc/pch_phub.c
index 10fc478..bbe4db9 100644
--- a/drivers/misc/pch_phub.c
+++ b/drivers/misc/pch_phub.c
@@ -731,10 +731,8 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
CLKCFG_CAN_50MHZ,
CLKCFG_CANCLK_MASK);

- /* quirk for CM-iTC board */
- board_name = dmi_get_system_info(DMI_BOARD_NAME);
- if (board_name && strstr(board_name, "CM-iTC"))
- pch_phub_read_modify_write_reg(chip,
+ /* Set the UART base clk rate to 192MHz */
+ pch_phub_read_modify_write_reg(chip,
(unsigned int)CLKCFG_REG_OFFSET,
CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
@@ -747,6 +745,14 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
chip->pch_opt_rom_start_address = PCH_PHUB_ROM_START_ADDR_EG20T;
chip->pch_mac_start_address = PCH_PHUB_MAC_START_ADDR_EG20T;
} else if (id->driver_data == 2) { /* ML7213 IOH */
+
+ /* Set the UART base clk rate to 192MHz */
+ pch_phub_read_modify_write_reg(chip,
+ (unsigned int)CLKCFG_REG_OFFSET,
+ CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
+ CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
+ CLKCFG_UART_MASK);
+
retval = sysfs_create_bin_file(&pdev->dev.kobj, &pch_bin_attr);
if (retval)
goto err_sysfs_create;
@@ -804,6 +810,8 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
chip->pch_mac_start_address = PCH_PHUB_MAC_START_ADDR_EG20T;
}

+ pr_info("PHUB controller: CLKCFG register is set to 0x%08x\n",
+ ioread32(chip->pch_phub_base_address + CLKCFG_REG_OFFSET));
chip->ioh_type = id->driver_data;
pci_set_drvdata(pdev, chip);

--
1.7.1


>
> Is the pch_phub.c register manipulation required for proper related? It seems to
> work without touching those registers if I just force the clock.
>
> The device I'm working with is EFI, and the dmi_get_system_info(DMI_BOARD_NAME)
> returns "(null)", so I can't use the same test to identify this board. Is there
> another common mechanism I might be able to use?
>
> The following patch (by way of example, not meant for inclusion) gets things
> working for this particular board with this command line:
>
> console=ttyPCH1,115200 pch_uart.clock_param=48000000
>
> I believe the right thing to do here is to discover a way to identify the board
> and special case the clock as is done for the CM-iTC, but I would like to
> understand the purpose of the pch_phub register manipulation code. Does this
> make sense?
>
> Thanks,
>
> Darren

2012-02-17 09:50:36

by Tomoya MORINAGA

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

Hi

2012$BG/(B2$B7n(B17$BF|(B16:28 Feng Tang <[email protected]>:
> I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
> This is done in pch_uart.c code, but there is some register manipulation done in
> the pch_phub.c driver and I don't understand the connection. How are the two
> related?
According to your use, need to configure clock registers which are in pch_phub .
Upstreamed version, UART_CLK can be used directly(neither multiple nor
division) as UART clock.

You can get clock configuration information from SourceForge.
(http://sourceforge.net/projects/ml7213/files/Kernel%202.6.37/Release/Ver1.2.0/EG20TPCH_ML7213_ML7223_ML7831_linux-2.6.37_v120_20110930.tar.bz2/
and extract pch_phub. you can find readme.)
I extract it and show below.

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<Configuration>
=======================

1. Over 115K baud rate UART settings
By default, UART can communicate less than 115Kbps.
In case you want UART to work more than 115Kbps, the following
clock configuration is necessary.
- Clock setting
Set BAUDSEL = usb_48mhz
Set PLL2VCO = "x 8" the clock
Set BAUDDIV = "x 1/6" the clock
Set UARTCLKSEL = PLL2 output
For details, please refer to ML7213/ML7223 EDS "5 Chip Configuration"
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

In case we want UART to work high baud rate(e.g.4Mbps), we set like above.
and execute "setserial /dev/ttyPCH0 baud_base 4000000".
I can see PCH_UART with 4Mbps works well.

Darren, is this answer for your question ?

> Tomoya, do you know if we can also set it to 192MHz for ML7223 IOH Bus-m/n?
Yes, you can.

--
ROHM Co., Ltd.
tomoya

2012-02-17 18:21:47

by Darren Hart

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

On 02/17/2012 01:50 AM, Tomoya MORINAGA wrote:
> Hi
>
> 2012$BG/(B2$B7n(B17$BF|(B16:28 Feng Tang <[email protected]>:
>> I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
>> This is done in pch_uart.c code, but there is some register manipulation done in
>> the pch_phub.c driver and I don't understand the connection. How are the two
>> related?
> According to your use, need to configure clock registers which are in pch_phub .
> Upstreamed version, UART_CLK can be used directly(neither multiple nor
> division) as UART clock.

I'm not following. I think you are saying that I need to configure the
clock registers - but in my patch I don't touch them and it works. Are
the registers intended to be read so I can determine HOW the device is
configured, are they intended to be written in order to change how it is
configured, or both.

Firmware sets up some initial state and this has been what I'm trying to
match in order to get an early serial console.

>
> You can get clock configuration information from SourceForge.
> (http://sourceforge.net/projects/ml7213/files/Kernel%202.6.37/Release/Ver1.2.0/EG20TPCH_ML7213_ML7223_ML7831_linux-2.6.37_v120_20110930.tar.bz2/
> and extract pch_phub. you can find readme.)
> I extract it and show below.
>
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> <Configuration>
> =======================
>
> 1. Over 115K baud rate UART settings
> By default, UART can communicate less than 115Kbps.
> In case you want UART to work more than 115Kbps, the following
> clock configuration is necessary.
> - Clock setting
> Set BAUDSEL = usb_48mhz
> Set PLL2VCO = "x 8" the clock
> Set BAUDDIV = "x 1/6" the clock
> Set UARTCLKSEL = PLL2 output
> For details, please refer to ML7213/ML7223 EDS "5 Chip Configuration"
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> In case we want UART to work high baud rate(e.g.4Mbps), we set like above.
> and execute "setserial /dev/ttyPCH0 baud_base 4000000".
> I can see PCH_UART with 4Mbps works well.
>
> Darren, is this answer for your question ?

Not quite. I need to be able to use the UART at boot as an early serial
console. So I believe I need to match the firmware UART configuration
early on (well before we could call setserial).

>
>> Tomoya, do you know if we can also set it to 192MHz for ML7223 IOH Bus-m/n?
> Yes, you can.
>

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

2012-02-17 18:21:46

by Darren Hart

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection



On 02/16/2012 05:30 PM, Greg Kroah-Hartman wrote:
> On Thu, Feb 16, 2012 at 04:57:59PM -0800, Darren Hart wrote:
>> I'm working on a tunnel creek (atom e6xx + topcliff PCH) development platform
>> that uses a 48MHz or 64MHz clock to drive the pch_uart. I've found that if I
>> force the uart_clock to 48MHz (or 64MHz on the latest rev) I can get the kernel
>> messages and getty on the serial port.
>>
>> I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
>> This is done in pch_uart.c code, but there is some register manipulation done in
>> the pch_phub.c driver and I don't understand the connection. How are the two
>> related?
>>
>> Is the pch_phub.c register manipulation required for proper related? It seems to
>> work without touching those registers if I just force the clock.
>>
>> The device I'm working with is EFI, and the dmi_get_system_info(DMI_BOARD_NAME)
>> returns "(null)", so I can't use the same test to identify this board. Is there
>> another common mechanism I might be able to use?
>
> There's no relevant DMI information for the board at all? What does:
> grep . /sys/class/dmi/id/*
> show?

Duh.

# cat /sys/class/dmi/id/product_name
Fish River Island II


>
>>
>> The following patch (by way of example, not meant for inclusion) gets things
>> working for this particular board with this command line:
>>
>> console=ttyPCH1,115200 pch_uart.clock_param=48000000
>>
>> I believe the right thing to do here is to discover a way to identify the board
>> and special case the clock as is done for the CM-iTC, but I would like to
>> understand the purpose of the pch_phub register manipulation code. Does this
>> make sense?
>>
>> Thanks,
>>
>> Darren
>>
>>
>> >From f83fa6cb575844d8e37f136890fe32258eb88dd2 Mon Sep 17 00:00:00 2001
>> Message-Id: <f83fa6cb575844d8e37f136890fe32258eb88dd2.1329439822.git.dvhart@linux.intel.com>
>> From: Darren Hart <[email protected]>
>> Date: Wed, 15 Feb 2012 15:44:18 -0800
>> Subject: [PATCH] pch_uart: Add clock parameter
>>
>> Allow for the specification of the clock as a module parameter. This is useful
>> when a board uses a non-standard clock, or when different versions of a board
>> use different clocks, and that board name or the version are not available to
>> the kernel.
>
> You also rename base_baud to uart_clock here, so you might want to
> mention it in the changelog entry :)
>

Right, thanks. I suspect I'll end up breaking that change out as a
semantic patch :-)

> greg k-h

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

2012-02-20 04:28:36

by Tomoya MORINAGA

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

Hi

Considering Feng's/Darren's proposal/question,
I should have set 192MHz as default uart clock setting.
So, I created the following patch. (not formal patch but for review)
Let me know your opinion.

---
drivers/misc/pch_phub.c | 17 ++++++-----------
drivers/tty/serial/pch_uart.c | 15 +++++++--------
2 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/drivers/misc/pch_phub.c b/drivers/misc/pch_phub.c
index 10fc478..8f9c1db 100644
--- a/drivers/misc/pch_phub.c
+++ b/drivers/misc/pch_phub.c
@@ -55,7 +55,7 @@
#define CLKCFG_CANCLK_MASK 0xFF000000
#define CLKCFG_UART_MASK 0xFFFFFF

-/* CM-iTC */
+/* 192MHz Clock configuration. USB_48MHz / 2 * 8 = 192 */
#define CLKCFG_UART_48MHZ (1 << 16)
#define CLKCFG_BAUDDIV (2 << 20)
#define CLKCFG_PLL2VCO (8 << 9)
@@ -715,8 +715,6 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
chip->pdev = pdev; /* Save pci device struct */

if (id->driver_data == 1) { /* EG20T PCH */
- const char *board_name;
-
retval = sysfs_create_file(&pdev->dev.kobj,
&dev_attr_pch_mac.attr);
if (retval)
@@ -731,14 +729,11 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
CLKCFG_CAN_50MHZ,
CLKCFG_CANCLK_MASK);

- /* quirk for CM-iTC board */
- board_name = dmi_get_system_info(DMI_BOARD_NAME);
- if (board_name && strstr(board_name, "CM-iTC"))
- pch_phub_read_modify_write_reg(chip,
- (unsigned int)CLKCFG_REG_OFFSET,
- CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
- CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
- CLKCFG_UART_MASK);
+ pch_phub_read_modify_write_reg(chip,
+ (unsigned int)CLKCFG_REG_OFFSET,
+ CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
+ CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
+ CLKCFG_UART_MASK);

/* set the prefech value */
iowrite32(0x000affaa, chip->pch_phub_base_address + 0x14);
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..941f887 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 = 0;
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,10 @@ static struct eg20t_port
*pch_uart_init_port(struct pci_dev *pdev,
if (!rxbuf)
goto init_port_free_txbuf;

- base_baud = DEFAULT_BAUD_RATE;
+ base_baud = DEFAULT_UART_CLOCK;

- /* 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. */
+ uart_clock = clock_param ? clock_param : uart_clock;

switch (port_type) {
case PORT_UNKNOWN:
@@ -1785,3 +1783,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));
--

thanks.
--
ROHM Co., Ltd.
tomoya

2012$BG/(B2$B7n(B18$BF|(B3:14 Darren Hart <[email protected]>:
> On 02/17/2012 01:50 AM, Tomoya MORINAGA wrote:
>> Hi
>>
>> 2012$BG/(B2$B7n(B17$BF|(B16:28 Feng Tang <[email protected]>:
>>> I see that the the CM-iTC board is special-cased to set a 192MHz uart_clock.
>>> This is done in pch_uart.c code, but there is some register manipulation done in
>>> the pch_phub.c driver and I don't understand the connection. How are the two
>>> related?
>> According to your use, need to configure clock registers which are in pch_phub .
>> Upstreamed version, UART_CLK can be used directly(neither multiple nor
>> division) as UART clock.
>
> I'm not following. I think you are saying that I need to configure the
> clock registers - but in my patch I don't touch them and it works. Are
> the registers intended to be read so I can determine HOW the device is
> configured, are they intended to be written in order to change how it is
> configured, or both.
>
> Firmware sets up some initial state and this has been what I'm trying to
> match in order to get an early serial console.
>
>>
>> You can get clock configuration information from SourceForge.
>> (http://sourceforge.net/projects/ml7213/files/Kernel%202.6.37/Release/Ver1.2.0/EG20TPCH_ML7213_ML7223_ML7831_linux-2.6.37_v120_20110930.tar.bz2/
>> and extract pch_phub. you can find readme.)
>> I extract it and show below.
>>
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> <Configuration>
>> =======================
>>
>> 1. Over 115K baud rate UART settings
>> By default, UART can communicate less than 115Kbps.
>> In case you want UART to work more than 115Kbps, the following
>> clock configuration is necessary.
>> - Clock setting
>> Set BAUDSEL = usb_48mhz
>> Set PLL2VCO = "x 8" the clock
>> Set BAUDDIV = "x 1/6" the clock
>> Set UARTCLKSEL = PLL2 output
>> For details, please refer to ML7213/ML7223 EDS "5 Chip Configuration"
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> In case we want UART to work high baud rate(e.g.4Mbps), we set like above.
>> and execute "setserial /dev/ttyPCH0 baud_base 4000000".
>> I can see PCH_UART with 4Mbps works well.
>>
>> Darren, is this answer for your question ?
>
> Not quite. I need to be able to use the UART at boot as an early serial
> console. So I believe I need to match the firmware UART configuration
> early on (well before we could call setserial).
>
>>
>>> Tomoya, do you know if we can also set it to 192MHz for ML7223 IOH Bus-m/n?
>> Yes, you can.
>>
>
> --
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Linux Kernel

2012-02-20 04:44:30

by Feng Tang

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

Hi Tomoya,

On Mon, Feb 20, 2012 at 01:28:33PM +0900, Tomoya MORINAGA wrote:
> Hi
>
> Considering Feng's/Darren's proposal/question,
> I should have set 192MHz as default uart clock setting.
> So, I created the following patch. (not formal patch but for review)
> Let me know your opinion.
>
> ---
> drivers/misc/pch_phub.c | 17 ++++++-----------
> drivers/tty/serial/pch_uart.c | 15 +++++++--------
> 2 files changed, 13 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/misc/pch_phub.c b/drivers/misc/pch_phub.c
> index 10fc478..8f9c1db 100644
> --- a/drivers/misc/pch_phub.c
> +++ b/drivers/misc/pch_phub.c
> @@ -55,7 +55,7 @@
> #define CLKCFG_CANCLK_MASK 0xFF000000
> #define CLKCFG_UART_MASK 0xFFFFFF
>
> -/* CM-iTC */
> +/* 192MHz Clock configuration. USB_48MHz / 2 * 8 = 192 */
> #define CLKCFG_UART_48MHZ (1 << 16)
> #define CLKCFG_BAUDDIV (2 << 20)
> #define CLKCFG_PLL2VCO (8 << 9)
> @@ -715,8 +715,6 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
> chip->pdev = pdev; /* Save pci device struct */
>
> if (id->driver_data == 1) { /* EG20T PCH */
> - const char *board_name;
> -
> retval = sysfs_create_file(&pdev->dev.kobj,
> &dev_attr_pch_mac.attr);
> if (retval)
> @@ -731,14 +729,11 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
> CLKCFG_CAN_50MHZ,
> CLKCFG_CANCLK_MASK);
>
> - /* quirk for CM-iTC board */
> - board_name = dmi_get_system_info(DMI_BOARD_NAME);
> - if (board_name && strstr(board_name, "CM-iTC"))
> - pch_phub_read_modify_write_reg(chip,
> - (unsigned int)CLKCFG_REG_OFFSET,
> - CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
> - CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
> - CLKCFG_UART_MASK);
> + pch_phub_read_modify_write_reg(chip,
> + (unsigned int)CLKCFG_REG_OFFSET,
> + CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
> + CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
> + CLKCFG_UART_MASK);

All the code looks fine to me except one point: Can we also set ML7213/7223's
default clk to 192MHz? 192MHz works fine on my ML7213 board. And using an
unified default clock rate for all EG20T compatible IOHs will save extra
effort of setting the uart clock.

Thanks,
Feng

2012-02-20 05:42:34

by Tomoya MORINAGA

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

2012$BG/(B2$B7n(B20$BF|(B13:43 Feng Tang <[email protected]>:
> Can we also set ML7213/7223's
> default clk to 192MHz? 192MHz works fine on my ML7213 board. And using an
> unified default clock rate for all EG20T compatible IOHs will save extra
> effort of setting the uart clock.
>
Yes, you can also use 192MHz for both ML7213 and ML7223.
In fact, we've already confirmed 192MHz works fine. (Windows driver).
thanks,

---
ROHM Co., Ltd.
tomoya

2012-02-20 05:58:16

by Feng Tang

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

Hi Tomoya,

On Mon, Feb 20, 2012 at 02:42:32PM +0900, Tomoya MORINAGA wrote:
> 2012年2月20日13:43 Feng Tang <[email protected]>:
> > Can we also set ML7213/7223's
> > default clk to 192MHz? 192MHz works fine on my ML7213 board. And using an
> > unified default clock rate for all EG20T compatible IOHs will save extra
> > effort of setting the uart clock.
> >
> Yes, you can also use 192MHz for both ML7213 and ML7223.
> In fact, we've already confirmed 192MHz works fine. (Windows driver).
> thanks,

Great! and glad to hear that. Could you update your patch which only touches
the EG20T UART clock setting? like taking the register setting code out of
the "switch/case" loop. something like:

---------------
diff --git a/drivers/misc/pch_phub.c b/drivers/misc/pch_phub.c
index 10fc478..299524c 100644
--- a/drivers/misc/pch_phub.c
+++ b/drivers/misc/pch_phub.c
@@ -55,7 +55,7 @@
#define CLKCFG_CANCLK_MASK 0xFF000000
#define CLKCFG_UART_MASK 0xFFFFFF

-/* CM-iTC */
+/* 192MHz Clock configuration. USB_48MHz / 2 * 8 = 192 */
#define CLKCFG_UART_48MHZ (1 << 16)
#define CLKCFG_BAUDDIV (2 << 20)
#define CLKCFG_PLL2VCO (8 << 9)
@@ -714,9 +714,13 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,

chip->pdev = pdev; /* Save pci device struct */

- if (id->driver_data == 1) { /* EG20T PCH */
- const char *board_name;
+ pch_phub_read_modify_write_reg(chip,
+ (unsigned int)CLKCFG_REG_OFFSET,
+ CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
+ CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
+ CLKCFG_UART_MASK);

+ if (id->driver_data == 1) { /* EG20T PCH */
retval = sysfs_create_file(&pdev->dev.kobj,
&dev_attr_pch_mac.attr);
if (retval)
@@ -731,15 +735,6 @@ static int __devinit pch_phub_probe(struct pci_dev *pdev,
CLKCFG_CAN_50MHZ,
CLKCFG_CANCLK_MASK);

- /* quirk for CM-iTC board */
- board_name = dmi_get_system_info(DMI_BOARD_NAME);
- if (board_name && strstr(board_name, "CM-iTC"))
- pch_phub_read_modify_write_reg(chip,
- (unsigned int)CLKCFG_REG_OFFSET,
- CLKCFG_UART_48MHZ | CLKCFG_BAUDDIV |
- CLKCFG_PLL2VCO | CLKCFG_UARTCLKSEL,
- CLKCFG_UART_MASK);
-
/* set the prefech value */
iowrite32(0x000affaa, chip->pch_phub_base_address + 0x14);
/* set the interrupt delay value */
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..b6ec42a 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 = 0;
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,10 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
if (!rxbuf)
goto init_port_free_txbuf;

- base_baud = DEFAULT_BAUD_RATE;
+ base_baud = DEFAULT_UART_CLOCK;

- /* 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. */
+ uart_clock = clock_param ? clock_param : uart_clock;

switch (port_type) {
case PORT_UNKNOWN:
>
> ---
> ROHM Co., Ltd.
> tomoya

2012-02-20 06:23:00

by Tomoya MORINAGA

[permalink] [raw]
Subject: Re: pch_uart and pch_phub clock selection

2012$BG/(B2$B7n(B20$BF|(B14:57 Feng Tang <[email protected]>:
> Great! and glad to hear that. Could you update your patch which only touches
> the EG20T UART clock setting? like taking the register setting code out of
> the "switch/case" loop. something like:

OK.
I will modify like you said and post the patch.

thanks.
---
ROHM Co., Ltd.
tomoya