[PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

This series does some minor clean-up to the pch_uart driver, adds support
for the Fish River Island II UART clock quirks, and introduces a user_uartclk
parameter to aid in developing for early and changing hardware.

In order to support an early serial console, we cannot rely on the pch_phub
probe function to setup the CFGCLK register. This series relies on the board
quirks and does not force the setting of the CLKREG in the pch_phub code.
Instead, it aligns with what is the default configuration (defined by firmware)
for a given board. The user_uartclk parameter provides a mechanism to force a
specific uartclk if necessary.

I looked at the PCI code briefly to see about forcing the clock to 192MHz on
all boards early on to avoid having to use quirks, but it was not clear to me
if this could be done before PCI was initialized as the IO base is derived
from pci_iomap(pdev...). Perhaps this can be done, and I would be happy to
revisit this as a follow-up patch. As this current patch series follows existing
precedent to support a specific board and it does not impact the existing boards
or the default behavior, I'd like to see this merged as is, rather than hold out
for a much more invasive change forcing the clock to 192MHz. Is this acceptable? 

V2: Incorporates Alan Cox's feedback into 2/4: Add Fish River Island II uart
    clock quirks, refactoring the clock quirks into a new function and using
    a more appropriate name for a reused string variable.
    Add support for the two firmware variants for the FRI2.

--
Darren

The following changes since commit 164974a8f2a482f1abcb027c6d1a89dd79b14297:

  ecryptfs: fix printk format warning for size_t (2012-02-28 16:55:30 -0800)

are available in the git repository at:
  git://git.infradead.org/users/dvhart/linux-2.6.git pch_uart_v2
  http://git.infradead.org/users/dvhart/linux-2.6.git/shortlog/refs/heads/pch_uart_v2

Darren Hart (4):
  pch_uart: Use uartclk instead of base_baud
  pch_uart: Add Fish River Island II uart clock quirks
  pch_uart: Add user_uartclk parameter
  pch_uart: Use existing default_baud in setup_console

 drivers/tty/serial/pch_uart.c |   59 +++++++++++++++++++++++++++--------------
 1 files changed, 39 insertions(+), 20 deletions(-)

-- 
1.7.6.5

[PATCH 1/4] pch_uart: Use uartclk instead of base_baud

[Darren Hart]

The term "base baud" refers to the fastest baud rate the device can communicate
at. This is clock/16. pch_uart is using base_baud as the clock itself. Rename
the variables to be semantically correct.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
 drivers/tty/serial/pch_uart.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..c565817 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_UARTCLK 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 uartclk;
 	int start_tx;
 	int start_rx;
 	int tx_empty;
@@ -293,7 +293,7 @@ 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 uartclk)
 {
 	struct eg20t_port *priv = pci_get_drvdata(pdev);
 
@@ -332,7 +332,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->uartclk / 16, baud);
 	if (div < 0 || USHRT_MAX <= div) {
 		dev_err(priv->port.dev, "Invalid Baud(div=0x%x)\n", div);
 		return -EINVAL;
@@ -1153,9 +1153,9 @@ static int pch_uart_startup(struct uart_port *port)
 	priv->tx_empty = 1;
 
 	if (port->uartclk)
-		priv->base_baud = port->uartclk;
+		priv->uartclk = port->uartclk;
 	else
-		port->uartclk = priv->base_baud;
+		port->uartclk = priv->uartclk;
 
 	pch_uart_hal_disable_interrupt(priv, PCH_UART_HAL_ALL_INT);
 	ret = pch_uart_hal_set_line(priv, default_baud,
@@ -1507,7 +1507,7 @@ static int __init pch_console_setup(struct console *co, char *options)
 		return -ENODEV;
 
 	/* setup uartclock */
-	port->uartclk = DEFAULT_BAUD_RATE;
+	port->uartclk = DEFAULT_UARTCLK;
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
@@ -1550,7 +1550,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, uartclk;
 	int port_type;
 	struct pch_uart_driver_data *board;
 	const char *board_name;
@@ -1566,12 +1566,12 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	if (!rxbuf)
 		goto init_port_free_txbuf;
 
-	base_baud = DEFAULT_BAUD_RATE;
+	uartclk = DEFAULT_UARTCLK;
 
 	/* 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 */
+		uartclk = 192000000; /* 192.0MHz */
 
 	switch (port_type) {
 	case PORT_UNKNOWN:
@@ -1597,7 +1597,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->uartclk = uartclk;
 	priv->port_type = PORT_MAX_8250 + port_type + 1;
 	priv->port.dev = &pdev->dev;
 	priv->port.iobase = iobase;
@@ -1614,7 +1614,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, uartclk);
 
 #ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
 	pch_uart_ports[board->line_no] = priv;
-- 
1.7.6.5

Re: [PATCH 1/4] pch_uart: Use uartclk instead of base_baud

[Greg Kroah-Hartman]

On Wed, Feb 29, 2012 at 10:24:44AM -0800, Darren Hart wrote:
> The term "base baud" refers to the fastest baud rate the device can communicate
> at. This is clock/16. pch_uart is using base_baud as the clock itself. Rename
> the variables to be semantically correct.

This patch doesn't apply to my tree, what did you make it against?

Care to refresh these against the linux-next tree and resend them so
that I can apply them?

thanks,

greg k-h

[PATCH 2/4] pch_uart: Add Fish River Island II uart clock quirks

[Darren Hart]

Add support for the Fish River Island II (FRI2) UART clock following the CM-iTC
quirk handling mechanism. Depending on the firmware installed on the device, the
FRI2 uses a 48MHz or a 64MHz UART clock. This is detected with DMI strings.

Add similar UART clock quirk handling to the pch_console_setup() function to
enable kernel messages on boards with non-standard UART clocks.

Per Alan's suggestion, abstract out UART clock selection into
pch_uart_get_uartclk() to avoid code duplication.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
 drivers/tty/serial/pch_uart.c |   42 +++++++++++++++++++++++++++-------------
 1 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index c565817..3a2b0ae 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -203,7 +203,10 @@ enum {
 
 #define BOTH_EMPTY (UART_LSR_TEMT | UART_LSR_THRE)
 
-#define DEFAULT_UARTCLK 1843200 /* 1.8432MHz */
+#define DEFAULT_UARTCLK   1843200 /*   1.8432 MHz */
+#define CMITC_UARTCLK   192000000 /* 192.0000 MHz */
+#define FRI2_64_UARTCLK  64000000 /*  64.0000 MHz */
+#define FRI2_48_UARTCLK  48000000 /*  48.0000 MHz */
 
 struct pch_uart_buffer {
 	unsigned char *buf;
@@ -292,6 +295,26 @@ 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 };
 
+/* Return UART clock, checking for board specific clocks. */
+static int pch_uart_get_uartclk(void)
+{
+	const char *cmp;
+
+	cmp = dmi_get_system_info(DMI_BOARD_NAME);
+	if (cmp && strstr(cmp, "CM-iTC"))
+		return CMITC_UARTCLK;
+
+	cmp = dmi_get_system_info(DMI_BIOS_VERSION);
+	if (cmp && strnstr(cmp, "FRI2", 4))
+		return FRI2_64_UARTCLK;
+
+	cmp = dmi_get_system_info(DMI_PRODUCT_NAME);
+	if (cmp && strstr(cmp, "Fish River Island II"))
+		return FRI2_48_UARTCLK;
+
+	return DEFAULT_UARTCLK;
+}
+
 static void pch_uart_hal_request(struct pci_dev *pdev, int fifosize,
 				 int uartclk)
 {
@@ -1506,8 +1529,7 @@ static int __init pch_console_setup(struct console *co, char *options)
 	if (!port || (!port->iobase && !port->membase))
 		return -ENODEV;
 
-	/* setup uartclock */
-	port->uartclk = DEFAULT_UARTCLK;
+	port->uartclk = pch_uart_get_uartclk();
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
@@ -1550,10 +1572,9 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	unsigned int iobase;
 	unsigned int mapbase;
 	unsigned char *rxbuf;
-	int fifosize, uartclk;
+	int fifosize;
 	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,13 +1587,6 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	if (!rxbuf)
 		goto init_port_free_txbuf;
 
-	uartclk = DEFAULT_UARTCLK;
-
-	/* quirk for CM-iTC board */
-	board_name = dmi_get_system_info(DMI_BOARD_NAME);
-	if (board_name && strstr(board_name, "CM-iTC"))
-		uartclk = 192000000; /* 192.0MHz */
-
 	switch (port_type) {
 	case PORT_UNKNOWN:
 		fifosize = 256; /* EG20T/ML7213: UART0 */
@@ -1597,7 +1611,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	priv->rxbuf.size = PAGE_SIZE;
 
 	priv->fifo_size = fifosize;
-	priv->uartclk = uartclk;
+	priv->uartclk = pch_uart_get_uartclk();
 	priv->port_type = PORT_MAX_8250 + port_type + 1;
 	priv->port.dev = &pdev->dev;
 	priv->port.iobase = iobase;
@@ -1614,7 +1628,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, uartclk);
+	pch_uart_hal_request(pdev, fifosize, priv->uartclk);
 
 #ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
 	pch_uart_ports[board->line_no] = priv;
-- 
1.7.6.5

[PATCH 3/4] pch_uart: Add user_uartclk parameter

[Darren Hart]

For cases where boards with non-default clocks are not yet added to the kernel
or when the clock varies across hardware revisions, it is useful to be
able to specify the UART clock on the kernel command line.

Add the user_uartclk parameter and prefer it, if set, to the default and
board specific UART clock settings. Specify user_uartclock on the command-line
with "pch_uart.user_uartclk=48000000".

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
 drivers/tty/serial/pch_uart.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 3a2b0ae..105d982 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -290,6 +290,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 user_uartclk = 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 };
@@ -300,6 +301,9 @@ static int pch_uart_get_uartclk(void)
 {
 	const char *cmp;
 
+	if (user_uartclk)
+		return user_uartclk;
+
 	cmp = dmi_get_system_info(DMI_BOARD_NAME);
 	if (cmp && strstr(cmp, "CM-iTC"))
 		return CMITC_UARTCLK;
@@ -1799,3 +1803,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(user_uartclk, uint, S_IRUGO);
-- 
1.7.6.5

Re: [PATCH 3/4] pch_uart: Add user_uartclk parameter

[Greg Kroah-Hartman]

On Wed, Feb 29, 2012 at 10:24:46AM -0800, Darren Hart wrote:
> For cases where boards with non-default clocks are not yet added to the kernel
> or when the clock varies across hardware revisions, it is useful to be
> able to specify the UART clock on the kernel command line.
> 
> Add the user_uartclk parameter and prefer it, if set, to the default and
> board specific UART clock settings. Specify user_uartclock on the command-line
> with "pch_uart.user_uartclk=48000000".
> 
> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
> CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
> CC: Feng Tang <feng.tang@intel.com>
> CC: Alan Cox <alan@linux.intel.com>
> ---
>  drivers/tty/serial/pch_uart.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
> index 3a2b0ae..105d982 100644
> --- a/drivers/tty/serial/pch_uart.c
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -290,6 +290,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 user_uartclk = 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 };
> @@ -300,6 +301,9 @@ static int pch_uart_get_uartclk(void)
>  {
>  	const char *cmp;
>  
> +	if (user_uartclk)
> +		return user_uartclk;
> +
>  	cmp = dmi_get_system_info(DMI_BOARD_NAME);
>  	if (cmp && strstr(cmp, "CM-iTC"))
>  		return CMITC_UARTCLK;
> @@ -1799,3 +1803,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(user_uartclk, uint, S_IRUGO);

I don't object to this, but you should provide some documentation on
what this option does.  How about a follow-on patch using
MODULE_PARM_DESC() for both of these options to help people out with how
they should be used?

thanks,

greg k-h

Re: [PATCH 3/4] pch_uart: Add user_uartclk parameter

[Darren Hart]

On 03/08/2012 10:56 AM, Greg Kroah-Hartman wrote:
> On Wed, Feb 29, 2012 at 10:24:46AM -0800, Darren Hart wrote:
>> For cases where boards with non-default clocks are not yet added to the kernel
>> or when the clock varies across hardware revisions, it is useful to be
>> able to specify the UART clock on the kernel command line.
>>
>> Add the user_uartclk parameter and prefer it, if set, to the default and
>> board specific UART clock settings. Specify user_uartclock on the command-line
>> with "pch_uart.user_uartclk=48000000".
>>
>> Signed-off-by: Darren Hart <dvhart@linux.intel.com>
>> CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
>> CC: Feng Tang <feng.tang@intel.com>
>> CC: Alan Cox <alan@linux.intel.com>
>> ---
>>  drivers/tty/serial/pch_uart.c |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
>> index 3a2b0ae..105d982 100644
>> --- a/drivers/tty/serial/pch_uart.c
>> +++ b/drivers/tty/serial/pch_uart.c
>> @@ -290,6 +290,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 user_uartclk = 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 };
>> @@ -300,6 +301,9 @@ static int pch_uart_get_uartclk(void)
>>  {
>>  	const char *cmp;
>>  
>> +	if (user_uartclk)
>> +		return user_uartclk;
>> +
>>  	cmp = dmi_get_system_info(DMI_BOARD_NAME);
>>  	if (cmp && strstr(cmp, "CM-iTC"))
>>  		return CMITC_UARTCLK;
>> @@ -1799,3 +1803,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(user_uartclk, uint, S_IRUGO);
> 
> I don't object to this, but you should provide some documentation on
> what this option does.  How about a follow-on patch using
> MODULE_PARM_DESC() for both of these options to help people out with how
> they should be used?

Will do (now I'm traveling, so might be a day). Will include with the
rebase to linux-next.

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

[PATCH 4/4] pch_uart: Use existing default_baud in setup_console

[Darren Hart]

Rather than hardcode 9600, use the existing default_baud parameter (which
also defaults to 9600).

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
 drivers/tty/serial/pch_uart.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 105d982..21af194b 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1516,7 +1516,7 @@ pch_console_write(struct console *co, const char *s, unsigned int count)
 static int __init pch_console_setup(struct console *co, char *options)
 {
 	struct uart_port *port;
-	int baud = 9600;
+	int baud = default_baud;
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';
-- 
1.7.6.5

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Tomoya MORINAGA]

Hi

Darren's patch series, I think OK.
However, need to proceed next step.

As I said before,
internal clock(USB_48MHz) should be used than external clock(UART_CLK)
as default setting.
--Reasons
  - Users don't always use 1.8432MHz as UART_CLK.
    So,  In case of using except 1.8432MHz,
    the user needs to change both pch_phub and pch_uart setting (It's
bother things).
  - High baud rate can't be used more than 115Kbps

I think there is no merit to use UART_CLK.

---Merit to use 192MHz
  - Users don't have to care both pch_phub and pch_uart clock setting.
  - High baud rate can be supported

thanks,

ROHM Co., Ltd.
tomoya

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

On 02/29/2012 04:38 PM, Tomoya MORINAGA wrote:
> Hi
> 
> Darren's patch series, I think OK.
> However, need to proceed next step.

Great, Greg are you happy with these as they are?

> 
> As I said before,
> internal clock(USB_48MHz) should be used than external clock(UART_CLK)
> as default setting.
> --Reasons
>   - Users don't always use 1.8432MHz as UART_CLK.
>     So,  In case of using except 1.8432MHz,
>     the user needs to change both pch_phub and pch_uart setting (It's
> bother things).
>   - High baud rate can't be used more than 115Kbps
> 
> I think there is no merit to use UART_CLK.
> 
> ---Merit to use 192MHz
>   - Users don't have to care both pch_phub and pch_uart clock setting.
>   - High baud rate can be supported

I have no objection to this provided the early serial console can work
without additional patches. So the question now is, how do we force the
192MHz clock setting BEFORE the pch_phub code runs?


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

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Greg Kroah-Hartman]

On Wed, Feb 29, 2012 at 04:47:58PM -0800, Darren Hart wrote:
> 
> 
> On 02/29/2012 04:38 PM, Tomoya MORINAGA wrote:
> > Hi
> > 
> > Darren's patch series, I think OK.
> > However, need to proceed next step.
> 
> Great, Greg are you happy with these as they are?

I'll look them over in the next few days and let you know.

thanks,

greg k-h

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

On 02/29/2012 04:53 PM, Greg Kroah-Hartman wrote:
> On Wed, Feb 29, 2012 at 04:47:58PM -0800, Darren Hart wrote:
>>
>>
>> On 02/29/2012 04:38 PM, Tomoya MORINAGA wrote:
>>> Hi
>>>
>>> Darren's patch series, I think OK.
>>> However, need to proceed next step.
>>
>> Great, Greg are you happy with these as they are?
> 
> I'll look them over in the next few days and let you know.
> 

Hey Greg. Any thoughts on these?


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

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Greg Kroah-Hartman]

On Wed, Mar 07, 2012 at 10:32:24AM -0800, Darren Hart wrote:
> 
> On 02/29/2012 04:53 PM, Greg Kroah-Hartman wrote:
> > On Wed, Feb 29, 2012 at 04:47:58PM -0800, Darren Hart wrote:
> >>
> >>
> >> On 02/29/2012 04:38 PM, Tomoya MORINAGA wrote:
> >>> Hi
> >>>
> >>> Darren's patch series, I think OK.
> >>> However, need to proceed next step.
> >>
> >> Great, Greg are you happy with these as they are?
> > 
> > I'll look them over in the next few days and let you know.
> > 
> 
> Hey Greg. Any thoughts on these?

Still in my "to-apply" queue, I was traveling yesterday and am trying to
catch up.  Don't worry, they aren't lost :)

thanks,

greg k-h

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Tomoya MORINAGA]

2012年3月1日9:47 Darren Hart <dvhart@linux.intel.com>:
> how do we force the
> 192MHz clock setting BEFORE the pch_phub code runs?

To realize your requirement,
I think that set by BIOS is only way.

ROHM Co., Ltd.
tomoya

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

On 02/29/2012 05:06 PM, Tomoya MORINAGA wrote:
> 2012年3月1日9:47 Darren Hart <dvhart@linux.intel.com>:
>> how do we force the
>> 192MHz clock setting BEFORE the pch_phub code runs?
> 
> To realize your requirement,
> I think that set by BIOS is only way.

Perhaps a pch_phub.bios_uartclk parameter could be used for setting up
the initial console and once the phub driver is loaded, it can make the
necessary changes and fix things up in the uart port? This would add a
binding between the pch_uart and pch_phub code.

Thoughts?

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

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Tomoya MORINAGA]

2012年3月1日10:12 Darren Hart <dvhart@linux.intel.com>:
> Perhaps a pch_phub.bios_uartclk parameter could be used for setting up
> the initial console and once the phub driver is loaded, it can make the
> necessary changes and fix things up in the uart port?

Yes, once phub driver is installed, uart works with new clock set by phub.

> This would add a binding between the pch_uart and pch_phub code.
What does the above "binding" mean ?

ROHM Co., Ltd.
tomoya

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

On 02/29/2012 06:06 PM, Tomoya MORINAGA wrote:
> 2012年3月1日10:12 Darren Hart <dvhart@linux.intel.com>:
>> Perhaps a pch_phub.bios_uartclk parameter could be used for setting up
>> the initial console and once the phub driver is loaded, it can make the
>> necessary changes and fix things up in the uart port?
> 
> Yes, once phub driver is installed, uart works with new clock set by phub.
> 
>> This would add a binding between the pch_uart and pch_phub code.
> What does the above "binding" mean ?
> 

Just that currently I can build pch_uart without the pch_phub driver. It
seems if we were to attempt the above, the pch_uart code would then
require the pch_phub driver and the pch_phub driver would need to know
how to trigger the pch_uart code to setup the uart port again with the
new clock setting.

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

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Tomoya MORINAGA]

