* [PATCH v2 1/2] serial: pl010: Move uart_register_driver call to device probe
From: Sjoerd Simons @ 2017-04-20 12:13 UTC (permalink / raw)
To: linux-serial
Cc: linux-renesas-soc, Greg Kroah-Hartman, linux-kernel, Russell King,
Jiri Slaby
In-Reply-To: <20170420121301.382-1-sjoerd.simons@collabora.co.uk>
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically
happen during device probe call.
In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with overlapping major/minor numbers are
included.
Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
Changes in v2:
- Add locking to prevent issues when two probes run in parallel
drivers/tty/serial/amba-pl010.c | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index f2f251075109..24180adb1cbb 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -697,6 +697,7 @@ static struct console amba_console = {
#define AMBA_CONSOLE NULL
#endif
+static DEFINE_MUTEX(amba_reg_lock);
static struct uart_driver amba_reg = {
.owner = THIS_MODULE,
.driver_name = "ttyAM",
@@ -749,6 +750,19 @@ static int pl010_probe(struct amba_device *dev, const struct amba_id *id)
amba_ports[i] = uap;
amba_set_drvdata(dev, uap);
+
+ mutex_lock(&amba_reg_lock);
+ if (!amba_reg.state) {
+ ret = uart_register_driver(&amba_reg);
+ if (ret < 0) {
+ mutex_unlock(&amba_reg_lock);
+ dev_err(uap->port.dev,
+ "Failed to register AMBA-PL010 driver\n");
+ return ret;
+ }
+ }
+ mutex_unlock(&amba_reg_lock);
+
ret = uart_add_one_port(&amba_reg, &uap->port);
if (ret)
amba_ports[i] = NULL;
@@ -760,12 +774,18 @@ static int pl010_remove(struct amba_device *dev)
{
struct uart_amba_port *uap = amba_get_drvdata(dev);
int i;
+ bool busy = false;
uart_remove_one_port(&amba_reg, &uap->port);
for (i = 0; i < ARRAY_SIZE(amba_ports); i++)
if (amba_ports[i] == uap)
amba_ports[i] = NULL;
+ else if (amba_ports[i])
+ busy = true;
+
+ if (!busy)
+ uart_unregister_driver(&amba_reg);
return 0;
}
@@ -816,23 +836,14 @@ static struct amba_driver pl010_driver = {
static int __init pl010_init(void)
{
- int ret;
-
printk(KERN_INFO "Serial: AMBA driver\n");
- ret = uart_register_driver(&amba_reg);
- if (ret == 0) {
- ret = amba_driver_register(&pl010_driver);
- if (ret)
- uart_unregister_driver(&amba_reg);
- }
- return ret;
+ return amba_driver_register(&pl010_driver);
}
static void __exit pl010_exit(void)
{
amba_driver_unregister(&pl010_driver);
- uart_unregister_driver(&amba_reg);
}
module_init(pl010_init);
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/2] serial: sh-sci: Move uart_register_driver call to device probe
From: Sjoerd Simons @ 2017-04-20 12:13 UTC (permalink / raw)
To: linux-serial
Cc: linux-renesas-soc, Greg Kroah-Hartman, linux-kernel, Jiri Slaby
In-Reply-To: <20170420121301.382-1-sjoerd.simons@collabora.co.uk>
uart_register_driver call binds the driver to a specific device
node through tty_register_driver call. This should typically
happen during device probe call.
In a multiplatform scenario, it is possible that multiple serial
drivers are part of the kernel. Currently the driver registration fails
if multiple serial drivers with overlapping major/minor numbers are
included.
Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk>
---
Changes in v2:
- Add locking to prevent issues when two probes run in parallel
drivers/tty/serial/sh-sci.c | 26 +++++++++++++++-----------
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9a47cc4f16a2..f23254add9a7 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -2935,6 +2935,7 @@ static inline int sci_probe_earlyprintk(struct platform_device *pdev)
static const char banner[] __initconst = "SuperH (H)SCI(F) driver initialized";
+static DEFINE_MUTEX(sci_uart_registration_lock);
static struct uart_driver sci_uart_driver = {
.owner = THIS_MODULE,
.driver_name = "sci",
@@ -3063,6 +3064,16 @@ static int sci_probe_single(struct platform_device *dev,
return -EINVAL;
}
+ mutex_lock(&sci_uart_registration_lock);
+ if (!sci_uart_driver.state) {
+ ret = uart_register_driver(&sci_uart_driver);
+ if (ret) {
+ mutex_unlock(&sci_uart_registration_lock);
+ return ret;
+ }
+ }
+ mutex_unlock(&sci_uart_registration_lock);
+
ret = sci_init_single(dev, sciport, index, p, false);
if (ret)
return ret;
@@ -3186,24 +3197,17 @@ static struct platform_driver sci_driver = {
static int __init sci_init(void)
{
- int ret;
-
pr_info("%s\n", banner);
- ret = uart_register_driver(&sci_uart_driver);
- if (likely(ret == 0)) {
- ret = platform_driver_register(&sci_driver);
- if (unlikely(ret))
- uart_unregister_driver(&sci_uart_driver);
- }
-
- return ret;
+ return platform_driver_register(&sci_driver);
}
static void __exit sci_exit(void)
{
platform_driver_unregister(&sci_driver);
- uart_unregister_driver(&sci_uart_driver);
+
+ if (sci_uart_driver.state)
+ uart_unregister_driver(&sci_uart_driver);
}
#ifdef CONFIG_SERIAL_SH_SCI_CONSOLE
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] RFC: serial: core: Dynamic minor support
From: Greg Kroah-Hartman @ 2017-04-20 12:15 UTC (permalink / raw)
To: Sjoerd Simons; +Cc: linux-serial, Geert Uytterhoeven, linux-kernel, Jiri Slaby
In-Reply-To: <20170420120357.18317-1-sjoerd.simons@collabora.co.uk>
On Thu, Apr 20, 2017 at 02:03:57PM +0200, Sjoerd Simons wrote:
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 3fe56894974a..37014c70dd69 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -44,6 +44,32 @@
> */
> static DEFINE_MUTEX(port_mutex);
>
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> +/* List of uarts to allow for allocation of dynamic minor ranges*/
> +LIST_HEAD(dynamic_uarts);
> +DEFINE_MUTEX(dynamic_uarts_mutex);
Both of these should be static.
> +
> +static void
> +uart_setup_major_minor(struct uart_driver *drv)
> +{
> + int start = 0;
> + struct uart_driver *d;
> +
Why not init the list head in here too?
> + drv->major = LOW_DENSITY_UART_MAJOR;
> + mutex_lock(&dynamic_uarts_mutex);
> +
> + list_for_each_entry(d, &dynamic_uarts, dynamic_uarts) {
> + if (start + drv->nr < d->minor)
> + break;
> + start = d->minor + d->nr;
> + }
> + list_add_tail(&drv->dynamic_uarts, &d->dynamic_uarts);
> + drv->minor = start;
> +
> + mutex_unlock(&dynamic_uarts_mutex);
> +}
> +#endif
> +
> /*
> * lockdep: port->lock is initialized in two places, but we
> * want only one lock-class:
> @@ -2458,6 +2484,11 @@ int uart_register_driver(struct uart_driver *drv)
>
> BUG_ON(drv->state);
>
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> + INIT_LIST_HEAD(&drv->dynamic_uarts);
That line should be in the function:
> + uart_setup_major_minor(drv);
> +#endif
And then provide a "empty" function for this if the option is not
enabled, to keep the #ifdef out of the main flow of the code.
> +
> /*
> * Maybe we should be using a slab cache for this, especially if
> * we have a large number of ports to handle.
> @@ -2527,6 +2558,13 @@ void uart_unregister_driver(struct uart_driver *drv)
> put_tty_driver(p);
> for (i = 0; i < drv->nr; i++)
> tty_port_destroy(&drv->state[i].port);
> +
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> + mutex_lock(&dynamic_uarts_mutex);
> + list_del(&drv->dynamic_uarts);
> + mutex_unlock(&dynamic_uarts_mutex);
> +#endif
Make this a function, like above, and do the same thing to keep #ifdef
out of here.
> +
> kfree(drv->state);
> drv->state = NULL;
> drv->tty_driver = NULL;
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 58484fb35cc8..890bbefb3f90 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -31,6 +31,8 @@
> #include <linux/sysrq.h>
> #include <uapi/linux/serial_core.h>
>
> +#define LOW_DENSITY_UART_MAJOR 204
Where are you stealing this from?
> +
> #ifdef CONFIG_SERIAL_CORE_CONSOLE
> #define uart_console(port) \
> ((port)->cons && (port)->cons->index == (port)->line)
> @@ -313,6 +315,10 @@ struct uart_driver {
> */
> struct uart_state *state;
> struct tty_driver *tty_driver;
> +
> +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> + struct list_head dynamic_uarts;
> +#endif
Why not just always have this?
Nice first try though!
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] RFC: serial: core: Dynamic minor support
From: Sjoerd Simons @ 2017-04-20 12:44 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, Geert Uytterhoeven, linux-kernel, Jiri Slaby
In-Reply-To: <20170420121544.GA1131@kroah.com>
On Thu, 2017-04-20 at 14:15 +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 20, 2017 at 02:03:57PM +0200, Sjoerd Simons wrote:
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -31,6 +31,8 @@
> > #include <linux/sysrq.h>
> > #include <uapi/linux/serial_core.h>
> >
> > +#define LOW_DENSITY_UART_MAJOR 204
>
> Where are you stealing this from?
Heh, 204 is defined as the "Low-density serial ports" in
devices.txt. As documented in the commit message, i've repurposed that
for dynamic minors (if configured). Maybe it's better to request a new
major for this purpose? But then again, then just means 204 will go
unused when the option is on so...
Lots of drivers do have it as a hard-coded number, seemed sane to put
it a bit more central for some potential later cleanup in other
drivers.
>
> > +
> > #ifdef CONFIG_SERIAL_CORE_CONSOLE
> > #define uart_console(port) \
> > ((port)->cons && (port)->cons->index == (port)->line)
> > @@ -313,6 +315,10 @@ struct uart_driver {
> > */
> > struct uart_state *state;
> > struct tty_driver *tty_driver;
> > +
> > +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> > + struct list_head dynamic_uarts;
> > +#endif
>
> Why not just always have this?
Trying to save a few bytes if the option is unused; Maybe overdoing it
:)
> Nice first try though!
Thanks, If there are no big comments onthe general approach i'll respin
without RFC soonish addressing your other comments.
--
Sjoerd Simons
Collabora Ltd.
^ permalink raw reply
* Re: [PATCH] RFC: serial: core: Dynamic minor support
From: Greg Kroah-Hartman @ 2017-04-20 12:54 UTC (permalink / raw)
To: Sjoerd Simons; +Cc: linux-serial, Geert Uytterhoeven, linux-kernel, Jiri Slaby
In-Reply-To: <1492692258.27393.8.camel@collabora.co.uk>
On Thu, Apr 20, 2017 at 02:44:18PM +0200, Sjoerd Simons wrote:
> On Thu, 2017-04-20 at 14:15 +0200, Greg Kroah-Hartman wrote:
> > On Thu, Apr 20, 2017 at 02:03:57PM +0200, Sjoerd Simons wrote:
> > > --- a/include/linux/serial_core.h
> > > +++ b/include/linux/serial_core.h
> > > @@ -31,6 +31,8 @@
> > > #include <linux/sysrq.h>
> > > #include <uapi/linux/serial_core.h>
> > >
> > > +#define LOW_DENSITY_UART_MAJOR 204
> >
> > Where are you stealing this from?
>
> Heh, 204 is defined as the "Low-density serial ports" in
> devices.txt. As documented in the commit message, i've repurposed that
> for dynamic minors (if configured). Maybe it's better to request a new
> major for this purpose? But then again, then just means 204 will go
> unused when the option is on so...
>
> Lots of drivers do have it as a hard-coded number, seemed sane to put
> it a bit more central for some potential later cleanup in other
> drivers.
Ah, no, that's fine, didn't realize it wasn't already defined somewhere
already.
> > > +
> > > #ifdef CONFIG_SERIAL_CORE_CONSOLE
> > > #define uart_console(port) \
> > > ((port)->cons && (port)->cons->index == (port)->line)
> > > @@ -313,6 +315,10 @@ struct uart_driver {
> > > */
> > > struct uart_state *state;
> > > struct tty_driver *tty_driver;
> > > +
> > > +#ifdef CONFIG_SERIAL_DYNAMIC_MINORS
> > > + struct list_head dynamic_uarts;
> > > +#endif
> >
> > Why not just always have this?
>
> Trying to save a few bytes if the option is unused; Maybe overdoing it
> :)
>
> > Nice first try though!
>
> Thanks, If there are no big comments onthe general approach i'll respin
> without RFC soonish addressing your other comments.
Wait, can you use an idr for this instead of rolling your own search
logic for a series? I think the usb-serial core did this same sort of
functionality in the drivers/usb/serial/usb-serial.c:allocate_minors()
function using an idr. Try that instead here as well.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 3/4] tty/serdev: add serdev registration interface
From: Rob Herring @ 2017-04-20 17:52 UTC (permalink / raw)
To: Johan Hovold
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170411170731.4085-4-johan@kernel.org>
On Tue, Apr 11, 2017 at 12:07 PM, Johan Hovold <johan@kernel.org> wrote:
> Add a new interface for registering a serdev controller and clients, and
> a helper function to deregister serdev devices (or a tty device) that
> were previously registered using the new interface.
>
> Once every driver currently using the tty_port_register_device() helpers
> have been vetted and converted to use the new serdev registration
> interface (at least for deregistration), we can move serdev registration
> to the current helpers and get rid of the serdev-specific functions.
I don't really think this is necessary. While in theory any tty port
can work with serdev, the reality is it only ever going to be used
with a few. There's about 31 possible drivers. Of those, there's a
fair number that an attached device is not going to make sense (e.g.
ISDN CAPI, rfcomm?, goldfish, etc.). Second, right now serdev only
works with DT binding. There are only 3 drivers supporting DT:
serial_core, goldfish, and ehv_bytechan. Likely drivers I'm aware of
to use serdev in addition to serial_core are USB serial and greybus.
But for those, we currently only could support them if the whole bus
topology is described in DT. Otherwise, we first have to figure out
how to apply DT overlays to arbitrary devices not described in DT.
OTOH, we are at least explicit with what tty ports support serdev.
Rob
^ permalink raw reply
* Re: Commit 18e8cf159177 broke qemu-system-sh4 serial input.
From: Geert Uytterhoeven @ 2017-04-21 7:26 UTC (permalink / raw)
To: Rob Landley
Cc: John Paul Adrian Glaubitz, Linux-sh list, Ulrich Hecht,
Geert Uytterhoeven, Linux-Renesas, linux-serial@vger.kernel.org
In-Reply-To: <41d6bb75-5be0-c563-0569-ea37aab58f1e@landley.net>
Hi Rob,
CC Adrian, linux-serial, linux-renesas-soc
On Fri, Apr 21, 2017 at 8:21 AM, Rob Landley <rob@landley.net> wrote:
> In 4.11-rc7 the qemu serial console hangs after the first character you
> type. To reproduce, configure linux with this mini.config:
>
> CONFIG_CPU_SUBTYPE_SH7751R=y
> CONFIG_MMU=y
> CONFIG_MEMORY_START=0x0c000000
> CONFIG_VSYSCALL=y
> CONFIG_SH_FPU=y
> CONFIG_SH_RTS7751R2D=y
> CONFIG_RTS7751R2D_PLUS=y
> CONFIG_SERIAL_SH_SCI=y
> CONFIG_SERIAL_SH_SCI_CONSOLE=y
> CONFIG_EARLY_PRINTK=y
> CONFIG_BLK_DEV_INITRD=y"
> CONFIG_RD_GZIP=y
> CONFIG_BINFMT_ELF=y
> CONFIG_BINFMT_SCRIPT=y
> CONFIG_MISC_FILESYSTEMS=y
> CONFIG_DEVTMPFS=y
>
> Using "make ARCH=sh allnoconfig KCONFIG_ALLCONFIG=mini.conf", then build
> the result, then boot under qemu using the following command line:
>
> qemu-system-sh4 -M r2d -monitor null -serial null -serial stdio
> -nographic -no-reboot -append "panic=1 HOST=sh4 console=ttySC1 noiotrap"
> -kernel zImage -initrd sh4-linux-musl-root.cpio.gz
>
> You'll need a simple cpio.gz initramfs (I built one using
> https://github.com/landley/mkroot and the musl-cross-make cross
> compiler, the cpio.gz is ~450k or I'd attach it here).
>
> I bisected it to commit 18e8cf159177 back in February. If you do this
> with the revision _before_ that commit, typing "ls -l" and hitting enter
> works fine. Afterwards you get an "l" and then it hangs. (If type enough
> it'll eventually deal a burst of characters all at once, and then hang
> again.)
SH7751R has both SCI and SCIF ports. ttySC1 is the second (SCIF) port.
arch/sh/kernel/cpu/sh4/setup-sh7750.c registers a port with type PORT_SCIF,
so that should become SCIx_SH4_SCIF_REGTYPE. Hence I don't think the issue
is caused by changing fifosize to 64 for SCIx_SH7705_SCIF_REGTYPE.
My first guess is that qemu has a bug emulating the triggering.
According to the SH7751R datasheet, SCFCR does have the RTRG1 and RTRG0 bits.
I assume the problem goes away if you comment out the call to scif_set_rtrg()?
Does anyone have access to real hardware to try?
Adrian: does it work on your LANDISK, which also has SH7751R?
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: Commit 18e8cf159177 broke qemu-system-sh4 serial input.
From: John Paul Adrian Glaubitz @ 2017-04-21 7:50 UTC (permalink / raw)
To: Geert Uytterhoeven, Rob Landley
Cc: Linux-sh list, Ulrich Hecht, Geert Uytterhoeven, Linux-Renesas,
linux-serial@vger.kernel.org
In-Reply-To: <CAMuHMdWtBHTBCNo2-oO2Zv7O8McypcaEMh+_8DD+D1T7GTxAzw@mail.gmail.com>
On 04/21/2017 09:26 AM, Geert Uytterhoeven wrote:
> Does anyone have access to real hardware to try?
> Adrian: does it work on your LANDISK, which also has SH7751R?
I can give it a try but that will take a few days since I am currently
not at home where my LANDISK boxes are. I also had issues with the
IDE driver in the past but Sato-san gave me some tips regarding it.
However, Rich also has a LANDISK device that I donated for him, so if his
hardware is currently ready to use, he could give it a try.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [PATCH 3/4] tty/serdev: add serdev registration interface
From: Johan Hovold @ 2017-04-21 15:16 UTC (permalink / raw)
To: Rob Herring
Cc: Johan Hovold, Greg Kroah-Hartman, Jiri Slaby,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+zm3EoO_-9zyNp-Oe1LRrvVQuE6Q8u2wSxY0mbLt2Lpw@mail.gmail.com>
On Thu, Apr 20, 2017 at 12:52:50PM -0500, Rob Herring wrote:
> On Tue, Apr 11, 2017 at 12:07 PM, Johan Hovold <johan@kernel.org> wrote:
> > Add a new interface for registering a serdev controller and clients, and
> > a helper function to deregister serdev devices (or a tty device) that
> > were previously registered using the new interface.
> >
> > Once every driver currently using the tty_port_register_device() helpers
> > have been vetted and converted to use the new serdev registration
> > interface (at least for deregistration), we can move serdev registration
> > to the current helpers and get rid of the serdev-specific functions.
>
> I don't really think this is necessary. While in theory any tty port
> can work with serdev, the reality is it only ever going to be used
> with a few. There's about 31 possible drivers. Of those, there's a
> fair number that an attached device is not going to make sense (e.g.
> ISDN CAPI, rfcomm?, goldfish, etc.). Second, right now serdev only
> works with DT binding. There are only 3 drivers supporting DT:
> serial_core, goldfish, and ehv_bytechan.
There are also about ten PCI-based drivers (e.g. rocket.c), which, if
I'm not mistaken, could have an associated DT-node already today. And so
could the SPI-based ifx6x60 driver (modem).
> Likely drivers I'm aware of to use serdev in addition to serial_core
> are USB serial and greybus. But for those, we currently only could
> support them if the whole bus topology is described in DT. Otherwise,
> we first have to figure out how to apply DT overlays to arbitrary
> devices not described in DT.
There's also cdc-acm and possibly fw-serial (in staging).
And while our current USB device-tree implementation is limited to
describing USB devices, extending this to interfaces, which our USB
drivers bind to, should be easily done (I'm looking into it now).
There's also a comment in serdev about adding support for "ACPI and
platform" descriptions, and you mentioned being able to switch between
cdev and serdev as a possible future extension.
The point is that serdev currently hooks into the tty layer through a
generic function, and if and how a client can be described is just
an implementation detail. Rather than using such a large hammer, it
seems to me that enabling serdev on a per-driver basis after making sure
that nothing breaks (e.g. resources are released on deregistration) is
preferred.
> OTOH, we are at least explicit with what tty ports support serdev.
That would be another benefit.
Johan
^ permalink raw reply
* [PATCH] serial: pch_uart: use offset_in_page() macro
From: Geliang Tang @ 2017-04-22 1:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby; +Cc: Geliang Tang, linux-serial, linux-kernel
In-Reply-To: <4dbc77ccaaed98b183cf4dba58a4fa325fd65048.1492758503.git.geliangtang@gmail.com>
Use offset_in_page() macro instead of open-coding.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
drivers/tty/serial/pch_uart.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 42caccb..d3796dc 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -878,8 +878,7 @@ static int dma_handle_rx(struct eg20t_port *priv)
sg_dma_len(sg) = priv->trigger_level;
sg_set_page(&priv->sg_rx, virt_to_page(priv->rx_buf_virt),
- sg_dma_len(sg), (unsigned long)priv->rx_buf_virt &
- ~PAGE_MASK);
+ sg_dma_len(sg), offset_in_page(priv->rx_buf_virt));
sg_dma_address(sg) = priv->rx_buf_dma;
--
2.9.3
^ permalink raw reply related
* [PATCH] tty/serial: atmel: use offset_in_page() macro
From: Geliang Tang @ 2017-04-22 1:21 UTC (permalink / raw)
To: Richard Genoud, Greg Kroah-Hartman, Jiri Slaby
Cc: Geliang Tang, linux-serial, linux-kernel
In-Reply-To: <4dbc77ccaaed98b183cf4dba58a4fa325fd65048.1492758503.git.geliangtang@gmail.com>
Use offset_in_page() macro instead of open-coding.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
drivers/tty/serial/atmel_serial.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index c355ac9..f398db8 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -959,7 +959,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
sg_set_page(&atmel_port->sg_tx,
virt_to_page(port->state->xmit.buf),
UART_XMIT_SIZE,
- (unsigned long)port->state->xmit.buf & ~PAGE_MASK);
+ offset_in_page(port->state->xmit.buf));
nent = dma_map_sg(port->dev,
&atmel_port->sg_tx,
1,
@@ -1141,7 +1141,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
sg_set_page(&atmel_port->sg_rx,
virt_to_page(ring->buf),
sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE,
- (unsigned long)ring->buf & ~PAGE_MASK);
+ offset_in_page(ring->buf));
nent = dma_map_sg(port->dev,
&atmel_port->sg_rx,
1,
--
2.9.3
^ permalink raw reply related
* Re: Commit 18e8cf159177 broke qemu-system-sh4 serial input.
From: Rob Landley @ 2017-04-22 8:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: John Paul Adrian Glaubitz, Linux-sh list, Ulrich Hecht,
Geert Uytterhoeven, Linux-Renesas, linux-serial@vger.kernel.org
In-Reply-To: <CAMuHMdWtBHTBCNo2-oO2Zv7O8McypcaEMh+_8DD+D1T7GTxAzw@mail.gmail.com>
On 04/21/2017 02:26 AM, Geert Uytterhoeven wrote:
> Hi Rob,
>
> CC Adrian, linux-serial, linux-renesas-soc
>
> On Fri, Apr 21, 2017 at 8:21 AM, Rob Landley <rob@landley.net> wrote:
>> In 4.11-rc7 the qemu serial console hangs after the first character you
>> type. To reproduce, configure linux with this mini.config:
>>
>> CONFIG_CPU_SUBTYPE_SH7751R=y
>> CONFIG_MMU=y
>> CONFIG_MEMORY_START=0x0c000000
>> CONFIG_VSYSCALL=y
>> CONFIG_SH_FPU=y
>> CONFIG_SH_RTS7751R2D=y
>> CONFIG_RTS7751R2D_PLUS=y
>> CONFIG_SERIAL_SH_SCI=y
>> CONFIG_SERIAL_SH_SCI_CONSOLE=y
>> CONFIG_EARLY_PRINTK=y
>> CONFIG_BLK_DEV_INITRD=y"
>> CONFIG_RD_GZIP=y
>> CONFIG_BINFMT_ELF=y
>> CONFIG_BINFMT_SCRIPT=y
>> CONFIG_MISC_FILESYSTEMS=y
>> CONFIG_DEVTMPFS=y
>>
>> Using "make ARCH=sh allnoconfig KCONFIG_ALLCONFIG=mini.conf", then build
>> the result, then boot under qemu using the following command line:
>>
>> qemu-system-sh4 -M r2d -monitor null -serial null -serial stdio
>> -nographic -no-reboot -append "panic=1 HOST=sh4 console=ttySC1 noiotrap"
>> -kernel zImage -initrd sh4-linux-musl-root.cpio.gz
>>
>> You'll need a simple cpio.gz initramfs (I built one using
>> https://github.com/landley/mkroot and the musl-cross-make cross
>> compiler, the cpio.gz is ~450k or I'd attach it here).
>>
>> I bisected it to commit 18e8cf159177 back in February. If you do this
>> with the revision _before_ that commit, typing "ls -l" and hitting enter
>> works fine. Afterwards you get an "l" and then it hangs. (If type enough
>> it'll eventually deal a burst of characters all at once, and then hang
>> again.)
>
> SH7751R has both SCI and SCIF ports. ttySC1 is the second (SCIF) port.
Yeah, my qemu invocation for sh4 is a bit eldrich, I got it from the
qemu mailing list. Possibly from:
https://lists.nongnu.org/archive/html/qemu-devel/2007-09/msg00530.html
The really _fun_ thing about this is qemu broke it a couple years back
and now if you hit ctrl-c it kills the _emulator_ rather than passing it
through to the Linux console. (Just sh4, the rest do it right.)
I complained about it on the qemu mailing list in 2014 but nobody there
cared:
http://lists.nongnu.org/archive/html/qemu-devel/2014-09/msg00000.html
Of course that's actually _useful_ for the testing I'm doing now because
when the kernel tries to shut down qemu it goes:
/ # exit
reboot: Restarting system
Unauthorized access
And then hangs eating 100% cpu. (Wheee!)
> registers a port with type PORT_SCIF,
> so that should become SCIx_SH4_SCIF_REGTYPE. Hence I don't think the issue
> is caused by changing fifosize to 64 for SCIx_SH7705_SCIF_REGTYPE.
>
> My first guess is that qemu has a bug emulating the triggering.
Very likely given how crappy the rest of its serial emulation is for
this architecture, but ever since qemu added glib support I've stopped
trying to understand their developers' thought processes.
> According to the SH7751R datasheet, SCFCR does have the RTRG1 and RTRG0 bits.
> I assume the problem goes away if you comment out the call to scif_set_rtrg()?
The current code has been further complicated by two more commits
(039403765 and 90afa5255) doing who knows what, but deleting the
"scif_set_rtrg(port, s->rx_trigger);" from sci_reset() does appear to
fix the problem.
Rob
^ permalink raw reply
* [PATCH] serial: exar: Fix stuck MSIs
From: Jan Kiszka @ 2017-04-22 9:36 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee,
Andy Shevchenko
[-- Attachment #1.1: Type: text/plain, Size: 2837 bytes --]
From: Jan Kiszka <jan.kiszka@siemens.com>
After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
without further interrupts because of the special wake-up event these
chips send. They are only cleared by reading INT0. As we fail to do so
during startup and shutdown, we can leave the interrupt line asserted,
which is fatal with edge-triggered MSIs.
Add the required reading of INT0 to startup and shutdown. Also account
for the fact that a pending wake-up interrupt means we have to return 1
from exar_handle_irq.
An alternative approach would have been disabling the wake-up interrupt.
Unfortunately, this feature (REGB[17] = 1) is not available on the
XR17D15X.
Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Regression of upcoming 4.11.
drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..3a3667880fcf 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -47,6 +47,7 @@
/*
* These are definitions for the Exar XR17V35X and XR17(C|D)15X
*/
+#define UART_EXAR_INT0 0x80
#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
#define UART_EXAR_DVID 0x8d /* Device identification */
@@ -1869,17 +1870,13 @@ static int serial8250_default_handle_irq(struct uart_port *port)
static int exar_handle_irq(struct uart_port *port)
{
unsigned int iir = serial_port_in(port, UART_IIR);
- int ret;
+ int ret = 0;
- ret = serial8250_handle_irq(port, iir);
+ if (((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X)) &&
+ serial_port_in(port, UART_EXAR_INT0) != 0)
+ ret = 1;
- if ((port->type == PORT_XR17V35X) ||
- (port->type == PORT_XR17D15X)) {
- serial_port_in(port, 0x80);
- serial_port_in(port, 0x81);
- serial_port_in(port, 0x82);
- serial_port_in(port, 0x83);
- }
+ ret |= serial8250_handle_irq(port, iir);
return ret;
}
@@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
/*
* At this point, there's no way the LSR could still be 0xff;
@@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
up->lsr_saved_flags = 0;
up->msr_saved_flags = 0;
--
2.12.0
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply related
* [PATCH] serial: 8250: omap: Disable DMA for console UART
From: Vignesh R @ 2017-04-22 13:07 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Peter Hurley, Vignesh R, Andy Shevchenko,
linux-serial, linux-omap, linux-kernel
Kernel always writes log messages to console via
serial8250_console_write()->serial8250_console_putchar() which directly
accesses UART_TX register _without_ using DMA.
But, if other processes like systemd using same UART port, then these
writes are handled by a different code flow using 8250_omap driver where
there is provision to use DMA.
It seems that it is possible that both DMA and CPU might simultaneously
put data to UART FIFO and lead to potential loss of data due to FIFO
overflow and weird data corruption. This happens when both kernel
console and userspace tries to write simultaneously to the same UART
port. Therefore, disable DMA on kernel console port to avoid potential
race between CPU and DMA.
Signed-off-by: Vignesh R <vigneshr@ti.com>
---
drivers/tty/serial/8250/8250_omap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index e7e64913a748..d81bac98d190 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -613,6 +613,10 @@ static int omap_8250_startup(struct uart_port *port)
up->lsr_saved_flags = 0;
up->msr_saved_flags = 0;
+ /* Disable DMA for console UART */
+ if (uart_console(port))
+ up->dma = NULL;
+
if (up->dma) {
ret = serial8250_request_dma(up);
if (ret) {
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] tty/serial: atmel: use offset_in_page() macro
From: Richard Genoud @ 2017-04-24 7:12 UTC (permalink / raw)
To: Geliang Tang; +Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <5faae8a0740a804663c1066b23e2d5e7e9ebcbd3.1492758735.git.geliangtang@gmail.com>
On 22/04/2017 03:21, Geliang Tang wrote:
> Use offset_in_page() macro instead of open-coding.
>
> Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> ---
> drivers/tty/serial/atmel_serial.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index c355ac9..f398db8 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -959,7 +959,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
> sg_set_page(&atmel_port->sg_tx,
> virt_to_page(port->state->xmit.buf),
> UART_XMIT_SIZE,
> - (unsigned long)port->state->xmit.buf & ~PAGE_MASK);
> + offset_in_page(port->state->xmit.buf));
> nent = dma_map_sg(port->dev,
> &atmel_port->sg_tx,
> 1,
> @@ -1141,7 +1141,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
> sg_set_page(&atmel_port->sg_rx,
> virt_to_page(ring->buf),
> sizeof(struct atmel_uart_char) * ATMEL_SERIAL_RINGSIZE,
> - (unsigned long)ring->buf & ~PAGE_MASK);
> + offset_in_page(ring->buf));
> nent = dma_map_sg(port->dev,
> &atmel_port->sg_rx,
> 1,
>
I guess you want to #include <linux/mm.h>
(even if it compiles ok without it)
Regards,
Richard.
^ permalink raw reply
* Re: Commit 18e8cf159177 broke qemu-system-sh4 serial input.
From: Ulrich Hecht @ 2017-04-24 7:23 UTC (permalink / raw)
To: Rob Landley
Cc: Geert Uytterhoeven, John Paul Adrian Glaubitz, Linux-sh list,
Geert Uytterhoeven, Linux-Renesas, linux-serial@vger.kernel.org
In-Reply-To: <fc641b90-e939-e9cc-8621-f4f6d202b0da@landley.net>
On Sat, Apr 22, 2017 at 10:49 AM, Rob Landley <rob@landley.net> wrote:
>
>
> On 04/21/2017 02:26 AM, Geert Uytterhoeven wrote:
>> According to the SH7751R datasheet, SCFCR does have the RTRG1 and RTRG0 bits.
>> I assume the problem goes away if you comment out the call to scif_set_rtrg()?
>
> The current code has been further complicated by two more commits
> (039403765 and 90afa5255) doing who knows what, but deleting the
> "scif_set_rtrg(port, s->rx_trigger);" from sci_reset() does appear to
> fix the problem.
Most(?) SCIFs have a feature that is always enabled and that asserts
DR even if the FIFO threshold has not been reached if no data is
received for 1.5 frames. Exceptions known to me are SCIFA/Bs, which
require special handling for it to work, and SH7705's SCIF, which does
not seem to have this at all. According to its datasheet, the
SH7751R's SCIF has the timeout feature, so I would have expected it to
work...
CU
Uli
^ permalink raw reply
* Re: [PATCH] serial: exar: Fix stuck MSIs
From: Andy Shevchenko @ 2017-04-24 8:59 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
In-Reply-To: <39750541-c6a9-0e49-59fa-9f0a2e202850@web.de>
On Sat, 2017-04-22 at 11:36 +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
>
> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> without further interrupts because of the special wake-up event these
> chips send. They are only cleared by reading INT0. As we fail to do so
> during startup and shutdown, we can leave the interrupt line asserted,
> which is fatal with edge-triggered MSIs.
>
> Add the required reading of INT0 to startup and shutdown. Also account
> for the fact that a pending wake-up interrupt means we have to return
> 1
> from exar_handle_irq.
>
> An alternative approach would have been disabling the wake-up
> interrupt.
> Unfortunately, this feature (REGB[17] = 1) is not available on the
> XR17D15X.
>
> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Regression of upcoming 4.11.
>
> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 6119516ef5fc..3a3667880fcf 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -47,6 +47,7 @@
> /*
> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> */
> +#define UART_EXAR_INT0 0x80
> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> #define UART_EXAR_DVID 0x8d /* Device
> identification */
>
> @@ -1869,17 +1870,13 @@ static int
> serial8250_default_handle_irq(struct uart_port *port)
> static int exar_handle_irq(struct uart_port *port)
> {
> unsigned int iir = serial_port_in(port, UART_IIR);
> - int ret;
> + int ret = 0;
>
> - ret = serial8250_handle_irq(port, iir);
> + if (((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X)) &&
> + serial_port_in(port, UART_EXAR_INT0) != 0)
> + ret = 1;
>
> - if ((port->type == PORT_XR17V35X) ||
> - (port->type == PORT_XR17D15X)) {
> - serial_port_in(port, 0x80);
> - serial_port_in(port, 0x81);
> - serial_port_in(port, 0x82);
> - serial_port_in(port, 0x83);
You replaced 4 reads by one. I'm suspecting that on multi-port cards you
still need to read all of them (I assume they are called INT0, INT1,
...). Perhaps you need a helper where you do that and call it from all
necessary places.
> - }
> + ret |= serial8250_handle_irq(port, iir);
>
> return ret;
> }
> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
>
> /*
> * At this point, there's no way the LSR could still be 0xff;
> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
> up->lsr_saved_flags = 0;
> up->msr_saved_flags = 0;
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH] serial: exar: Fix stuck MSIs
From: Jan Kiszka @ 2017-04-24 9:06 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
In-Reply-To: <1493024379.24567.154.camel@linux.intel.com>
[-- Attachment #1.1: Type: text/plain, Size: 3632 bytes --]
On 2017-04-24 10:59, Andy Shevchenko wrote:
> On Sat, 2017-04-22 at 11:36 +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
>> without further interrupts because of the special wake-up event these
>> chips send. They are only cleared by reading INT0. As we fail to do so
>> during startup and shutdown, we can leave the interrupt line asserted,
>> which is fatal with edge-triggered MSIs.
>>
>> Add the required reading of INT0 to startup and shutdown. Also account
>> for the fact that a pending wake-up interrupt means we have to return
>> 1
>> from exar_handle_irq.
>>
>> An alternative approach would have been disabling the wake-up
>> interrupt.
>> Unfortunately, this feature (REGB[17] = 1) is not available on the
>> XR17D15X.
>>
>> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>
>> Regression of upcoming 4.11.
>>
>> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
>> 1 file changed, 10 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250_port.c
>> b/drivers/tty/serial/8250/8250_port.c
>> index 6119516ef5fc..3a3667880fcf 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -47,6 +47,7 @@
>> /*
>> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
>> */
>> +#define UART_EXAR_INT0 0x80
>> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
>> #define UART_EXAR_DVID 0x8d /* Device
>> identification */
>>
>> @@ -1869,17 +1870,13 @@ static int
>> serial8250_default_handle_irq(struct uart_port *port)
>> static int exar_handle_irq(struct uart_port *port)
>> {
>> unsigned int iir = serial_port_in(port, UART_IIR);
>> - int ret;
>> + int ret = 0;
>>
>> - ret = serial8250_handle_irq(port, iir);
>> + if (((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X)) &&
>> + serial_port_in(port, UART_EXAR_INT0) != 0)
>> + ret = 1;
>>
>> - if ((port->type == PORT_XR17V35X) ||
>> - (port->type == PORT_XR17D15X)) {
>
>> - serial_port_in(port, 0x80);
>> - serial_port_in(port, 0x81);
>> - serial_port_in(port, 0x82);
>> - serial_port_in(port, 0x83);
>
> You replaced 4 reads by one. I'm suspecting that on multi-port cards you
> still need to read all of them (I assume they are called INT0, INT1,
> ...). Perhaps you need a helper where you do that and call it from all
> necessary places.
Nope, we never had to read them all: "Wake-up Indicator is cleared by
reading the INT0 register." (Exar manual) INT0 contains the interrupt
status for all channels.
Jan
>
>> - }
>> + ret |= serial8250_handle_irq(port, iir);
>>
>> return ret;
>> }
>> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>>
>> /*
>> * At this point, there's no way the LSR could still be 0xff;
>> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
>> *port)
>> serial_port_in(port, UART_RX);
>> serial_port_in(port, UART_IIR);
>> serial_port_in(port, UART_MSR);
>> + if ((port->type == PORT_XR17V35X) || (port->type ==
>> PORT_XR17D15X))
>> + serial_port_in(port, UART_EXAR_INT0);
>> up->lsr_saved_flags = 0;
>> up->msr_saved_flags = 0;
>>
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [PATCH] serial: exar: Fix stuck MSIs
From: Andy Shevchenko @ 2017-04-24 9:45 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
In-Reply-To: <d227c786-24eb-5acb-777d-02bd28d14254@web.de>
On Mon, 2017-04-24 at 11:06 +0200, Jan Kiszka wrote:
> On 2017-04-24 10:59, Andy Shevchenko wrote:
> > On Sat, 2017-04-22 at 11:36 +0200, Jan Kiszka wrote:
> > > From: Jan Kiszka <jan.kiszka@siemens.com>
> > >
> > > After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> > > without further interrupts because of the special wake-up event
> > > these
> > > chips send. They are only cleared by reading INT0. As we fail to
> > > do so
> > > during startup and shutdown, we can leave the interrupt line
> > > asserted,
> > > which is fatal with edge-triggered MSIs.
> > >
> > > Add the required reading of INT0 to startup and shutdown. Also
> > > account
> > > for the fact that a pending wake-up interrupt means we have to
> > > return
> > > 1
> > > from exar_handle_irq.
> > >
> > > An alternative approach would have been disabling the wake-up
> > > interrupt.
> > > Unfortunately, this feature (REGB[17] = 1) is not available on the
> > > XR17D15X.
> > >
> > > Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
> > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> > > ---
> > >
> > > Regression of upcoming 4.11.
> > >
> > > drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
> > > 1 file changed, 10 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_port.c
> > > b/drivers/tty/serial/8250/8250_port.c
> > > index 6119516ef5fc..3a3667880fcf 100644
> > > --- a/drivers/tty/serial/8250/8250_port.c
> > > +++ b/drivers/tty/serial/8250/8250_port.c
> > > @@ -47,6 +47,7 @@
> > > /*
> > > * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> > > */
> > > +#define UART_EXAR_INT0 0x80
> > > #define UART_EXAR_SLEEP 0x8b /* Sleep mode
> > > */
> > > #define UART_EXAR_DVID 0x8d /* Device
> > > identification */
> > >
> > > @@ -1869,17 +1870,13 @@ static int
> > > serial8250_default_handle_irq(struct uart_port *port)
> > > static int exar_handle_irq(struct uart_port *port)
> > > {
> > > unsigned int iir = serial_port_in(port, UART_IIR);
> > > - int ret;
> > > + int ret = 0;
> > >
> > > - ret = serial8250_handle_irq(port, iir);
> > > + if (((port->type == PORT_XR17V35X) || (port->type ==
> > > PORT_XR17D15X)) &&
> > > + serial_port_in(port, UART_EXAR_INT0) != 0)
> > > + ret = 1;
> > >
> > > - if ((port->type == PORT_XR17V35X) ||
> > > - (port->type == PORT_XR17D15X)) {
> > > - serial_port_in(port, 0x80);
> > > - serial_port_in(port, 0x81);
> > > - serial_port_in(port, 0x82);
> > > - serial_port_in(port, 0x83);
> >
> > You replaced 4 reads by one. I'm suspecting that on multi-port cards
> > you
> > still need to read all of them (I assume they are called INT0, INT1,
> > ...). Perhaps you need a helper where you do that and call it from
> > all
> > necessary places.
>
> Nope, we never had to read them all: "Wake-up Indicator is cleared by
> reading the INT0 register." (Exar manual) INT0 contains the interrupt
> status for all channels.
Perhaps it needs to be mentioned in commit message as well.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* [PATCH v2] serial: exar: Fix stuck MSIs
From: Jan Kiszka @ 2017-04-24 10:30 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
In-Reply-To: <1493027143.24567.155.camel@linux.intel.com>
After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
without further interrupts because of the special wake-up event these
chips send. They are only cleared by reading INT0. As we fail to do so
during startup and shutdown, we can leave the interrupt line asserted,
which is fatal with edge-triggered MSIs.
Add the required reading of INT0 to startup and shutdown. Also account
for the fact that a pending wake-up interrupt means we have to return 1
from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
this - those never reset anything.
An alternative approach would have been disabling the wake-up interrupt.
Unfortunately, this feature (REGB[17] = 1) is not available on the
XR17D15X.
Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
Changes in v2:
- mention removal of INT1..3 reading in commit log [Andy]
(no functional changes)
drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 6119516ef5fc..3a3667880fcf 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -47,6 +47,7 @@
/*
* These are definitions for the Exar XR17V35X and XR17(C|D)15X
*/
+#define UART_EXAR_INT0 0x80
#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
#define UART_EXAR_DVID 0x8d /* Device identification */
@@ -1869,17 +1870,13 @@ static int serial8250_default_handle_irq(struct uart_port *port)
static int exar_handle_irq(struct uart_port *port)
{
unsigned int iir = serial_port_in(port, UART_IIR);
- int ret;
+ int ret = 0;
- ret = serial8250_handle_irq(port, iir);
+ if (((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X)) &&
+ serial_port_in(port, UART_EXAR_INT0) != 0)
+ ret = 1;
- if ((port->type == PORT_XR17V35X) ||
- (port->type == PORT_XR17D15X)) {
- serial_port_in(port, 0x80);
- serial_port_in(port, 0x81);
- serial_port_in(port, 0x82);
- serial_port_in(port, 0x83);
- }
+ ret |= serial8250_handle_irq(port, iir);
return ret;
}
@@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
/*
* At this point, there's no way the LSR could still be 0xff;
@@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port *port)
serial_port_in(port, UART_RX);
serial_port_in(port, UART_IIR);
serial_port_in(port, UART_MSR);
+ if ((port->type == PORT_XR17V35X) || (port->type == PORT_XR17D15X))
+ serial_port_in(port, UART_EXAR_INT0);
up->lsr_saved_flags = 0;
up->msr_saved_flags = 0;
--
2.12.0
^ permalink raw reply related
* Re: [PATCH v2] serial: exar: Fix stuck MSIs
From: Andy Shevchenko @ 2017-04-24 12:07 UTC (permalink / raw)
To: Jan Kiszka, Greg Kroah-Hartman
Cc: Linux Kernel Mailing List, linux-serial, Sudip Mukherjee
In-Reply-To: <b76259dc-42bf-4b3c-8122-04e0b0d217a2@siemens.com>
On Mon, 2017-04-24 at 12:30 +0200, Jan Kiszka wrote:
> After migrating 8250_exar to MSI in 172c33cb61da, we can get stuck
> without further interrupts because of the special wake-up event these
> chips send. They are only cleared by reading INT0. As we fail to do so
> during startup and shutdown, we can leave the interrupt line asserted,
> which is fatal with edge-triggered MSIs.
>
> Add the required reading of INT0 to startup and shutdown. Also account
> for the fact that a pending wake-up interrupt means we have to return
> 1
> from exar_handle_irq. Drop the unneeded reading of INT1..3 along with
> this - those never reset anything.
>
> An alternative approach would have been disabling the wake-up
> interrupt.
> Unfortunately, this feature (REGB[17] = 1) is not available on the
> XR17D15X.
FWIW:
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> Fixes: 172c33cb61da ("serial: exar: Enable MSI support")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>
> Changes in v2:
> - mention removal of INT1..3 reading in commit log [Andy]
> (no functional changes)
>
> drivers/tty/serial/8250/8250_port.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 6119516ef5fc..3a3667880fcf 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -47,6 +47,7 @@
> /*
> * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> */
> +#define UART_EXAR_INT0 0x80
> #define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> #define UART_EXAR_DVID 0x8d /* Device
> identification */
>
> @@ -1869,17 +1870,13 @@ static int
> serial8250_default_handle_irq(struct uart_port *port)
> static int exar_handle_irq(struct uart_port *port)
> {
> unsigned int iir = serial_port_in(port, UART_IIR);
> - int ret;
> + int ret = 0;
>
> - ret = serial8250_handle_irq(port, iir);
> + if (((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X)) &&
> + serial_port_in(port, UART_EXAR_INT0) != 0)
> + ret = 1;
>
> - if ((port->type == PORT_XR17V35X) ||
> - (port->type == PORT_XR17D15X)) {
> - serial_port_in(port, 0x80);
> - serial_port_in(port, 0x81);
> - serial_port_in(port, 0x82);
> - serial_port_in(port, 0x83);
> - }
> + ret |= serial8250_handle_irq(port, iir);
>
> return ret;
> }
> @@ -2177,6 +2174,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
>
> /*
> * At this point, there's no way the LSR could still be 0xff;
> @@ -2335,6 +2334,8 @@ int serial8250_do_startup(struct uart_port
> *port)
> serial_port_in(port, UART_RX);
> serial_port_in(port, UART_IIR);
> serial_port_in(port, UART_MSR);
> + if ((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X))
> + serial_port_in(port, UART_EXAR_INT0);
> up->lsr_saved_flags = 0;
> up->msr_saved_flags = 0;
>
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: Commit 18e8cf159177 broke qemu-system-sh4 serial input.
From: Rob Landley @ 2017-04-24 20:46 UTC (permalink / raw)
To: Ulrich Hecht
Cc: Geert Uytterhoeven, John Paul Adrian Glaubitz, Linux-sh list,
Geert Uytterhoeven, Linux-Renesas, linux-serial@vger.kernel.org
In-Reply-To: <CAO3366wMgKrDXM5=HO27TiU7kZqBO-hFxuzVJkJLbNz82zAodw@mail.gmail.com>
On 04/24/2017 02:23 AM, Ulrich Hecht wrote:
> On Sat, Apr 22, 2017 at 10:49 AM, Rob Landley <rob@landley.net> wrote:
>> On 04/21/2017 02:26 AM, Geert Uytterhoeven wrote:
>>> According to the SH7751R datasheet, SCFCR does have the RTRG1 and RTRG0 bits.
>>> I assume the problem goes away if you comment out the call to scif_set_rtrg()?
>>
>> The current code has been further complicated by two more commits
>> (039403765 and 90afa5255) doing who knows what, but deleting the
>> "scif_set_rtrg(port, s->rx_trigger);" from sci_reset() does appear to
>> fix the problem.
>
> Most(?) SCIFs have a feature that is always enabled and that asserts
> DR even if the FIFO threshold has not been reached if no data is
> received for 1.5 frames. Exceptions known to me are SCIFA/Bs, which
> require special handling for it to work, and SH7705's SCIF, which does
> not seem to have this at all. According to its datasheet, the
> SH7751R's SCIF has the timeout feature, so I would have expected it to
> work...
QEMU is an emulator. I've run sh4 linux on it for about 10 years now.
Sounds like they never implemented this feature because nothing used it
before now.
My general approach when dealing with this sort of thing is to point
figures at the thing that changed, and caused "it was working" to go to
"it's not working". When qemu broke this same sh4 serial port, I asked
_them_ to fix it:
https://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg03929.html
Asking qemu to change means current kernels will never run on existing
qemu installations again, when they did for a decade now, which seems
like a regression to me?
Rob
^ permalink raw reply
* Re: Commit 18e8cf159177 broke qemu-system-sh4 serial input.
From: Rich Felker @ 2017-04-25 1:23 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: Geert Uytterhoeven, Rob Landley, Linux-sh list, Ulrich Hecht,
Geert Uytterhoeven, Linux-Renesas, linux-serial@vger.kernel.org
In-Reply-To: <f27afb36-f75c-0a03-dca2-8f318265f520@physik.fu-berlin.de>
On Fri, Apr 21, 2017 at 09:50:39AM +0200, John Paul Adrian Glaubitz wrote:
> On 04/21/2017 09:26 AM, Geert Uytterhoeven wrote:
> > Does anyone have access to real hardware to try?
> > Adrian: does it work on your LANDISK, which also has SH7751R?
>
> I can give it a try but that will take a few days since I am currently
> not at home where my LANDISK boxes are. I also had issues with the
> IDE driver in the past but Sato-san gave me some tips regarding it.
>
> However, Rich also has a LANDISK device that I donated for him, so if his
> hardware is currently ready to use, he could give it a try.
Presently my LANDISK is in storage along with lots of other stuff due
to fire, but as far as I know it was not damaged. However I never got
directions for how to install a new kernel and point the bootloader at
it. It'll probably be another couple months before I get to moving
back in and unpacking everything from storage, but if you or anyone
else can get me the info between now and then on how to install new
kernel rather than just dd'ing a whole disk image, I'll follow up as
soon as I can.
Rich
^ permalink raw reply
* [PATCH] serial: sh-sci: Fix race condition causing garbage during shutdown
From: Geert Uytterhoeven @ 2017-04-25 18:15 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Richard Genoud, Yoshihiro Shimoda, linux-serial,
linux-renesas-soc, linux-kernel, Geert Uytterhoeven
If DMA is enabled and used, a burst of old data may be seen on the
serial console during "poweroff" or "reboot". uart_flush_buffer()
clears the circular buffer, but sci_port.tx_dma_len is not reset.
This leads to a circular buffer overflow, dumping (UART_XMIT_SIZE -
sci_port.tx_dma_len) bytes.
To fix this, add a .flush_buffer() callback that resets
sci_port.tx_dma_len.
Inspired by commit 31ca2c63fdc0aee7 ("tty/serial: atmel: fix race
condition (TX+DMA)").
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
A big thanks to Richard!
I had no idea what was happening here, until I saw his patch passing by
in a stable backport.
In v4.3 and older, the field is called sg_len_tx.
---
drivers/tty/serial/sh-sci.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 9a47cc4f16a2798f..a15739202d6c75f2 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1545,7 +1545,16 @@ static void sci_free_dma(struct uart_port *port)
if (s->chan_rx)
sci_rx_dma_release(s, false);
}
-#else
+
+static void sci_flush_buffer(struct uart_port *port)
+{
+ /*
+ * In uart_flush_buffer(), the xmit circular buffer has just been
+ * cleared, so we have to reset tx_dma_len accordingly.
+ */
+ to_sci_port(port)->tx_dma_len = 0;
+}
+#else /* !CONFIG_SERIAL_SH_SCI_DMA */
static inline void sci_request_dma(struct uart_port *port)
{
}
@@ -1553,7 +1562,9 @@ static inline void sci_request_dma(struct uart_port *port)
static inline void sci_free_dma(struct uart_port *port)
{
}
-#endif
+
+#define sci_flush_buffer NULL
+#endif /* !CONFIG_SERIAL_SH_SCI_DMA */
static irqreturn_t sci_rx_interrupt(int irq, void *ptr)
{
@@ -2566,6 +2577,7 @@ static const struct uart_ops sci_uart_ops = {
.break_ctl = sci_break_ctl,
.startup = sci_startup,
.shutdown = sci_shutdown,
+ .flush_buffer = sci_flush_buffer,
.set_termios = sci_set_termios,
.pm = sci_pm,
.type = sci_type,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] RFC: serial: core: Dynamic minor support
From: Alan Cox @ 2017-04-25 19:43 UTC (permalink / raw)
To: Sjoerd Simons
Cc: Greg Kroah-Hartman, linux-serial, Geert Uytterhoeven,
linux-kernel, Jiri Slaby
In-Reply-To: <20170420120357.18317-1-sjoerd.simons@collabora.co.uk>
> Furthermore some other drivers rely upon the usage of a dynamic major
> e.g. the tegra serial port driver. However as there is only a block of
> 20 major numbers reserved for dynamic assignment that isn't going to
> scale either for multiplatform kernels (I can already easily build 18
> different serial drivers on an arm64 kernel today).
The patch makes sense to me - we really don't need the static minors any
more.
> This does not try to address device naming by making all serial ports be
> ttyS<value> as that would first need a better way of deterministically
> specifying serial console devices. While it would be an option to move
> to consistent serial device naming while keeping the current console
> names, having both not aligned would probably confuse everyone
> massively.
The existing ttyS being 8250 is ABI and there are lots of things that
know far too much about what each type of ttyFOO is to ever fix that.
What could certainly be done is to do what some other classes of device
do and start also registering
/dev/ttyport/bus-information/0 /1 /2 .. etc
in parallel.
Alan
^ 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