2011-06-22 09:50:45

by Alexander Stein

[permalink] [raw]
Subject: [PATCH] pch_uart: Add MSI support

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

diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 4652109..9db9773 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -234,6 +234,7 @@ struct eg20t_port {
int tx_dma_use;
void *rx_buf_virt;
dma_addr_t rx_buf_dma;
+ int use_msi;
};

/**
@@ -1429,6 +1430,12 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
goto init_port_hal_free;
}

+ ret = pci_enable_msi(pdev);
+ if (ret)
+ priv->use_msi = 0;
+ else
+ priv->use_msi = 1;
+
iobase = pci_resource_start(pdev, 0);
mapbase = pci_resource_start(pdev, 1);
priv->mapbase = mapbase;
@@ -1485,6 +1492,9 @@ static void pch_uart_pci_remove(struct pci_dev *pdev)
struct eg20t_port *priv;

priv = (struct eg20t_port *)pci_get_drvdata(pdev);
+
+ if (priv->use_msi)
+ pci_disable_msi(pdev);
pch_uart_exit_port(priv);
pci_disable_device(pdev);
kfree(priv);
@@ -1568,6 +1578,8 @@ static int __devinit pch_uart_pci_probe(struct pci_dev *pdev,
return ret;

probe_disable_device:
+ if (priv->use_msi)
+ pci_disable_msi(pdev);
pci_disable_device(pdev);
probe_error:
return ret;
--
1.7.3.4


2011-06-23 00:14:55

by Tomoya MORINAGA

[permalink] [raw]
Subject: Re: [PATCH] pch_uart: Add MSI support

(2011/06/22 18:50), Alexander Stein wrote:
> Signed-off-by: Alexander Stein<[email protected]>
> ---
> drivers/tty/serial/pch_uart.c | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
> index 4652109..9db9773 100644
> --- a/drivers/tty/serial/pch_uart.c
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -234,6 +234,7 @@ struct eg20t_port {
> int tx_dma_use;
> void *rx_buf_virt;
> dma_addr_t rx_buf_dma;
> + int use_msi;
> };
>
> /**
> @@ -1429,6 +1430,12 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
> goto init_port_hal_free;
> }
>
> + ret = pci_enable_msi(pdev);
> + if (ret)
> + priv->use_msi = 0;
> + else
> + priv->use_msi = 1;
> +
> iobase = pci_resource_start(pdev, 0);
> mapbase = pci_resource_start(pdev, 1);
> priv->mapbase = mapbase;
> @@ -1485,6 +1492,9 @@ static void pch_uart_pci_remove(struct pci_dev *pdev)
> struct eg20t_port *priv;
>
> priv = (struct eg20t_port *)pci_get_drvdata(pdev);
> +
> + if (priv->use_msi)
> + pci_disable_msi(pdev);
> pch_uart_exit_port(priv);
> pci_disable_device(pdev);
> kfree(priv);
> @@ -1568,6 +1578,8 @@ static int __devinit pch_uart_pci_probe(struct pci_dev *pdev,
> return ret;
>
> probe_disable_device:
> + if (priv->use_msi)
> + pci_disable_msi(pdev);
> pci_disable_device(pdev);
> probe_error:
> return ret;

Thank you for your updating.

--
tomoya
OKI SEMICONDUCTOR CO., LTD.

2011-06-23 00:34:49

by Roland Dreier

[permalink] [raw]
Subject: Re: [PATCH] pch_uart: Add MSI support

On Wed, Jun 22, 2011 at 2:50 AM, Alexander Stein
<[email protected]> wrote:
> + ? ? ? if (priv->use_msi)
> + ? ? ? ? ? ? ? pci_disable_msi(pdev);

Trivial nit: pci_disable_msi() is safe even if you didn't enable msi.
So you can drop the use_msi flag (the PCI layer has dev->msi_enabled
internally to track this), and just always call pci_disable_msi().

- R.

2011-06-23 07:00:25

by Alexander Stein

[permalink] [raw]
Subject: [PATCH v2] pch_uart: Add MSI support

Signed-off-by: Alexander Stein <[email protected]>
---
Changes in v2:
* msi_enabled is stored in pci_dev already, skip it in our code

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 a3d0c9e..ab34576 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1578,6 +1578,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
goto init_port_hal_free;
}

+ pci_enable_msi(pdev);
+
iobase = pci_resource_start(pdev, 0);
mapbase = pci_resource_start(pdev, 1);
priv->mapbase = mapbase;
@@ -1640,6 +1642,8 @@ static void pch_uart_pci_remove(struct pci_dev *pdev)
struct eg20t_port *priv;

priv = (struct eg20t_port *)pci_get_drvdata(pdev);
+
+ pci_disable_msi(pdev);
#ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
pch_uart_ports[priv->port.line] = NULL;
#endif
@@ -1726,6 +1730,7 @@ static int __devinit pch_uart_pci_probe(struct pci_dev *pdev,
return ret;

probe_disable_device:
+ pci_disable_msi(pdev);
pci_disable_device(pdev);
probe_error:
return ret;
--
1.7.3.4

2011-06-23 07:16:43

by Clemens Ladisch

[permalink] [raw]
Subject: Re: [PATCH v2] pch_uart: Add MSI support

Alexander Stein wrote:
> +++ b/drivers/tty/serial/pch_uart.c
> @@ -1578,6 +1578,8 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
>
> + pci_enable_msi(pdev);

Pretty much every driver that tries this eventually ends up with
a blacklist of devices that don't implement MSI correctly.
Did you test this with all the models that are supported by this driver?


Regards,
Clemens

2011-06-23 07:38:15

by Alexander Stein

[permalink] [raw]
Subject: Re: [PATCH v2] pch_uart: Add MSI support

On Thursday 23 June 2011 09:16:38 Clemens Ladisch wrote:
> Alexander Stein wrote:
> > +++ b/drivers/tty/serial/pch_uart.c
> > @@ -1578,6 +1578,8 @@ static struct eg20t_port *pch_uart_init_port(struct
> > pci_dev *pdev,
> >
> > + pci_enable_msi(pdev);
>
> Pretty much every driver that tries this eventually ends up with
> a blacklist of devices that don't implement MSI correctly.
> Did you test this with all the models that are supported by this driver?

I don't have all models supported by this driver. So I could only test with
this one:
#lspci -v -s 02:0a.1
02:0a.1 Serial controller: Intel Corporation Device 8811 (prog-if 02 [16550])
Subsystem: Intel Corporation Device 8811
Flags: bus master, fast devsel, latency 0, IRQ 40
I/O ports at 9040 [size=8]
Memory at 9fefb2f0 (32-bit, non-prefetchable) [size=16]
Capabilities: [40] MSI: Mask- 64bit- Count=1/1 Enable+
Capabilities: [50] Power Management version 2
Kernel driver in use: pch_uart

There are 3 more for the other ports.

Regards,
Alexander