2012年3月1日11:07 Darren Hart <dvhart@linux.intel.com>:
> Just that currently I can build pch_uart without the pch_phub driver. It
> seems if we were to attempt the above, the pch_uart code would then
> require the pch_phub driver and the pch_phub driver would need to know
> how to trigger the pch_uart code to setup the uart port again with the
> new clock setting.
Looks good If pch_phub can notify pch_uart to change clock setting.

thanks.
-- 
ROHM Co., Ltd.
tomoya

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Feng Tang]

On Wed, Feb 29, 2012 at 05:12:08PM -0800, Darren Hart wrote:
> 
> 
> On 02/29/2012 05:06 PM, Tomoya MORINAGA wrote:
> > 2012年3月1日9:47 Darren Hart <dvhart@linux.intel.com>:
> >> how do we force the
> >> 192MHz clock setting BEFORE the pch_phub code runs?
> > 
> > To realize your requirement,
> > I think that set by BIOS is only way.
> 
> Perhaps a pch_phub.bios_uartclk parameter could be used for setting up
> the initial console and once the phub driver is loaded, it can make the
> necessary changes and fix things up in the uart port? This would add a
> binding between the pch_uart and pch_phub code.
> 
> Thoughts?

To satisfy the early console, one way may be add a x86_init_ops for all
Tunnel Creek compatible platform like what Moorestown/Medfield have done,
and insert the uart clk setting work into one of the early init function. 

