* Re: [PATCH] serial: 8250: Add cpufreq support
From: Alan Cox @ 2012-04-05 10:53 UTC (permalink / raw)
To: Shankarmurthy, Akshay
Cc: linux-serial@vger.kernel.org, alan@linux.intel.com,
gregkh@linuxfoundation.org, paul.gortmaker@windriver.com,
jamie@jamieiles.com, swarren@nvidia.com, dianders@chromium.org,
davinci-linux-open-source@linux.davincidsp.com,
linux-kernel@vger.kernel.org
In-Reply-To: <6F6B80BD2E18844D97BB4B51FE3705C831790EA0@DBDE01.ent.ti.com>
On Thu, 5 Apr 2012 10:40:55 +0000
"Shankarmurthy, Akshay" <akshay.s@ti.com> wrote:
> Hi,
>
> On Tue, Apr 03, 2012 at 15:25:30, Alan Cox wrote:
> > > This patch was submitted 2 years ago but didn't make it to the mainline. Now i am reposting it.
> >
> > Can you fix it instead of just reposting it ?
> >
> >
> > > +#ifdef CONFIG_CPU_FREQ
> > > +static int serial8250_cpufreq_transition(struct notifier_block *nb,
> > > + unsigned long val, void *data)
> > > +{
> > > + struct uart_8250_port *p;
> > > + struct uart_port *uport;
> >
> > What is your locking model ?
> >
>
> I will have a look at this and add the lock if necessary.
At the very least you need reference counts held on the tty struct and to
allow for the tty having vanished under you.
> >
> > > +static inline void serial8250_cpufreq_deregister(struct uart_8250_port *p)
> >
> > unregister
>
> Ok. I will change it.
>
> >
> > > + ret = serial8250_cpufreq_register(uart);
> > > + if (ret < 0)
> > > + printk(KERN_ERR "Failed to add cpufreq notifier\n");
> >
> > Why do this for devices that don't care.
>
> This is taken care in the code. If the device's frequency doesn't change after the
> change in the cpu frequency then it doesn't go for execution of the function "serial8250_set_termios".
But you still call each function and walk the list for each port on each
CPU transition. There are people with 128 ports in their systems and the
cpu frequency change is latency critical.
Nothing should even be getting registered in such cases.
^ permalink raw reply
* Re: [PATCH v3] OMAP2+: UART: Remove cpu checks for populating errata flags
From: Shubhrajyoti @ 2012-04-05 11:12 UTC (permalink / raw)
To: Govindraj.R
Cc: linux-omap, linux-serial, linux-arm-kernel, Jon Hunter,
Paul Walmsley, Kevin Hilman, Felipe Balbi
In-Reply-To: <1333460554-7356-1-git-send-email-govindraj.raja@ti.com>
Hi Govind,
On Tuesday 03 April 2012 07:12 PM, Govindraj.R wrote:
> From: "Govindraj.R" <govindraj.raja@ti.com>
>
> Currently the errata is populated based on cpu checks this can
> be removed and replaced with module version check of uart ip block.
> MVR reg is provided within the uart reg map use the same
> to populate the errata and thus now errata population and handling
> can be managed within the driver itself.
>
> Cc: Jon Hunter <jon-hunter@ti.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Kevin Hilman <khilman@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> Signed-off-by: Govindraj.R <govindraj.raja@ti.com>
> ---
> arch/arm/mach-omap2/serial.c | 8 ---
> arch/arm/plat-omap/include/plat/omap-serial.h | 1 -
> drivers/tty/serial/omap-serial.c | 74 ++++++++++++++++++++++++-
> 3 files changed, 73 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 0cdd359..6affdd4 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -355,14 +355,6 @@ void __init omap_serial_init_port(struct omap_board_data *bdata,
> omap_up.dma_rx_poll_rate = info->dma_rx_poll_rate;
> omap_up.autosuspend_timeout = info->autosuspend_timeout;
>
> - /* Enable the MDR1 Errata i202 for OMAP2430/3xxx/44xx */
> - if (!cpu_is_omap2420() && !cpu_is_ti816x())
> - omap_up.errata |= UART_ERRATA_i202_MDR1_ACCESS;
> -
> - /* Enable DMA Mode Force Idle Errata i291 for omap34xx/3630 */
> - if (cpu_is_omap34xx() || cpu_is_omap3630())
34xx is true for 3630 also thanks for fixing the unneeded oring.
:-)
> - omap_up.errata |= UART_ERRATA_i291_DMA_FORCEIDLE;
> -
> pdata = &omap_up;
> pdata_size = sizeof(struct omap_uart_port_info);
>
> diff --git a/arch/arm/plat-omap/include/plat/omap-serial.h b/arch/arm/plat-omap/include/plat/omap-serial.h
> index 9ff4444..1a52725 100644
> --- a/arch/arm/plat-omap/include/plat/omap-serial.h
> +++ b/arch/arm/plat-omap/include/plat/omap-serial.h
> @@ -65,7 +65,6 @@ struct omap_uart_port_info {
> bool dma_enabled; /* To specify DMA Mode */
> unsigned int uartclk; /* UART clock rate */
> upf_t flags; /* UPF_* flags */
> - u32 errata;
> unsigned int dma_rx_buf_size;
> unsigned int dma_rx_timeout;
> unsigned int autosuspend_timeout;
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index 0121486..0555c96 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -44,6 +44,13 @@
> #include <plat/dmtimer.h>
> #include <plat/omap-serial.h>
>
> +#define UART_BUILD_REVISION(x, y) (((x) << 8) | (y))
> +
> +#define OMAP_UART_REV_42 0x0402
> +#define OMAP_UART_REV_46 0x0406
> +#define OMAP_UART_REV_52 0x0502
> +#define OMAP_UART_REV_63 0x0603
Maybe a comment can be added or the #define changed to reflect
the processor also so that it is easy to map.
A dev_info print could be added so that the bootup logs(probe) could
tell what ip rev is booted is running.
Feel free to ignore such trivial comments.
> +
> #define DEFAULT_CLK_SPEED 48000000 /* 48Mhz*/
>
> /* SCR register bitmasks */
> @@ -53,6 +60,17 @@
> #define OMAP_UART_FCR_RX_FIFO_TRIG_SHIFT 6
> #define OMAP_UART_FCR_RX_FIFO_TRIG_MASK (0x3 << 6)
>
> +/* MVR register bitmasks */
> +#define OMAP_UART_MVR_SCHEME_SHIFT 30
> +
> +#define OMAP_UART_LEGACY_MVR_MAJ_MASK 0xf0
> +#define OMAP_UART_LEGACY_MVR_MAJ_SHIFT 4
> +#define OMAP_UART_LEGACY_MVR_MIN_MASK 0x0f
> +
> +#define OMAP_UART_MVR_MAJ_MASK 0x700
> +#define OMAP_UART_MVR_MAJ_SHIFT 8
> +#define OMAP_UART_MVR_MIN_MASK 0x3f
> +
> static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
>
> /* Forward declaration of functions */
> @@ -1346,6 +1364,59 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
> return;
> }
>
> +static void omap_serial_fill_features_erratas(struct uart_omap_port *up)
> +{
> + u32 mvr, scheme;
> + u16 revision, major, minor;
> +
> + mvr = serial_in(up, UART_OMAP_MVER);
> +
> + /* Check revision register scheme */
> + scheme = mvr >> OMAP_UART_MVR_SCHEME_SHIFT;
> +
> + switch (scheme) {
> + case 0: /* Legacy Scheme: OMAP2/3 */
> + /* MINOR_REV[0:4], MAJOR_REV[4:7] */
> + major = (mvr & OMAP_UART_LEGACY_MVR_MAJ_MASK) >>
> + OMAP_UART_LEGACY_MVR_MAJ_SHIFT;
> + minor = (mvr & OMAP_UART_LEGACY_MVR_MIN_MASK);
> + break;
> + case 1:
> + /* New Scheme: OMAP4+ */
> + /* MINOR_REV[0:5], MAJOR_REV[8:10] */
> + major = (mvr & OMAP_UART_MVR_MAJ_MASK) >>
> + OMAP_UART_MVR_MAJ_SHIFT;
> + minor = (mvr & OMAP_UART_MVR_MIN_MASK);
> + break;
> + default:
> + dev_warn(&up->pdev->dev,
> + "Unknown %s revision, defaulting to highest\n",
> + up->name);
> + /* highest possible revision */
> + major = 0xff;
> + minor = 0xff;
> + }
> +
> + /* normalize revision for the driver */
> + revision = UART_BUILD_REVISION(major, minor);
> +
> + switch (revision) {
> + case OMAP_UART_REV_46:
> + up->errata |= (UART_ERRATA_i202_MDR1_ACCESS |
> + UART_ERRATA_i291_DMA_FORCEIDLE);
> + break;
> + case OMAP_UART_REV_52:
> + up->errata |= (UART_ERRATA_i202_MDR1_ACCESS |
> + UART_ERRATA_i291_DMA_FORCEIDLE);
> + break;
> + case OMAP_UART_REV_63:
> + up->errata |= UART_ERRATA_i202_MDR1_ACCESS;
> + break;
> + default:
> + break;
> + }
> +}
> +
> static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
> {
> struct omap_uart_port_info *omap_up_info;
> @@ -1443,7 +1514,6 @@ static int serial_omap_probe(struct platform_device *pdev)
> "%d\n", DEFAULT_CLK_SPEED);
> }
> up->uart_dma.uart_base = mem->start;
> - up->errata = omap_up_info->errata;
>
> if (omap_up_info->dma_enabled) {
> up->uart_dma.uart_dma_tx = dma_tx->start;
> @@ -1473,6 +1543,8 @@ static int serial_omap_probe(struct platform_device *pdev)
> pm_runtime_enable(&pdev->dev);
> pm_runtime_get_sync(&pdev->dev);
>
> + omap_serial_fill_features_erratas(up);
Nit : errata ( it is the prural of erratum)
> +
> ui[up->port.line] = up;
> serial_omap_add_console_port(up);
>
^ permalink raw reply
* Re: [PATCH 0/3] OMAP2+: UART: Enable tx wakeup + remove cpu checks
From: Raja, Govindraj @ 2012-04-05 11:24 UTC (permalink / raw)
To: Tony Lindgren, Alan Cox, Greg KH
Cc: linux-serial, linux-arm-kernel, Kevin Hilman, Paul Walmsley,
Felipe Balbi, Govindraj.R, linux-omap
In-Reply-To: <1332325501-10467-1-git-send-email-govindraj.raja@ti.com>
On Wed, Mar 21, 2012 at 3:54 PM, Govindraj.R <govindraj.raja@ti.com> wrote:
> From: "Govindraj.R" <govindraj.raja@ti.com>
>
> Based on Linux-OMAP tree uart branch.
> (git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
> remotes/origin/uart)
>
> * Removes the cpu checks wherever possible and use version reg
> for populating features and errata's
> * enable tx wakeup available in wer reg for applicable
> module revision's
>
> Govindraj.R (3):
> OMAP2+: UART: Remove cpu checks for populating errata flags
> OMAP2+: UART: enable tx wakeup bit for wer reg
> OMAP2+: UART: replace omap34xx/omap4xx cpu checks with not omap24xx
>
> arch/arm/mach-omap2/serial.c | 13 +----
> arch/arm/plat-omap/include/plat/omap-serial.h | 8 +++-
> drivers/tty/serial/omap-serial.c | 71 ++++++++++++++++++++++++-
> 3 files changed, 78 insertions(+), 14 deletions(-)
Alan / Greg,
With you ack on tty parts can this series be upstreamed from
omap tree ?
Tony,
Can you take these series into your uart fixes branch for 3.4-rc
(Let me know if you need a pull request with patches based on
linux-omap tree uart branch)
--
Thanks,
Govindraj.R
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/3] OMAP2+: UART: Enable tx wakeup + remove cpu checks
From: Alan Cox @ 2012-04-05 12:26 UTC (permalink / raw)
To: Raja, Govindraj
Cc: Tony Lindgren, Greg KH, linux-serial, linux-arm-kernel,
Kevin Hilman, Paul Walmsley, Felipe Balbi, linux-omap
In-Reply-To: <CAMrsUd+_coW7ebuN_o=yZbbSVKnkx5R6=yHjUEUc0EcQGtgSRQ@mail.gmail.com>
On Thu, 5 Apr 2012 16:54:03 +0530
"Raja, Govindraj" <govindraj.raja@ti.com> wrote:
> On Wed, Mar 21, 2012 at 3:54 PM, Govindraj.R <govindraj.raja@ti.com>
> wrote:
> > From: "Govindraj.R" <govindraj.raja@ti.com>
> >
> > Based on Linux-OMAP tree uart branch.
> > (git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
> > remotes/origin/uart)
> >
> > * Removes the cpu checks wherever possible and use version reg
> > for populating features and errata's
> > * enable tx wakeup available in wer reg for applicable
> > module revision's
> >
> > Govindraj.R (3):
> > OMAP2+: UART: Remove cpu checks for populating errata flags
> > OMAP2+: UART: enable tx wakeup bit for wer reg
> > OMAP2+: UART: replace omap34xx/omap4xx cpu checks with not omap24xx
> >
> > arch/arm/mach-omap2/serial.c | 13 +----
> > arch/arm/plat-omap/include/plat/omap-serial.h | 8 +++-
> > drivers/tty/serial/omap-serial.c | 71
> > ++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 14
> > deletions(-)
>
> Alan / Greg,
>
> With you ack on tty parts can this series be upstreamed from
> omap tree ?
Fine by me - this seems primarily to be architectural details.
--
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
^ permalink raw reply
* Re: [PATCH 0/3] OMAP2+: UART: Enable tx wakeup + remove cpu checks
From: Greg KH @ 2012-04-05 16:47 UTC (permalink / raw)
To: Alan Cox
Cc: Raja, Govindraj, Tony Lindgren, linux-serial, linux-arm-kernel,
Kevin Hilman, Paul Walmsley, Felipe Balbi, linux-omap
In-Reply-To: <20120405132617.76e04217@bob.linux.org.uk>
On Thu, Apr 05, 2012 at 01:26:17PM +0100, Alan Cox wrote:
> On Thu, 5 Apr 2012 16:54:03 +0530
> "Raja, Govindraj" <govindraj.raja@ti.com> wrote:
>
> > On Wed, Mar 21, 2012 at 3:54 PM, Govindraj.R <govindraj.raja@ti.com>
> > wrote:
> > > From: "Govindraj.R" <govindraj.raja@ti.com>
> > >
> > > Based on Linux-OMAP tree uart branch.
> > > (git://git.kernel.org/pub/scm/linux/kernel/git/tmlind/linux-omap.git
> > > remotes/origin/uart)
> > >
> > > * Removes the cpu checks wherever possible and use version reg
> > > for populating features and errata's
> > > * enable tx wakeup available in wer reg for applicable
> > > module revision's
> > >
> > > Govindraj.R (3):
> > > OMAP2+: UART: Remove cpu checks for populating errata flags
> > > OMAP2+: UART: enable tx wakeup bit for wer reg
> > > OMAP2+: UART: replace omap34xx/omap4xx cpu checks with not omap24xx
> > >
> > > arch/arm/mach-omap2/serial.c | 13 +----
> > > arch/arm/plat-omap/include/plat/omap-serial.h | 8 +++-
> > > drivers/tty/serial/omap-serial.c | 71
> > > ++++++++++++++++++++++++- 3 files changed, 78 insertions(+), 14
> > > deletions(-)
> >
> > Alan / Greg,
> >
> > With you ack on tty parts can this series be upstreamed from
> > omap tree ?
>
> Fine by me - this seems primarily to be architectural details.
Fine by me as well:
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
--
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
^ permalink raw reply
* [PATCH 0/6] rework quirks for the "kt" serial port
From: Dan Williams @ 2012-04-06 18:44 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, linux-serial, alan
The kt serial port intermittently reports unreliable data in its IIR
register and needs to reset fifos to stay in sync with firmware when the
device is reset.
The IIR fixes that went into 3.3 were not complete so patch 3 replaces
them with an explicit way to turn on UART_BUG_THRE (i.e. the "I don't
trust my iir" flag).
A new fix for resetting the fifos on device reset is patch 5. Alan
asked to make this a new quirk rather than add more workarounds to the
core (patch 4).
Patch 6 is an untested RFC / RFT from anyone that has a serial
suspend/resume use case with an 'init' quirk that has resources that
need de-allocation at 'exit'.
--
Dan
---
Dan Williams (5):
Revert "serial/8250_pci: init-quirk msi support for kt serial controller"
Revert "serial/8250_pci: setup-quirk workaround for the kt serial controller"
serial/8250_pci: add a "force background timer" flag and use it for the "kt" serial port
tegra, serial8250: add ->handle_break() uart_port op
serial/8250_pci: fix suspend/resume vs init/exit quirks
Sudhakar Mamillapalli (1):
serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI
arch/arm/mach-tegra/board-harmony.c | 1 +
arch/arm/mach-tegra/board-paz00.c | 2 +
arch/arm/mach-tegra/board-seaboard.c | 1 +
arch/arm/mach-tegra/board-trimslice.c | 1 +
arch/arm/mach-tegra/devices.c | 18 ++++++++++
arch/arm/mach-tegra/devices.h | 2 +
drivers/tty/serial/8250/8250.c | 56 ++++++++++--------------------
drivers/tty/serial/8250/8250.h | 2 +
drivers/tty/serial/8250/8250_pci.c | 61 +++++++++++++++++++++++++--------
include/linux/serial_8250.h | 1 +
include/linux/serial_core.h | 7 +++-
11 files changed, 99 insertions(+), 53 deletions(-)
^ permalink raw reply
* [PATCH 1/6] Revert "serial/8250_pci: init-quirk msi support for kt serial controller"
From: Dan Williams @ 2012-04-06 18:49 UTC (permalink / raw)
To: gregkh
Cc: Sudhakar Mamillapalli, linux-kernel, stable, linux-serial,
Nhan H Mai, Alan Cox
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
This reverts commit e86ff4a63c9fdd875ba8492577cd1ad2252f525c.
This tried to enforce the semantics of one interrupt per iir read of the
THRE (transmit-hold empty) status, but events from other sources
(particularly modem status) defeat this guarantee.
This change also broke 8250_pci suspend/resume support as
pciserial_resume_ports() re-runs .init() quirks, but does not run
.exit() quirks in pciserial_suspend_ports() leading to reports like:
sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:16.3/msi_irqs'
...and a subsequent crash. The mismatch of init/exit at suspend/resume
seems like a bug in its own right.
[stable: 3.3.x]
Cc: <stable@vger.kernel.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Sudhakar Mamillapalli <sudhakar@fb.com>
Reported-by: Nhan H Mai <nhan.h.mai@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/tty/serial/8250/8250_pci.c | 14 --------------
1 files changed, 0 insertions(+), 14 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index da2b0b0..1d4ccf8 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1118,18 +1118,6 @@ pci_xr17c154_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
-static int try_enable_msi(struct pci_dev *dev)
-{
- /* use msi if available, but fallback to legacy otherwise */
- pci_enable_msi(dev);
- return 0;
-}
-
-static void disable_msi(struct pci_dev *dev)
-{
- pci_disable_msi(dev);
-}
-
#define PCI_VENDOR_ID_SBSMODULARIO 0x124B
#define PCI_SUBVENDOR_ID_SBSMODULARIO 0x124B
#define PCI_DEVICE_ID_OCTPRO 0x0001
@@ -1249,9 +1237,7 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
.device = PCI_DEVICE_ID_INTEL_PATSBURG_KT,
.subvendor = PCI_ANY_ID,
.subdevice = PCI_ANY_ID,
- .init = try_enable_msi,
.setup = kt_serial_setup,
- .exit = disable_msi,
},
/*
* ITE
^ permalink raw reply related
* [PATCH 2/6] Revert "serial/8250_pci: setup-quirk workaround for the kt serial controller"
From: Dan Williams @ 2012-04-06 18:49 UTC (permalink / raw)
To: gregkh
Cc: Sudhakar Mamillapalli, linux-kernel, stable, linux-serial,
Nhan H Mai, Alan Cox
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
This reverts commit 448ac154c957c4580531fa0c8f2045816fe2f0e7.
The semantic of UPF_IIR_ONCE is only guaranteed to workaround the race
condition in the kt serial's iir register if the only source of
interrupts is THRE (fifo-empty) events. An modem status event at the
wrong time can again cause an iir read to drop the 'empty' status
leading to a hang. So, revert this in preparation for using the
existing "I don't trust my iir register" workaround in the 8250 core
(UART_BUG_THRE).
[stable: 3.3.x]
Cc: <stable@vger.kernel.org>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Sudhakar Mamillapalli <sudhakar@fb.com>
Reported-by: Nhan H Mai <nhan.h.mai@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/tty/serial/8250/8250.c | 4 +---
drivers/tty/serial/8250/8250_pci.c | 17 +----------------
include/linux/serial_core.h | 1 -
3 files changed, 2 insertions(+), 20 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 9b7336f..ec7d99b 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -1592,13 +1592,11 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
do {
struct uart_8250_port *up;
struct uart_port *port;
- bool skip;
up = list_entry(l, struct uart_8250_port, list);
port = &up->port;
- skip = pass_counter && up->port.flags & UPF_IIR_ONCE;
- if (!skip && port->handle_irq(port)) {
+ if (port->handle_irq(port)) {
handled = 1;
end = NULL;
} else if (end == NULL)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 1d4ccf8..105dcfb 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1092,14 +1092,6 @@ static int skip_tx_en_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
-static int kt_serial_setup(struct serial_private *priv,
- const struct pciserial_board *board,
- struct uart_port *port, int idx)
-{
- port->flags |= UPF_IIR_ONCE;
- return skip_tx_en_setup(priv, board, port, idx);
-}
-
static int pci_eg20t_init(struct pci_dev *dev)
{
#if defined(CONFIG_SERIAL_PCH_UART) || defined(CONFIG_SERIAL_PCH_UART_MODULE)
@@ -1118,6 +1110,7 @@ pci_xr17c154_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
+/* This should be in linux/pci_ids.h */
#define PCI_VENDOR_ID_SBSMODULARIO 0x124B
#define PCI_SUBVENDOR_ID_SBSMODULARIO 0x124B
#define PCI_DEVICE_ID_OCTPRO 0x0001
@@ -1147,7 +1140,6 @@ pci_xr17c154_setup(struct serial_private *priv,
#define PCI_DEVICE_ID_OXSEMI_16PCI958 0x9538
#define PCIE_DEVICE_ID_NEO_2_OX_IBM 0x00F6
#define PCI_DEVICE_ID_PLX_CRONYX_OMEGA 0xc001
-#define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
/* Unknown vendors/cards - this should not be in linux/pci_ids.h */
#define PCI_SUBDEVICE_ID_UNKNOWN_0x1584 0x1584
@@ -1232,13 +1224,6 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
.subdevice = PCI_ANY_ID,
.setup = ce4100_serial_setup,
},
- {
- .vendor = PCI_VENDOR_ID_INTEL,
- .device = PCI_DEVICE_ID_INTEL_PATSBURG_KT,
- .subvendor = PCI_ANY_ID,
- .subdevice = PCI_ANY_ID,
- .setup = kt_serial_setup,
- },
/*
* ITE
*/
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c91ace7..c56d114 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -355,7 +355,6 @@ struct uart_port {
#define UPF_CONS_FLOW ((__force upf_t) (1 << 23))
#define UPF_SHARE_IRQ ((__force upf_t) (1 << 24))
#define UPF_EXAR_EFR ((__force upf_t) (1 << 25))
-#define UPF_IIR_ONCE ((__force upf_t) (1 << 26))
/* The exact UART type is known and should not be probed. */
#define UPF_FIXED_TYPE ((__force upf_t) (1 << 27))
#define UPF_BOOT_AUTOCONF ((__force upf_t) (1 << 28))
^ permalink raw reply related
* [PATCH 3/6] serial/8250_pci: add a "force background timer" flag and use it for the "kt" serial port
From: Dan Williams @ 2012-04-06 18:49 UTC (permalink / raw)
To: gregkh
Cc: Sudhakar Mamillapalli, linux-kernel, stable, linux-serial,
Nhan H Mai, Alan Cox
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
Workaround dropped notifications in the iir register. Register reads
coincident with new interrupt notifications sometimes result in this
device clearing the interrupt event without reporting it in the read
data.
The serial core already has a heuristic for determining when a device
has an untrustworthy iir register. In this case when we apriori know
that the iir is faulty use a flag (UPF_BUG_THRE) to bypass the test and
force usage of the background timer.
Cc: Alan Cox <alan@linux.intel.com>
[stable: 3.3.x]
Cc: <stable@vger.kernel.org>
Reported-by: Nhan H Mai <nhan.h.mai@intel.com>
Reported-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Tested-by: Nhan H Mai <nhan.h.mai@intel.com>
Tested-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/tty/serial/8250/8250.c | 8 +++++---
drivers/tty/serial/8250/8250_pci.c | 17 ++++++++++++++++-
include/linux/serial_core.h | 1 +
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index ec7d99b..0e2c703 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -2055,10 +2055,12 @@ static int serial8250_startup(struct uart_port *port)
spin_unlock_irqrestore(&up->port.lock, flags);
/*
- * If the interrupt is not reasserted, setup a timer to
- * kick the UART on a regular basis.
+ * If the interrupt is not reasserted, or we otherwise
+ * don't trust the iir, setup a timer to kick the UART
+ * on a regular basis.
*/
- if (!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) {
+ if ((!(iir1 & UART_IIR_NO_INT) && (iir & UART_IIR_NO_INT)) ||
+ up->port.flags & UPF_BUG_THRE) {
up->bugs |= UART_BUG_THRE;
pr_debug("ttyS%d - using backup timer\n",
serial_index(port));
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 105dcfb..858dca8 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1092,6 +1092,14 @@ static int skip_tx_en_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
+static int kt_serial_setup(struct serial_private *priv,
+ const struct pciserial_board *board,
+ struct uart_port *port, int idx)
+{
+ port->flags |= UPF_BUG_THRE;
+ return skip_tx_en_setup(priv, board, port, idx);
+}
+
static int pci_eg20t_init(struct pci_dev *dev)
{
#if defined(CONFIG_SERIAL_PCH_UART) || defined(CONFIG_SERIAL_PCH_UART_MODULE)
@@ -1110,7 +1118,6 @@ pci_xr17c154_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
-/* This should be in linux/pci_ids.h */
#define PCI_VENDOR_ID_SBSMODULARIO 0x124B
#define PCI_SUBVENDOR_ID_SBSMODULARIO 0x124B
#define PCI_DEVICE_ID_OCTPRO 0x0001
@@ -1140,6 +1147,7 @@ pci_xr17c154_setup(struct serial_private *priv,
#define PCI_DEVICE_ID_OXSEMI_16PCI958 0x9538
#define PCIE_DEVICE_ID_NEO_2_OX_IBM 0x00F6
#define PCI_DEVICE_ID_PLX_CRONYX_OMEGA 0xc001
+#define PCI_DEVICE_ID_INTEL_PATSBURG_KT 0x1d3d
/* Unknown vendors/cards - this should not be in linux/pci_ids.h */
#define PCI_SUBDEVICE_ID_UNKNOWN_0x1584 0x1584
@@ -1224,6 +1232,13 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = {
.subdevice = PCI_ANY_ID,
.setup = ce4100_serial_setup,
},
+ {
+ .vendor = PCI_VENDOR_ID_INTEL,
+ .device = PCI_DEVICE_ID_INTEL_PATSBURG_KT,
+ .subvendor = PCI_ANY_ID,
+ .subdevice = PCI_ANY_ID,
+ .setup = kt_serial_setup,
+ },
/*
* ITE
*/
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index c56d114..cbb3d12 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -355,6 +355,7 @@ struct uart_port {
#define UPF_CONS_FLOW ((__force upf_t) (1 << 23))
#define UPF_SHARE_IRQ ((__force upf_t) (1 << 24))
#define UPF_EXAR_EFR ((__force upf_t) (1 << 25))
+#define UPF_BUG_THRE ((__force upf_t) (1 << 26))
/* The exact UART type is known and should not be probed. */
#define UPF_FIXED_TYPE ((__force upf_t) (1 << 27))
#define UPF_BOOT_AUTOCONF ((__force upf_t) (1 << 28))
^ permalink raw reply related
* [PATCH 4/6] tegra, serial8250: add ->handle_break() uart_port op
From: Dan Williams @ 2012-04-06 18:49 UTC (permalink / raw)
To: gregkh
Cc: Sudhakar Mamillapalli, Stephen Warren, linux-kernel, linux-serial,
Colin Cross, Olof Johansson, Nhan H Mai, Alan Cox, alan
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
The "KT" serial port has another use case for a "received break" quirk,
so before adding another special case to the 8250 core take this
opportunity to push such quirks out of the core and into a uart_port op.
Cc: Nhan H Mai <nhan.h.mai@intel.com>
Cc: Colin Cross <ccross@android.com>
Cc: Olof Johansson <olof@lixom.net>
Cc: Stephen Warren <swarren@nvidia.com>
Acked-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Reported-by: Alan Cox <alan@lxorguk.ukuu.org.uk>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
arch/arm/mach-tegra/board-harmony.c | 1 +
arch/arm/mach-tegra/board-paz00.c | 2 ++
arch/arm/mach-tegra/board-seaboard.c | 1 +
arch/arm/mach-tegra/board-trimslice.c | 1 +
arch/arm/mach-tegra/devices.c | 18 +++++++++++++++++
arch/arm/mach-tegra/devices.h | 2 ++
drivers/tty/serial/8250/8250.c | 34 +++------------------------------
include/linux/serial_8250.h | 1 +
include/linux/serial_core.h | 5 +++++
9 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/arch/arm/mach-tegra/board-harmony.c b/arch/arm/mach-tegra/board-harmony.c
index 789bdc9..7a4755b 100644
--- a/arch/arm/mach-tegra/board-harmony.c
+++ b/arch/arm/mach-tegra/board-harmony.c
@@ -52,6 +52,7 @@ static struct plat_serial8250_port debug_uart_platform_data[] = {
.irq = INT_UARTD,
.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_TYPE,
.type = PORT_TEGRA,
+ .handle_break = tegra_serial_handle_break,
.iotype = UPIO_MEM,
.regshift = 2,
.uartclk = 216000000,
diff --git a/arch/arm/mach-tegra/board-paz00.c b/arch/arm/mach-tegra/board-paz00.c
index 330afdf..9426e76 100644
--- a/arch/arm/mach-tegra/board-paz00.c
+++ b/arch/arm/mach-tegra/board-paz00.c
@@ -55,6 +55,7 @@ static struct plat_serial8250_port debug_uart_platform_data[] = {
.irq = INT_UARTA,
.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_TYPE,
.type = PORT_TEGRA,
+ .handle_break = tegra_serial_handle_break,
.iotype = UPIO_MEM,
.regshift = 2,
.uartclk = 216000000,
@@ -65,6 +66,7 @@ static struct plat_serial8250_port debug_uart_platform_data[] = {
.irq = INT_UARTC,
.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_TYPE,
.type = PORT_TEGRA,
+ .handle_break = tegra_serial_handle_break,
.iotype = UPIO_MEM,
.regshift = 2,
.uartclk = 216000000,
diff --git a/arch/arm/mach-tegra/board-seaboard.c b/arch/arm/mach-tegra/board-seaboard.c
index ebac65f..11b8d27 100644
--- a/arch/arm/mach-tegra/board-seaboard.c
+++ b/arch/arm/mach-tegra/board-seaboard.c
@@ -47,6 +47,7 @@ static struct plat_serial8250_port debug_uart_platform_data[] = {
/* Memory and IRQ filled in before registration */
.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_TYPE,
.type = PORT_TEGRA,
+ .handle_break = tegra_serial_handle_break,
.iotype = UPIO_MEM,
.regshift = 2,
.uartclk = 216000000,
diff --git a/arch/arm/mach-tegra/board-trimslice.c b/arch/arm/mach-tegra/board-trimslice.c
index cd52820..e7763ff 100644
--- a/arch/arm/mach-tegra/board-trimslice.c
+++ b/arch/arm/mach-tegra/board-trimslice.c
@@ -48,6 +48,7 @@ static struct plat_serial8250_port debug_uart_platform_data[] = {
.irq = INT_UARTA,
.flags = UPF_BOOT_AUTOCONF | UPF_FIXED_TYPE,
.type = PORT_TEGRA,
+ .handle_break = tegra_serial_handle_break,
.iotype = UPIO_MEM,
.regshift = 2,
.uartclk = 216000000,
diff --git a/arch/arm/mach-tegra/devices.c b/arch/arm/mach-tegra/devices.c
index 7a2a02d..d9301bc 100644
--- a/arch/arm/mach-tegra/devices.c
+++ b/arch/arm/mach-tegra/devices.c
@@ -17,11 +17,13 @@
*/
+#include <linux/delay.h>
#include <linux/resource.h>
#include <linux/platform_device.h>
#include <linux/dma-mapping.h>
#include <linux/fsl_devices.h>
#include <linux/serial_8250.h>
+#include <linux/serial_reg.h>
#include <linux/i2c-tegra.h>
#include <linux/platform_data/tegra_usb.h>
#include <asm/pmu.h>
@@ -704,3 +706,19 @@ struct platform_device tegra_pcm_device = {
.name = "tegra-pcm-audio",
.id = -1,
};
+
+void tegra_serial_handle_break(struct uart_port *p)
+{
+ unsigned int status, tmout = 10000;
+
+ do {
+ status = p->serial_in(p, UART_LSR);
+ if (status & (UART_LSR_FIFOE | UART_LSR_BRK_ERROR_BITS))
+ status = p->serial_in(p, UART_RX);
+ else
+ break;
+ if (--tmout == 0)
+ break;
+ udelay(1);
+ } while (1);
+}
diff --git a/arch/arm/mach-tegra/devices.h b/arch/arm/mach-tegra/devices.h
index 873ecb2..c2253be 100644
--- a/arch/arm/mach-tegra/devices.h
+++ b/arch/arm/mach-tegra/devices.h
@@ -20,6 +20,7 @@
#define __MACH_TEGRA_DEVICES_H
#include <linux/platform_device.h>
+#include <linux/serial_8250.h>
extern struct platform_device tegra_gpio_device;
extern struct platform_device tegra_pinmux_device;
@@ -49,4 +50,5 @@ extern struct platform_device tegra_i2s_device2;
extern struct platform_device tegra_das_device;
extern struct platform_device tegra_pcm_device;
+void tegra_serial_handle_break(struct uart_port *p);
#endif
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 0e2c703..d1e0b3d 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -1353,27 +1353,6 @@ static void serial8250_enable_ms(struct uart_port *port)
}
/*
- * Clear the Tegra rx fifo after a break
- *
- * FIXME: This needs to become a port specific callback once we have a
- * framework for this
- */
-static void clear_rx_fifo(struct uart_8250_port *up)
-{
- unsigned int status, tmout = 10000;
- do {
- status = serial_in(up, UART_LSR);
- if (status & (UART_LSR_FIFOE | UART_LSR_BRK_ERROR_BITS))
- status = serial_in(up, UART_RX);
- else
- break;
- if (--tmout == 0)
- break;
- udelay(1);
- } while (1);
-}
-
-/*
* serial8250_rx_chars: processes according to the passed in LSR
* value, and returns the remaining LSR bits not handled
* by this Rx routine.
@@ -1406,20 +1385,10 @@ serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
up->lsr_saved_flags = 0;
if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
- /*
- * For statistics only
- */
if (lsr & UART_LSR_BI) {
lsr &= ~(UART_LSR_FE | UART_LSR_PE);
up->port.icount.brk++;
/*
- * If tegra port then clear the rx fifo to
- * accept another break/character.
- */
- if (up->port.type == PORT_TEGRA)
- clear_rx_fifo(up);
-
- /*
* We do the SysRQ and SAK checking
* here because otherwise the break
* may get masked by ignore_status_mask
@@ -3047,6 +3016,7 @@ static int __devinit serial8250_probe(struct platform_device *dev)
port.serial_in = p->serial_in;
port.serial_out = p->serial_out;
port.handle_irq = p->handle_irq;
+ port.handle_break = p->handle_break;
port.set_termios = p->set_termios;
port.pm = p->pm;
port.dev = &dev->dev;
@@ -3219,6 +3189,8 @@ int serial8250_register_port(struct uart_port *port)
uart->port.set_termios = port->set_termios;
if (port->pm)
uart->port.pm = port->pm;
+ if (port->handle_break)
+ uart->port.handle_break = port->handle_break;
if (serial8250_isa_config != NULL)
serial8250_isa_config(0, &uart->port,
diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
index 8f012f8..a522fd9 100644
--- a/include/linux/serial_8250.h
+++ b/include/linux/serial_8250.h
@@ -38,6 +38,7 @@ struct plat_serial8250_port {
int (*handle_irq)(struct uart_port *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned old);
+ void (*handle_break)(struct uart_port *);
};
/*
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index cbb3d12..82f6147 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -308,6 +308,7 @@ struct uart_port {
int (*handle_irq)(struct uart_port *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int old);
+ void (*handle_break)(struct uart_port *);
unsigned int irq; /* irq number */
unsigned long irqflags; /* irq flags */
unsigned int uartclk; /* base uart clock */
@@ -521,6 +522,10 @@ uart_handle_sysrq_char(struct uart_port *port, unsigned int ch)
static inline int uart_handle_break(struct uart_port *port)
{
struct uart_state *state = port->state;
+
+ if (port->handle_break)
+ port->handle_break(port);
+
#ifdef SUPPORT_SYSRQ
if (port->cons && port->cons->index == port->line) {
if (!port->sysrq) {
^ permalink raw reply related
* [PATCH 5/6] serial/8250_pci: Clear FIFOs for Intel ME Serial Over Lan device on BI
From: Dan Williams @ 2012-04-06 18:50 UTC (permalink / raw)
To: gregkh
Cc: Sudhakar Mamillapalli, Nhan H Mai, linux-kernel, linux-serial,
Alan Cox
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
From: Sudhakar Mamillapalli <sudhakar@fb.com>
When using Serial Over Lan (SOL) over the virtual serial port in a Intel
management engine (ME) device, on device reset the serial FIFOs need to
be cleared to keep the FIFO indexes in-sync between the host and the
engine.
On a reset the serial device assertes BI, so using that as a cue FIFOs
are cleared. So for this purpose a new handle_break callback has been
added. One other problem is that the serial registers might temporarily
go to 0 on reset of this device. So instead of using the IER register
read, if 0 returned use the ier value in uart_8250_port. This is hidden
under a custom serial_in.
Cc: Alan Cox <alan@linux.intel.com>
Cc: Nhan H Mai <nhan.h.mai@intel.com>
Signed-off-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/tty/serial/8250/8250.c | 10 +++++++++
drivers/tty/serial/8250/8250.h | 2 ++
drivers/tty/serial/8250/8250_pci.c | 39 ++++++++++++++++++++++++++++++++++++
3 files changed, 51 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index d1e0b3d..3fc7619 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -590,6 +590,16 @@ static void serial8250_clear_fifos(struct uart_8250_port *p)
}
}
+void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p)
+{
+ unsigned char fcr;
+
+ serial8250_clear_fifos(p);
+ fcr = uart_config[p->port.type].fcr;
+ serial_out(p, UART_FCR, fcr);
+}
+EXPORT_SYMBOL_GPL(serial8250_clear_and_reinit_fifos);
+
/*
* IER sleep support. UARTs which have EFRs need the "extended
* capability" bit enabled. Note that on XR16C850s, we need to
diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
index ae027be..5efa6c5 100644
--- a/drivers/tty/serial/8250/8250.h
+++ b/drivers/tty/serial/8250/8250.h
@@ -86,6 +86,8 @@ struct serial8250_config {
#define SERIAL8250_SHARE_IRQS 0
#endif
+void serial8250_clear_and_reinit_fifos(struct uart_8250_port *p);
+
#if defined(__alpha__) && !defined(CONFIG_PCI)
/*
* Digital did something really horribly wrong with the OUT1 and OUT2
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 858dca8..024551a 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -17,6 +17,7 @@
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/tty.h>
+#include <linux/serial_reg.h>
#include <linux/serial_core.h>
#include <linux/8250_pci.h>
#include <linux/bitops.h>
@@ -1092,11 +1093,49 @@ static int skip_tx_en_setup(struct serial_private *priv,
return pci_default_setup(priv, board, port, idx);
}
+static void kt_handle_break(struct uart_port *p)
+{
+ struct uart_8250_port *up =
+ container_of(p, struct uart_8250_port, port);
+ /*
+ * On receipt of a BI, serial device in Intel ME (Intel
+ * management engine) needs to have its fifos cleared for sane
+ * SOL (Serial Over Lan) output.
+ */
+ serial8250_clear_and_reinit_fifos(up);
+}
+
+static unsigned int kt_serial_in(struct uart_port *p, int offset)
+{
+ struct uart_8250_port *up =
+ container_of(p, struct uart_8250_port, port);
+ unsigned int val;
+
+ /*
+ * When the Intel ME (management engine) gets reset its serial
+ * port registers could return 0 momentarily. Functions like
+ * serial8250_console_write, read and save the IER, perform
+ * some operation and then restore it. In order to avoid
+ * setting IER register inadvertently to 0, if the value read
+ * is 0, double check with ier value in uart_8250_port and use
+ * that instead. up->ier should be the same value as what is
+ * currently configured.
+ */
+ val = inb(p->iobase + offset);
+ if (offset == UART_IER) {
+ if (val == 0)
+ val = up->ier;
+ }
+ return val;
+}
+
static int kt_serial_setup(struct serial_private *priv,
const struct pciserial_board *board,
struct uart_port *port, int idx)
{
port->flags |= UPF_BUG_THRE;
+ port->serial_in = kt_serial_in;
+ port->handle_break = kt_handle_break;
return skip_tx_en_setup(priv, board, port, idx);
}
^ permalink raw reply related
* [RFC PATCH 6/6] serial/8250_pci: fix suspend/resume vs init/exit quirks
From: Dan Williams @ 2012-04-06 18:50 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, linux-serial, Alan Cox
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
Commit e86ff4a6 "serial/8250_pci: init-quirk msi support for kt serial
controller" introduced a regression in suspend/resume by causing msi's
to be enabled twice without an intervening disable.
That patch has since been reverted, but by inspection it seems that
pciserial_suspend_ports() should be invoking .exit() quirks to release
resources acquired during .init().
Cc: Alan Cox <alan@linux.intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
drivers/tty/serial/8250/8250_pci.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 024551a..24ea98c 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -2814,6 +2814,12 @@ void pciserial_suspend_ports(struct serial_private *priv)
for (i = 0; i < priv->nr; i++)
if (priv->line[i] >= 0)
serial8250_suspend_port(priv->line[i]);
+
+ /*
+ * Ensure that every init quirk is properly torn down
+ */
+ if (priv->quirk->exit)
+ priv->quirk->exit(priv->dev);
}
EXPORT_SYMBOL_GPL(pciserial_suspend_ports);
^ permalink raw reply related
* Re: [PATCH 1/6] Revert "serial/8250_pci: init-quirk msi support for kt serial controller"
From: Alan Cox @ 2012-04-06 19:05 UTC (permalink / raw)
To: Dan Williams
Cc: gregkh, Sudhakar Mamillapalli, linux-kernel, stable, linux-serial,
Nhan H Mai
In-Reply-To: <20120406184934.22088.63056.stgit@dwillia2-linux.jf.intel.com>
On Fri, 06 Apr 2012 11:49:37 -0700
Dan Williams <dan.j.williams@intel.com> wrote:
> This reverts commit e86ff4a63c9fdd875ba8492577cd1ad2252f525c.
Series
Acked-by: Alan Cox <alan@linux.intel.com>
^ permalink raw reply
* Re: [PATCH 0/6] rework quirks for the "kt" serial port
From: Greg KH @ 2012-04-06 19:14 UTC (permalink / raw)
To: Dan Williams; +Cc: linux-kernel, linux-serial, alan
In-Reply-To: <20120406184346.21744.18004.stgit@dwillia2-linux.jf.intel.com>
On Fri, Apr 06, 2012 at 11:44:22AM -0700, Dan Williams wrote:
> The kt serial port intermittently reports unreliable data in its IIR
> register and needs to reset fifos to stay in sync with firmware when the
> device is reset.
>
> The IIR fixes that went into 3.3 were not complete so patch 3 replaces
> them with an explicit way to turn on UART_BUG_THRE (i.e. the "I don't
> trust my iir" flag).
>
So the first 3 patches should go into 3.4 (and 3.3-stable) kernels,
right?
> A new fix for resetting the fifos on device reset is patch 5. Alan
> asked to make this a new quirk rather than add more workarounds to the
> core (patch 4).
These can wait for 3.5?
> Patch 6 is an untested RFC / RFT from anyone that has a serial
> suspend/resume use case with an 'init' quirk that has resources that
> need de-allocation at 'exit'.
And I'll hold off on this one entirely for now, right?
greg k-h
^ permalink raw reply
* Re: [PATCH 0/6] rework quirks for the "kt" serial port
From: Williams, Dan J @ 2012-04-06 19:49 UTC (permalink / raw)
To: Greg KH; +Cc: linux-kernel, linux-serial, alan
In-Reply-To: <20120406191451.GA14211@kroah.com>
On Fri, Apr 6, 2012 at 12:14 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Apr 06, 2012 at 11:44:22AM -0700, Dan Williams wrote:
>> The kt serial port intermittently reports unreliable data in its IIR
>> register and needs to reset fifos to stay in sync with firmware when the
>> device is reset.
>>
>> The IIR fixes that went into 3.3 were not complete so patch 3 replaces
>> them with an explicit way to turn on UART_BUG_THRE (i.e. the "I don't
>> trust my iir" flag).
>>
>
> So the first 3 patches should go into 3.4 (and 3.3-stable) kernels,
> right?
Yes, since this fixes the workaround that went into 3.3 properly and
addresses a regression.
>> A new fix for resetting the fifos on device reset is patch 5. Alan
>> asked to make this a new quirk rather than add more workarounds to the
>> core (patch 4).
>
> These can wait for 3.5?
I think they can wait especially because of the cross tree changes.
>> Patch 6 is an untested RFC / RFT from anyone that has a serial
>> suspend/resume use case with an 'init' quirk that has resources that
>> need de-allocation at 'exit'.
>
> And I'll hold off on this one entirely for now, right?
Yeah, if anyone runs into a similar problem with serial suspend /
resume they may want to take a look at this patch. But for now I
don't have a valid test case for it.
--
Dan
^ permalink raw reply
* Re: [PATCH 4/6] tegra, serial8250: add ->handle_break() uart_port op
From: Stephen Warren @ 2012-04-06 21:01 UTC (permalink / raw)
To: Dan Williams
Cc: gregkh, Sudhakar Mamillapalli, linux-kernel, linux-serial,
Colin Cross, Olof Johansson, Nhan H Mai, Alan Cox, alan
In-Reply-To: <20120406184956.22088.85217.stgit@dwillia2-linux.jf.intel.com>
On 04/06/2012 12:49 PM, Dan Williams wrote:
> The "KT" serial port has another use case for a "received break" quirk,
> so before adding another special case to the 8250 core take this
> opportunity to push such quirks out of the core and into a uart_port op.
This doesn't seem quite right. Why do the board files have to set up
this .handle_break function; they're already setting .type=PORT_TEGRA,
which should be enough to drive the setup of any required quirks.
If plat_serial8250_port must contain this field, then
drivers/tty/serial/of_serial.c needs a similar change so that this all
works when booting using device tree.
I'm not sure what the implication is of moving the call to clr_fifo()
into uart_handle_break(). What's the benefit of one location over the other?
If the callback function is to no longer live in 8250.c itself,
arch/arm/mach-tegra/devices.c isn't logically a good place to put it,
and that file will be going away once we get rid of all the board files
and move solely to device tree.
^ permalink raw reply
* Re: [PATCH 4/6] tegra, serial8250: add ->handle_break() uart_port op
From: Williams, Dan J @ 2012-04-06 21:28 UTC (permalink / raw)
To: Stephen Warren
Cc: gregkh, Sudhakar Mamillapalli, linux-kernel, linux-serial,
Colin Cross, Olof Johansson, Nhan H Mai, Alan Cox, alan
In-Reply-To: <4F7F59B2.4090400@wwwdotorg.org>
On Fri, Apr 6, 2012 at 2:01 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/06/2012 12:49 PM, Dan Williams wrote:
>> The "KT" serial port has another use case for a "received break" quirk,
>> so before adding another special case to the 8250 core take this
>> opportunity to push such quirks out of the core and into a uart_port op.
>
> This doesn't seem quite right. Why do the board files have to set up
> this .handle_break function; they're already setting .type=PORT_TEGRA,
> which should be enough to drive the setup of any required quirks.
Because struct serial8250_config does not convey any uart_port ops.
> If plat_serial8250_port must contain this field, then
> drivers/tty/serial/of_serial.c needs a similar change so that this all
> works when booting using device tree.
>
> I'm not sure what the implication is of moving the call to clr_fifo()
> into uart_handle_break(). What's the benefit of one location over the other?
This was the location where the core was already doing it's break
handling, so it made sense to check here if the device had any quirks
to run. There shouldn't be any implications because the core was
already doing clear_rx_fifo() immediately before calling
uart_handle_break. Here is the relevant hunk with a bit more context:
@@ -1399,34 +1378,24 @@ serial8250_rx_chars(struct uart_8250_port *up,
unsigned char lsr)
*/
ch = 0;
flag = TTY_NORMAL;
up->port.icount.rx++;
lsr |= up->lsr_saved_flags;
up->lsr_saved_flags = 0;
if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
- /*
- * For statistics only
- */
if (lsr & UART_LSR_BI) {
lsr &= ~(UART_LSR_FE | UART_LSR_PE);
up->port.icount.brk++;
/*
- * If tegra port then clear the rx fifo to
- * accept another break/character.
- */
- if (up->port.type == PORT_TEGRA)
- clear_rx_fifo(up);
-
- /*
* We do the SysRQ and SAK checking
* here because otherwise the break
* may get masked by ignore_status_mask
* or read_status_mask.
*/
if (uart_handle_break(&up->port))
goto ignore_char;
} else if (lsr & UART_LSR_PE)
up->port.icount.parity++;
else if (lsr & UART_LSR_FE)
> If the callback function is to no longer live in 8250.c itself,
> arch/arm/mach-tegra/devices.c isn't logically a good place to put it,
> and that file will be going away once we get rid of all the board files
> and move solely to device tree.
Can you help me with an incremental patch to fix this up? I can
muddle my way through Tegra internals, but I already missed the
of_serial.c hook.
--
Dan
^ permalink raw reply
* Re: [PATCH 4/6] tegra, serial8250: add ->handle_break() uart_port op
From: Stephen Warren @ 2012-04-06 21:56 UTC (permalink / raw)
To: Williams, Dan J
Cc: gregkh, Sudhakar Mamillapalli, linux-kernel, linux-serial,
Colin Cross, Olof Johansson, Nhan H Mai, Alan Cox, alan
In-Reply-To: <CABE8wws23LSLBxz34LYuyYtXMCG+-YTShhojXdeVkfiOvHWseQ@mail.gmail.com>
On 04/06/2012 03:28 PM, Williams, Dan J wrote:
> On Fri, Apr 6, 2012 at 2:01 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 04/06/2012 12:49 PM, Dan Williams wrote:
>>> The "KT" serial port has another use case for a "received break" quirk,
>>> so before adding another special case to the 8250 core take this
>>> opportunity to push such quirks out of the core and into a uart_port op.
>>
>> This doesn't seem quite right. Why do the board files have to set up
>> this .handle_break function; they're already setting .type=PORT_TEGRA,
>> which should be enough to drive the setup of any required quirks.
>
> Because struct serial8250_config does not convey any uart_port ops.
But couldn't it be enhanced to do so, just like this patch added a field
to struct uart_port for this? If you went this route, then the change
would be entirely isolated within 8250.c, so you could drop all the
arch/arm/mach-tegra changes, and also not need to update of_serial.c.
>> I'm not sure what the implication is of moving the call to clr_fifo()
>> into uart_handle_break(). What's the benefit of one location over the other?
>
> This was the location where the core was already doing it's break
> handling, so it made sense to check here if the device had any quirks
> to run. There shouldn't be any implications because the core was
> already doing clear_rx_fifo() immediately before calling
> uart_handle_break. Here is the relevant hunk with a bit more context:
Ah OK, that part seems fine then.
^ permalink raw reply
* Re: [PATCH 4/6] tegra, serial8250: add ->handle_break() uart_port op
From: Williams, Dan J @ 2012-04-06 22:25 UTC (permalink / raw)
To: Stephen Warren
Cc: gregkh, Sudhakar Mamillapalli, linux-kernel, linux-serial,
Colin Cross, Olof Johansson, Nhan H Mai, Alan Cox, alan
In-Reply-To: <4F7F66A4.8070701@wwwdotorg.org>
On Fri, Apr 6, 2012 at 2:56 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 04/06/2012 03:28 PM, Williams, Dan J wrote:
>> On Fri, Apr 6, 2012 at 2:01 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 04/06/2012 12:49 PM, Dan Williams wrote:
>>>> The "KT" serial port has another use case for a "received break" quirk,
>>>> so before adding another special case to the 8250 core take this
>>>> opportunity to push such quirks out of the core and into a uart_port op.
>>>
>>> This doesn't seem quite right. Why do the board files have to set up
>>> this .handle_break function; they're already setting .type=PORT_TEGRA,
>>> which should be enough to drive the setup of any required quirks.
>>
>> Because struct serial8250_config does not convey any uart_port ops.
>
> But couldn't it be enhanced to do so, just like this patch added a field
> to struct uart_port for this? If you went this route, then the change
> would be entirely isolated within 8250.c, so you could drop all the
> arch/arm/mach-tegra changes, and also not need to update of_serial.c.
That's kind of the point, it seems the goal of serial port hardware
design is to find new and interesting ways to make them fail. Putting
everyone's fail in 8250.c is messy so when we tried to add another
"handle_break" quirk we were rightly asked to make the core interface
generic and push the logic out of the core.
I don't see how moving quirk routines into serial8250_config makes
this any cleaner, we'd either be left with two methods for specifying
quirks, or requiring custom serial8250_configs for drivers that only
want to set one of the ops.
Hmm, is there any precendent for an openfirmware serial port that
needs one of these custom ops?
unsigned int (*serial_in)(struct uart_port *, int);
void (*serial_out)(struct uart_port *, int, int);
void (*set_termios)(struct uart_port *,
struct ktermios *new,
struct ktermios *old);
int (*handle_irq)(struct uart_port *);
void (*pm)(struct uart_port *, unsigned int state,
unsigned int old);
void (*handle_break)(struct uart_port *);
...I would be surprised if this is the first time this has come up
given the prevalence of broken uarts.
--
Dan
^ permalink raw reply
* Re: Suspend broken on 3.3?
From: Paul Walmsley @ 2012-04-09 16:01 UTC (permalink / raw)
To: Kevin Hilman
Cc: Raja\, Govindraj, Joe Woodward, linux-omap, Felipe Balbi, neilb,
linux-serial
In-Reply-To: <87zkalc75h.fsf@ti.com>
cc linux-serial
Hi
On Mon, 9 Apr 2012, Kevin Hilman wrote:
> Presumably, if a user disables UART wakeups, it means either 1) that
> UART is not used
#1 seems easy enough to handle without the use of power/wakeup. If there
are no users of the TTY, then the driver can simply not configure hardware
wakeups.
> or 2) performance is not critical.
Can you think of a use-case for #2?
> IMO, we simply need to ensure that the defaults are correct. When UARTs
> are initialized/enabled wakeups should be enabled by default. The user
> can then override this if desired, and will obviously see a performance
> impact. But that's what happens with wakeups disabled.
I don't understand why a user would ever want to disable dynamic wakeups
on an in-use serial port via the sysfs power/wakeup file. (Disabling
wakeups from suspend is a different matter, of course.) The OMAP UART
driver needs hardware wakeups to function for FIFO drain wakeups, etc.
So to me it really doesn't make sense to disable those types of wakeup
events, ever. But maybe you know of some use-case that I don't?
If the user just wants a transmit-only serial port, they could use the
termios CREAD flag as Neil mentioned a few months ago, and the driver
could disable wakeups on incoming RXD (modulo any active flow control of
course).
- Paul
^ permalink raw reply
* Re: [PATCH 1/2] 8250_pci: Replace commits 448ac154 and e86ff4a
From: Greg KH @ 2012-04-09 16:47 UTC (permalink / raw)
To: Alan Cox
Cc: sudhakar, linux-serial, alan, dan.j.williams, nhan.h.mai,
linux-kernel
In-Reply-To: <20120403215832.0520f93d@pyramind.ukuu.org.uk>
On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
> On Tue, 3 Apr 2012 12:43:20 -0700
> sudhakar <sudhakar@fb.com> wrote:
>
> >
> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
> >
> >
> > The following commits don't completely fix the KT serial missing
> > interrupt problems. So they have been replaced instead by
> > enabling serial backup timer. The iir-once + msi tried approach tried
> > to enforce one read of the iir per THRE event, but its defeated by any
> > other event occurring at the wrong time, particularly MSR events.
> >
> > 448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller
> > e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller
>
> These are already in tree. If you are reversing them and adding a
> different fix then please post the series as
>
> - two revert patches exactly reverting the two commits
> - a follow on patch or two adding the replacement
>
> That makes it much easier to review and much more obvious. It also helps
> make testing simpler.
I agree, Sudhakar, can you please redo these 2 patches?
greg k-h
^ permalink raw reply
* Re: [PATCH 1/2] 8250_pci: Replace commits 448ac154 and e86ff4a
From: Williams, Dan J @ 2012-04-09 16:53 UTC (permalink / raw)
To: Greg KH; +Cc: Alan Cox, sudhakar, linux-serial, alan, nhan.h.mai, linux-kernel
In-Reply-To: <20120409164759.GA8800@kroah.com>
On Mon, Apr 9, 2012 at 9:47 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
>> On Tue, 3 Apr 2012 12:43:20 -0700
>> sudhakar <sudhakar@fb.com> wrote:
>>
>> >
>> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
>> >
>> >
>> > The following commits don't completely fix the KT serial missing
>> > interrupt problems. So they have been replaced instead by
>> > enabling serial backup timer. The iir-once + msi tried approach tried
>> > to enforce one read of the iir per THRE event, but its defeated by any
>> > other event occurring at the wrong time, particularly MSR events.
>> >
>> > 448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller
>> > e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller
>>
>> These are already in tree. If you are reversing them and adding a
>> different fix then please post the series as
>>
>> - two revert patches exactly reverting the two commits
>> - a follow on patch or two adding the replacement
>>
>> That makes it much easier to review and much more obvious. It also helps
>> make testing simpler.
>
> I agree, Sudhakar, can you please redo these 2 patches?
>
Hi Greg,
This was in the series:
[PATCH 0/6] rework quirks for the "kt" serial port [1]
...but I have just noticed it collides with current git, and I also
need to address Stephen's comments for Tegra. Expect a v2 shortly.
--
Dan
[1]: http://marc.info/?l=linux-serial&m=133373702606784&w=2
--
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
^ permalink raw reply
* Re: Suspend broken on 3.3?
From: Kevin Hilman @ 2012-04-09 17:10 UTC (permalink / raw)
To: Paul Walmsley
Cc: Raja, Govindraj, Joe Woodward, linux-omap, Felipe Balbi, neilb,
linux-serial
In-Reply-To: <alpine.DEB.2.00.1204090944191.6640@utopia.booyaka.com>
Paul Walmsley <paul@pwsan.com> writes:
[...]
> I don't understand why a user would ever want to disable dynamic wakeups
> on an in-use serial port via the sysfs power/wakeup file. (Disabling
> wakeups from suspend is a different matter, of course.) The OMAP UART
> driver needs hardware wakeups to function for FIFO drain wakeups, etc.
> So to me it really doesn't make sense to disable those types of wakeup
> events, ever. But maybe you know of some use-case that I don't?
No, I don't have a use-case in mind.
The more I try to remember why we added support to use the sysfs wakeup
attribute to manage idle, the more I think we can probably drop it now.
IIRC, it was added because on most boards we used to blindly initialize
all the UARTs, including default wakeup settings (we still do this on
many boards.)
However, now that we have a real PM-aware driver for OMAP UARTs, this
should all be handled from the driver itself, so the sysfs wakeup
attribute should go back to only managing wakeups from suspend and
wakeups during idle should always be on for in-use UARTs.
Kevin
^ permalink raw reply
* Re: [PATCH] pch_uart: Fix dma channel unallocated issue
From: Greg Kroah-Hartman @ 2012-04-09 17:44 UTC (permalink / raw)
To: Tomoya MORINAGA
Cc: Alan Cox, linux-serial, linux-kernel, qi.wang, yong.y.wang,
joel.clark, kok.howg.ewe
In-Reply-To: <1332842162-26113-1-git-send-email-tomoya.rohm@gmail.com>
On Tue, Mar 27, 2012 at 06:56:02PM +0900, Tomoya MORINAGA wrote:
> This driver anticipates pch_uart_verify_port() is not called
> during installation.
> However, actually pch_uart_verify_port() is called during
> installation.
> As a result, memory access violation occurs like below.
>
> 0. initial value: use_dma=0
> 1. starup()
> - dma channel is not allocated because use_dma=0
> 2. pch_uart_verify_port()
> - Set use_dma=1
> 3. UART processing acts DMA mode because use_dma=1
> - memory access violation occurs!
>
> This patch fixes the issue.
>
> Solution:
> Whenever pch_uart_verify_port() is called and then
> dma channel is not allocated, the channel should be allocated.
>
> Signed-off-by: Tomoya MORINAGA <tomoya.rohm@gmail.com>
> ---
> drivers/tty/serial/pch_uart.c | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
This patch does not apply to 3.4-rc2, so I can not apply it, sorry.
Care to redo it?
greg k-h
^ permalink raw reply
* Re: [PATCH 1/2] 8250_pci: Replace commits 448ac154 and e86ff4a
From: Greg KH @ 2012-04-09 17:48 UTC (permalink / raw)
To: Williams, Dan J
Cc: Alan Cox, sudhakar, linux-serial, alan, nhan.h.mai, linux-kernel
In-Reply-To: <CABE8wwsHZLiihEReRQwkk+SufkmBw2W4n-WibQgS-rEOnKNGMA@mail.gmail.com>
On Mon, Apr 09, 2012 at 09:53:47AM -0700, Williams, Dan J wrote:
> On Mon, Apr 9, 2012 at 9:47 AM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Apr 03, 2012 at 09:58:32PM +0100, Alan Cox wrote:
> >> On Tue, 3 Apr 2012 12:43:20 -0700
> >> sudhakar <sudhakar@fb.com> wrote:
> >>
> >> >
> >> > From: Sudhakar Mamillapalli <sudhakar@fb.com>
> >> >
> >> >
> >> > The following commits don't completely fix the KT serial missing
> >> > interrupt problems. So they have been replaced instead by
> >> > enabling serial backup timer. The iir-once + msi tried approach tried
> >> > to enforce one read of the iir per THRE event, but its defeated by any
> >> > other event occurring at the wrong time, particularly MSR events.
> >> >
> >> > 448ac15 serial/8250_pci: setup-quirk workaround for the kt serial controller
> >> > e86ff4a serial/8250_pci: init-quirk msi support for kt serial controller
> >>
> >> These are already in tree. If you are reversing them and adding a
> >> different fix then please post the series as
> >>
> >> - two revert patches exactly reverting the two commits
> >> - a follow on patch or two adding the replacement
> >>
> >> That makes it much easier to review and much more obvious. It also helps
> >> make testing simpler.
> >
> > I agree, Sudhakar, can you please redo these 2 patches?
> >
>
> Hi Greg,
>
> This was in the series:
>
> [PATCH 0/6] rework quirks for the "kt" serial port [1]
>
> ...but I have just noticed it collides with current git, and I also
> need to address Stephen's comments for Tegra. Expect a v2 shortly.
Ok, thanks. Note, I just applied the first 3 to go to Linus soon, those
didn't need to be reworked, right?
greg k-h
--
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox