2012-02-29 18:26:32

by Darren Hart

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

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


2012-02-29 18:25:30

by Darren Hart

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

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 <[email protected]>
CC: Tomoya MORINAGA <[email protected]>
CC: Feng Tang <[email protected]>
CC: Greg Kroah-Hartman <[email protected]>
CC: Alan Cox <[email protected]>
CC: [email protected]
---
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

2012-02-29 18:25:39

by Darren Hart

[permalink] [raw]
Subject: [PATCH 3/4] pch_uart: Add user_uartclk parameter

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 <[email protected]>
CC: Tomoya MORINAGA <[email protected]>
CC: Feng Tang <[email protected]>
CC: Greg Kroah-Hartman <[email protected]>
CC: Alan Cox <[email protected]>
CC: [email protected]
---
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

2012-02-29 18:26:23

by Darren Hart

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

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

Signed-off-by: Darren Hart <[email protected]>
CC: Tomoya MORINAGA <[email protected]>
CC: Feng Tang <[email protected]>
CC: Greg Kroah-Hartman <[email protected]>
CC: Alan Cox <[email protected]>
CC: [email protected]
---
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

2012-02-29 18:26:51

by Darren Hart

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

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 <[email protected]>
CC: Tomoya MORINAGA <[email protected]>
CC: Feng Tang <[email protected]>
CC: Greg Kroah-Hartman <[email protected]>
CC: Alan Cox <[email protected]>
CC: [email protected]
---
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

2012-03-01 00:38:44

by Tomoya MORINAGA

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

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

2012-03-01 00:49:36

by Darren Hart

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



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

2012-03-01 00:53:21

by Greg Kroah-Hartman

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

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

2012-03-01 01:06:40

by Tomoya MORINAGA

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

2012$BG/(B3$B7n(B1$BF|(B9:47 Darren Hart <[email protected]>:
> 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

2012-03-01 01:12:46

by Darren Hart

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



On 02/29/2012 05:06 PM, Tomoya MORINAGA wrote:
> 2012$BG/(B3$B7n(B1$BF|(B9:47 Darren Hart <[email protected]>:
>> 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

2012-03-01 02:06:31

by Tomoya MORINAGA

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

2012$BG/(B3$B7n(B1$BF|(B10:12 Darren Hart <[email protected]>:
> 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

2012-03-01 02:08:30

by Darren Hart

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



On 02/29/2012 06:06 PM, Tomoya MORINAGA wrote:
> 2012$BG/(B3$B7n(B1$BF|(B10:12 Darren Hart <[email protected]>:
>> 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

2012-03-01 02:27:24

by Tomoya MORINAGA

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

2012$BG/(B3$B7n(B1$BF|(B11:07 Darren Hart <[email protected]>:
> 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

2012-03-01 02:42:50

by Feng Tang

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

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 <[email protected]>:
> >> 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