Thanks,
Feng
> 
> -- 
> 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 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Feng Tang]

On Wed, Feb 29, 2012 at 05:12:08PM -0800, Darren Hart wrote:
> 
> 
> On 02/29/2012 05:06 PM, Tomoya MORINAGA wrote:
> > 2012年3月1日9:47 Darren Hart <dvhart@linux.intel.com>:
> >> how do we force the
> >> 192MHz clock setting BEFORE the pch_phub code runs?
> > 
> > To realize your requirement,
> > I think that set by BIOS is only way.
> 
> Perhaps a pch_phub.bios_uartclk parameter could be used for setting up
> the initial console and once the phub driver is loaded, it can make the
> necessary changes and fix things up in the uart port? This would add a
> binding between the pch_uart and pch_phub code.
> 
> Thoughts?

To satisfy the early console, one way may be add a x86_init_ops for all
Tunnel Creek compatible platform like what Moorestown/Medfield have done,
and insert the uart clk setting work into one of the early init function. 

Thanks,
Feng
> 
> -- 
> Darren Hart
> Intel Open Source Technology Center
> Yocto Project - Linux Kernel

Re: [PATCH 0/4 V2] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

On 02/29/2012 06:40 PM, Feng Tang wrote:
> On Wed, Feb 29, 2012 at 05:12:08PM -0800, Darren Hart wrote:
>>
>>
>> On 02/29/2012 05:06 PM, Tomoya MORINAGA wrote:
>>> 2012年3月1日9:47 Darren Hart <dvhart@linux.intel.com>:
>>>> how do we force the
>>>> 192MHz clock setting BEFORE the pch_phub code runs?
>>>
>>> To realize your requirement,
>>> I think that set by BIOS is only way.
>>
>> Perhaps a pch_phub.bios_uartclk parameter could be used for setting up
>> the initial console and once the phub driver is loaded, it can make the
>> necessary changes and fix things up in the uart port? This would add a
>> binding between the pch_uart and pch_phub code.
>>
>> Thoughts?
> 
> To satisfy the early console, one way may be add a x86_init_ops for all
> Tunnel Creek compatible platform like what Moorestown/Medfield have done,
> and insert the uart clk setting work into one of the early init function. 
> 

I believe this is inline with what Alan was recommending in his response
to the V1 patch series, and it sounds like a reasonable next step.

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

[PATCH 0/4] pch_uart: Cleanups, board quirks, and user uartclk parameter

[Darren Hart]

This series does some minor clean-up to the pch_uart driver, adds support
for the Fish River Island II UART clock, and introduces a user_uartclk
parameter to aid in developing for early and changing hardware.

Note that this series is my proposed alternative solution to that provided
by Tomoya MORNIAGA and Feng Tang which drops the board quirks and opts to
assume a 192 MHz clock on all boards. The problem with this approach is
that the CLKCFG register may have been set to something other than the
192MHz configuration by the firmware. If so, then the pch_uart will send
garbage between the time the boot console is disabled and the pch_phub
sets the CLKCFG register again. In my case, the pch_phub PCI probe occurs
after the pch_uart_console_setup. Even if it happened before, the output
up until the PCI probing would be garbage.

In order to support an early serial console, we cannot rely on the pch_phub
probe function to setup the CFGCLK register. This series relies on the board
quirks and doesn't force the setting of the CLKREG in the pch_phub code.
Instead, it aligns with what is the default configuration (defined by firmware)
for a given board. The user_uartclk provides a mechanism to force a specific
uartclk if necessary.

--
Darren

The following changes since commit 27e74da9800289e69ba907777df1e2085231eff7:

  i387: export 'fpu_owner_task' per-cpu variable (2012-02-20 19:34:10 -0800)

are available in the git repository at:
  git://git.infradead.org/users/dvhart/linux-2.6.git pch_uart
  http://git.infradead.org/users/dvhart/linux-2.6.git/shortlog/refs/heads/pch_uart
Darren Hart (4):
  pch_uart: Use uartclk instead of base_baud
  pch_uart: Add Fish River Island II uart clock quirks
  pch_uart: Add user_uartclk parameter
  pch_uart: Use existing default_baud in setup_console

 drivers/tty/serial/pch_uart.c |   52 +++++++++++++++++++++++++++++-----------
 1 files changed, 37 insertions(+), 15 deletions(-)

-- 
1.7.6.5

[PATCH 1/4] pch_uart: Use uartclk instead of base_baud

[Darren Hart]

The term "base baud" refers to the fastest baud rate the device can communicate
at. This is clock/16. pch_uart is using base_baud as the clock itself. Rename
the variables to be semantically correct.

Signed-off-by: Darren Hart <dvhart@linux.intel.com>
CC: Tomoya MORINAGA <tomoya.rohm@gmail.com>
CC: Feng Tang <feng.tang@intel.com>
CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC: Alan Cox <alan@linux.intel.com>
CC: linux-serial@vger.kernel.org
---
 drivers/tty/serial/pch_uart.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 17ae657..c565817 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_UARTCLK 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 uartclk;
 	int start_tx;
 	int start_rx;
 	int tx_empty;
@@ -293,7 +293,7 @@ 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 uartclk)
 {
 	struct eg20t_port *priv = pci_get_drvdata(pdev);
 
@@ -332,7 +332,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->uartclk / 16, baud);
 	if (div < 0 || USHRT_MAX <= div) {
 		dev_err(priv->port.dev, "Invalid Baud(div=0x%x)\n", div);
 		return -EINVAL;
@@ -1153,9 +1153,9 @@ static int pch_uart_startup(struct uart_port *port)
 	priv->tx_empty = 1;
 
 	if (port->uartclk)
-		priv->base_baud = port->uartclk;
+		priv->uartclk = port->uartclk;
 	else
-		port->uartclk = priv->base_baud;
+		port->uartclk = priv->uartclk;
 
 	pch_uart_hal_disable_interrupt(priv, PCH_UART_HAL_ALL_INT);
 	ret = pch_uart_hal_set_line(priv, default_baud,
@@ -1507,7 +1507,7 @@ static int __init pch_console_setup(struct console *co, char *options)
 		return -ENODEV;
 
 	/* setup uartclock */
-	port->uartclk = DEFAULT_BAUD_RATE;
+	port->uartclk = DEFAULT_UARTCLK;
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
@@ -1550,7 +1550,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, uartclk;
 	int port_type;
 	struct pch_uart_driver_data *board;
 	const char *board_name;
@@ -1566,12 +1566,12 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
 	if (!rxbuf)
 		goto init_port_free_txbuf;
 
-	base_baud = DEFAULT_BAUD_RATE;
+	uartclk = DEFAULT_UARTCLK;
 
 	/* 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 */
+		uartclk = 192000000; /* 192.0MHz */
 
 	switch (port_type) {
 	case PORT_UNKNOWN:
@@ -1597,7 +1597,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->uartclk = uartclk;
 	priv->port_type = PORT_MAX_8250 + port_type + 1;
 	priv->port.dev = &pdev->dev;
 	priv->port.iobase = iobase;
@@ -1614,7 +1614,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, uartclk);
 
 #ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
 	pch_uart_ports[board->line_no] = priv;
-- 
1.7.6.5