* [PATCH v2 1/5] console: move console_init() out of tty_io.c
From: Nicolas Pitre @ 2017-04-01 22:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: linux-kernel, linux-arm-kernel
In-Reply-To: <20170401222119.25106-1-nicolas.pitre@linaro.org>
All the console driver handling code lives in printk.c.
Move console_init() there as well so console support can still be used
when the TTY code is configured out.
Signed-off-by: Nicolas Pitre <nico@linaro.org>
---
drivers/tty/tty_io.c | 24 ------------------------
include/linux/console.h | 2 ++
include/linux/tty.h | 7 ++++---
init/main.c | 2 +-
kernel/printk/printk.c | 24 ++++++++++++++++++++++++
5 files changed, 31 insertions(+), 28 deletions(-)
diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index e6d1a65108..2100295861 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -3578,30 +3578,6 @@ void tty_default_fops(struct file_operations *fops)
*fops = tty_fops;
}
-/*
- * Initialize the console device. This is called *early*, so
- * we can't necessarily depend on lots of kernel help here.
- * Just do some early initializations, and do the complex setup
- * later.
- */
-void __init console_init(void)
-{
- initcall_t *call;
-
- /* Setup the default TTY line discipline. */
- n_tty_init();
-
- /*
- * set up the console device so that later boot sequences can
- * inform about problems etc..
- */
- call = __con_initcall_start;
- while (call < __con_initcall_end) {
- (*call)();
- call++;
- }
-}
-
static char *tty_devnode(struct device *dev, umode_t *mode)
{
if (!mode)
diff --git a/include/linux/console.h b/include/linux/console.h
index 5949d18555..b8920a031a 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -212,4 +212,6 @@ extern bool vgacon_text_force(void);
static inline bool vgacon_text_force(void) { return false; }
#endif
+extern void console_init(void);
+
#endif /* _LINUX_CONSOLE_H */
diff --git a/include/linux/tty.h b/include/linux/tty.h
index 1017e904c0..f1106d7c73 100644
--- a/include/linux/tty.h
+++ b/include/linux/tty.h
@@ -390,7 +390,6 @@ static inline bool tty_throttled(struct tty_struct *tty)
}
#ifdef CONFIG_TTY
-extern void console_init(void);
extern void tty_kref_put(struct tty_struct *tty);
extern struct pid *tty_get_pgrp(struct tty_struct *tty);
extern void tty_vhangup_self(void);
@@ -402,8 +401,6 @@ extern struct tty_struct *get_current_tty(void);
extern int __init tty_init(void);
extern const char *tty_name(const struct tty_struct *tty);
#else
-static inline void console_init(void)
-{ }
static inline void tty_kref_put(struct tty_struct *tty)
{ }
static inline struct pid *tty_get_pgrp(struct tty_struct *tty)
@@ -669,7 +666,11 @@ extern int tty_ldisc_receive_buf(struct tty_ldisc *ld, const unsigned char *p,
/* n_tty.c */
extern void n_tty_inherit_ops(struct tty_ldisc_ops *ops);
+#ifdef CONFIG_TTY
extern void __init n_tty_init(void);
+#else
+static inline void n_tty_init(void) { }
+#endif
/* tty_audit.c */
#ifdef CONFIG_AUDIT
diff --git a/init/main.c b/init/main.c
index f9c9d99482..b9bd0edf21 100644
--- a/init/main.c
+++ b/init/main.c
@@ -27,7 +27,7 @@
#include <linux/initrd.h>
#include <linux/bootmem.h>
#include <linux/acpi.h>
-#include <linux/tty.h>
+#include <linux/console.h>
#include <linux/nmi.h>
#include <linux/percpu.h>
#include <linux/kmod.h>
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2984fb0f02..3a09406526 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2611,6 +2611,30 @@ int unregister_console(struct console *console)
EXPORT_SYMBOL(unregister_console);
/*
+ * Initialize the console device. This is called *early*, so
+ * we can't necessarily depend on lots of kernel help here.
+ * Just do some early initializations, and do the complex setup
+ * later.
+ */
+void __init console_init(void)
+{
+ initcall_t *call;
+
+ /* Setup the default TTY line discipline. */
+ n_tty_init();
+
+ /*
+ * set up the console device so that later boot sequences can
+ * inform about problems etc..
+ */
+ call = __con_initcall_start;
+ while (call < __con_initcall_end) {
+ (*call)();
+ call++;
+ }
+}
+
+/*
* Some boot consoles access data that is in the init section and which will
* be discarded after the initcalls have been run. To make sure that no code
* will access this data, unregister the boot consoles in a late initcall.
--
2.9.3
^ permalink raw reply related
* [PATCH v2 0/5] minitty: a minimal TTY layer alternative for embedded systems
From: Nicolas Pitre @ 2017-04-01 22:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial
Cc: linux-kernel, linux-arm-kernel
Many embedded systems don't need the full TTY layer support. Most of the
time, the TTY layer is only a conduit for outputting debugging messages
over a serial port. The TTY layer also implements many features that are
very unlikely to ever be used in such a setup. There is great potential
for both code and dynamic memory size reduction on small systems. This is
what this patch series is achieving.
The existing TTY code is quite large and complex. Trying to shrink it
is risky as the potential for breakage is non negligeable, and its
interchangeable layers impose a lower limit on the code to implement it.
Therefore, the approach used here consists in the creation of a parallel
implementation with the very minimal amount of code collapsed together
that interfaces with existing UART drivers directly and provides TTY-like
character devices to user space. When the regular TTY layer is disabled,
then this minitty alternative layer is proposed by Kconfig.
For more details on the rationale and motivations driving this approach
please see: https://lkml.org/lkml/2017/3/24/634
Of course, making it "mini" means there are limitations to what it does:
- This supports serial ports only. No VT's, no PTY's.
- The default n_tty line discipline is hardcoded and no other line
discipline are supported.
- The line discipline features are not all implemented. Notably, XON/XOFF
is currently not implemented (although this might not require a lot of
code to do it if someone were to need it).
- Hung-up state is not implemented.
- No error handling on RX bytes other than counting them.
- Job control is currently not supported (this may change in the future and
be configurable).
But again, most small embedded systems simply don't need those things.
This can be used on any architecture of course, but here's some numbers
using a minimal ARM config.
When CONFIG_TTY=y, the following files are linked into the kernel:
text data bss dec hex filename
8796 128 0 8924 22dc drivers/tty/n_tty.o
11809 276 0 12085 2f35 drivers/tty/serial/fulltty_serial.o
1376 0 0 1376 560 drivers/tty/tty_buffer.o
13571 172 132 13875 3633 drivers/tty/tty_io.o
3072 0 0 3072 c00 drivers/tty/tty_ioctl.o
2457 2 120 2579 a13 drivers/tty/tty_ldisc.o
1328 0 0 1328 530 drivers/tty/tty_ldsem.o
316 0 0 316 13c drivers/tty/tty_mutex.o
2516 0 0 2516 9d4 drivers/tty/tty_port.o
45241 578 252 46071 b3f7 (TOTALS)
When CONFIG_TTY=n and CONFIG_MINITTY_SERIAL=y, the above files are replaced
by the following:
text data bss dec hex filename
8063 8 64 8135 1fc7 drivers/tty/serial/minitty_serial.o
That's it! And the runtime buffer usage is much less as well. Future plans
such as removing runtime baudrate handling for those targets with a known
fixed baudrate will shrink the code even more.
Changes from v1:
- Added an entry to the MAINTAINERS file.
- Factored out more common core code into serial_lib.c.
- Implemented a few more TTY callback functions to be compatible with
more UART drivers.
Overall diffstat:
MAINTAINERS | 8 +-
drivers/tty/Kconfig | 10 +-
drivers/tty/Makefile | 3 +-
drivers/tty/serial/Kconfig | 12 +-
drivers/tty/serial/Makefile | 7 +-
.../serial/{serial_core.c => fulltty_serial.c} | 419 +---
drivers/tty/serial/minitty_serial.c | 1793 +++++++++++++++++
drivers/tty/serial/serial_lib.c | 440 ++++
drivers/tty/tty_baudrate.c | 232 +++
drivers/tty/tty_io.c | 24 -
drivers/tty/tty_ioctl.c | 222 --
include/linux/console.h | 2 +
include/linux/serial_core.h | 1 +
include/linux/tty.h | 7 +-
include/linux/tty_flip.h | 9 +
init/main.c | 2 +-
kernel/printk/printk.c | 24 +
17 files changed, 2538 insertions(+), 677 deletions(-)
^ permalink raw reply
* [GIT PULL] TTY/Serial driver fixes for 4.11-rc5
From: Greg KH @ 2017-04-01 15:59 UTC (permalink / raw)
To: Linus Torvalds, Jiri Slaby; +Cc: Andrew Morton, linux-kernel, linux-serial
The following changes since commit c02ed2e75ef4c74e41e421acb4ef1494671585e8:
Linux 4.11-rc4 (2017-03-26 14:15:16 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git/ tags/tty-4.11-rc5
for you to fetch changes up to e53e597fd4c4a0b6ae58e57d76a240927fd17eaa:
tty: pl011: fix earlycon work-around for QDF2400 erratum 44 (2017-04-01 11:07:29 +0200)
----------------------------------------------------------------
TTY/Serial fixes for 4.11-rc5
Here are some small fixes for some serial drivers and Kconfig help text
for 4.11-rc5. Nothing major here at all, a few things resolving
reported bugs in some random serial drivers.
I don't think these made the last linux-next due to me getting to them
yesterday, but I am not sure, they might have snuck in. The patches
only affect drivers that the maintainers of sent me these patches for,
so we should be safe here :)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
----------------------------------------------------------------
Nicolas Ferre (1):
tty/serial: atmel: fix TX path in atmel_console_write()
Paul Gortmaker (1):
serial: 8250_EXAR: fix duplicate Kconfig text and add missing help text
Richard Genoud (1):
tty/serial: atmel: fix race condition (TX+DMA)
Timur Tabi (1):
tty: pl011: fix earlycon work-around for QDF2400 erratum 44
Uwe Kleine-König (1):
serial: mxs-auart: Fix baudrate calculation
drivers/tty/serial/8250/Kconfig | 8 ++++++--
drivers/tty/serial/amba-pl011.c | 23 +++++++++++++++++++++--
drivers/tty/serial/atmel_serial.c | 8 ++++++++
drivers/tty/serial/mxs-auart.c | 2 +-
4 files changed, 36 insertions(+), 5 deletions(-)
^ permalink raw reply
* [PATCH] tty: pl011: fix earlycon work-around for QDF2400 erratum 44
From: Timur Tabi @ 2017-03-31 22:05 UTC (permalink / raw)
To: Shanker Donthineni, Christopher Covington, Russell King,
Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-arm-kernel,
yousaf.kaukab, agraf, mbrugger
The work-around for the Qualcomm Datacenter Technologies QDF2400
erratum 44 sets the "qdf2400_e44_present" global variable if the
work-around is needed. However, this check does not happen until after
earlycon is initialized, which means the work-around is not
used, and the console hangs as soon as it displays one character.
Fixes: d8a4995bcea1 ("tty: pl011: Work around QDF2400 E44 stuck BUSY bit")
Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
drivers/tty/serial/amba-pl011.c | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 56f92d7..b0a3777 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2452,18 +2452,37 @@ static void pl011_early_write(struct console *con, const char *s, unsigned n)
uart_console_write(&dev->port, s, n, pl011_putc);
}
+/*
+ * On non-ACPI systems, earlycon is enabled by specifying
+ * "earlycon=pl011,<address>" on the kernel command line.
+ *
+ * On ACPI ARM64 systems, an "early" console is enabled via the SPCR table,
+ * by specifying only "earlycon" on the command line. Because it requires
+ * SPCR, the console starts after ACPI is parsed, which is later than a
+ * traditional early console.
+ *
+ * To get the traditional early console that starts before ACPI is parsed,
+ * specify the full "earlycon=pl011,<address>" option.
+ */
static int __init pl011_early_console_setup(struct earlycon_device *device,
const char *opt)
{
if (!device->port.membase)
return -ENODEV;
- device->con->write = qdf2400_e44_present ?
- qdf2400_e44_early_write : pl011_early_write;
+ /* On QDF2400 SOCs affected by Erratum 44, the "qdf2400_e44" must
+ * also be specified, e.g. "earlycon=pl011,<address>,qdf2400_e44".
+ */
+ if (!strcmp(device->options, "qdf2400_e44"))
+ device->con->write = qdf2400_e44_early_write;
+ else
+ device->con->write = pl011_early_write;
+
return 0;
}
OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
OF_EARLYCON_DECLARE(pl011, "arm,sbsa-uart", pl011_early_console_setup);
+EARLYCON_DECLARE(qdf2400_e44, pl011_early_console_setup);
#else
#define AMBA_CONSOLE NULL
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc. Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply related
* Re: [PATCH 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Benjamin Herrenschmidt @ 2017-03-31 21:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Joel Stanley
Cc: Jiri Slaby, Mark Rutland, Rob Herring, Jeremy Kerr, linux-serial,
linux-kernel, openbmc, devicetree
In-Reply-To: <20170331133120.GA26078@kroah.com>
On Fri, 2017-03-31 at 15:31 +0200, Greg Kroah-Hartman wrote:
> DEVICE_ATTR_RW()?
>
> And why random sysfs files for a uart? Where have you documented
> these?
We should stick a file somewhere in Documentation I suppose or
at least put that in a comment blob at the head of the file.
FYI:
The VUART is basically two UART 'front ends' connected by their FIFO
(no actual serial line in between). One is on the BMC side (management
controller) and one is on the host CPU side.
It allows the BMC to provide to the host a "UART" that pipes into
the BMC itself and can then be turned by the BMC into a network console
of some sort for example.
This driver is for the BMC side. The sysfs files allow the BMC
userspace which owns the system configuration policy, to specify
at what IO port and interrupt number the host side will appear
to the host on the Host <-> BMC LPC bus. It could be different on a
different system (though most of them use 3f8/4).
Cheers,
Ben.
^ permalink raw reply
* [PATCH v3 3/4] tty/serial: meson_uart: add the core clock handling to the driver
From: Helmut Klein @ 2017-03-31 16:54 UTC (permalink / raw)
To: gregkh, carlo, khilman
Cc: Helmut Klein, linux-serial, linux-amlogic, linux-arm-kernel,
linux-kernel
In-Reply-To: <20170331165437.26227-1-hgkr.klein@gmail.com>
This patch gets the core clock as provided by the DT and enables it.
The code was taken from Amlogic's serial driver, and was tested on my
board.
Signed-off-by: Helmut Klein <hgkr.klein@gmail.com>
---
drivers/tty/serial/meson_uart.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f16795d16b..cb99112288eb 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -600,6 +600,7 @@ static int meson_uart_probe(struct platform_device *pdev)
struct resource *res_mem, *res_irq;
struct uart_port *port;
struct clk *clk;
+ struct clk *core_clk;
int ret = 0;
if (pdev->dev.of_node)
@@ -625,6 +626,15 @@ static int meson_uart_probe(struct platform_device *pdev)
if (!port)
return -ENOMEM;
+ core_clk = devm_clk_get(&pdev->dev, "core");
+ if (!IS_ERR(core_clk)) {
+ ret = clk_prepare_enable(core_clk);
+ if (ret) {
+ dev_err(&pdev->dev, "couldn't enable clkc\n");
+ return ret;
+ }
+ }
+
clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(clk))
return PTR_ERR(clk);
--
2.11.0
^ permalink raw reply related
* [PATCH v3 0/4] tty/serial: meson_uart: add support for core clock handling
From: Helmut Klein @ 2017-03-31 16:54 UTC (permalink / raw)
Cc: Helmut Klein, linux-clk, linux-serial, linux-amlogic,
linux-arm-kernel, linux-kernel, devicetree
To be able to use the three none AO uarts of the meson gx SoCs (uart_A,
uart_B & uart_C), the core clock has to be enabled (see chapter 22.3 of
the public s905 data sheet).
At least the u-boot of my s905 based media player (netxeon minimx-g)
doesn't do this. so the driver must enable the clock.
This patch set does:
- exposes the UART clock ids to the dtb
- adds documentation for the dt-bindings of meson_uart
- adds the core clock handling to the driver
- adds the core clock handling to meson-gxbb.dtsi and meson-gxl.dtsi
The patchset is based on the branch "master" of the repository in [1]
Changes since v2
- mail subjects reworked
- add clocks/clock-names to the documentation
- add core clock handling to meson-gxbb.dtsi & meson-gxl.dtsi
Changes since v1
- use git to produce the patch set
- added the clock ids for uart_B and uart_C
[1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-amlogic.git
Helmut Klein (4):
clk: meson: gxbb: expose CLKID_UARTx
dt-bindings: meson_uart: add documentation for meson UARTs
tty/serial: meson_uart: add the core clock handling to the driver
ARM64: dts: meson-gx: add core clock support for uart_A, uart_B and
uart_C
.../bindings/serial/amlogic,meson_uart.txt | 30 ++++++++++++++++++++++
arch/arm64/boot/dts/amlogic/meson-gxbb.dtsi | 15 +++++++++++
arch/arm64/boot/dts/amlogic/meson-gxl.dtsi | 15 +++++++++++
drivers/clk/meson/gxbb.h | 6 ++---
drivers/tty/serial/meson_uart.c | 10 ++++++++
include/dt-bindings/clock/gxbb-clkc.h | 3 +++
6 files changed, 76 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/amlogic,meson_uart.txt
--
2.11.0
^ permalink raw reply
* Re: [PATCH v3] serdev: Add serdev_device_write subroutine
From: Rob Herring @ 2017-03-31 16:38 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Andrey Smirnov, Chris Healy, Guenter Roeck,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAHp75VdPg3K2+TYN7UTKi3aiq5NVn4gRnM+SwpE9uYwO89G_bQ@mail.gmail.com>
On Fri, Mar 31, 2017 at 9:35 AM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Mar 31, 2017 at 4:22 PM, Rob Herring <robh@kernel.org> wrote:
>> On Thu, Mar 30, 2017 at 5:00 PM, Andrey Smirnov
>> <andrew.smirnov@gmail.com> wrote:
>
>>> +int serdev_device_write(struct serdev_device *serdev,
>>> + const unsigned char *buf, size_t count,
>>> + unsigned long timeout)
>>> {
>>> struct serdev_controller *ctrl = serdev->ctrl;
>>> + int ret;
>>>
>>> + if (!ctrl || !ctrl->ops->write_buf ||
>>> + (timeout && !serdev->ops->write_wakeup))
>>> return -EINVAL;
>>>
>>> + mutex_lock(&serdev->write_lock);
>>> + do {
>>> + reinit_completion(&serdev->write_comp);
>>> +
>>> + ret = ctrl->ops->write_buf(ctrl, buf, count);
>>> + if (ret < 0)
>>> + break;
>>> +
>>> + buf += ret;
>>> + count -= ret;
>>> +
>>> + } while (count &&
>>> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
>>> + timeout)));
>>
>> Need to test for timeout < 0 here and return timeout when < 0.
>
> It can't be the case since the variable is of unsigned type.
Oh right, that's only the interruptible version. NM
Reviewed-by: Rob Herring <robh@kernel.org>
Rob
^ permalink raw reply
* Re: [PATCH] serial: Do not treat the IIR register as a bitfield
From: Andy Shevchenko @ 2017-03-31 14:44 UTC (permalink / raw)
To: Olliver Schinagl
Cc: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby, Laxman Dewangan,
Stephen Warren, Thierry Reding, Alexandre Courbot,
David S . Miller, dev, Ed Blake, Alexander Sverdlin,
Yegor Yefremov, Wan Ahmad Zainie, Kefeng Wang, Heikki Krogerus,
Heiko Stuebner, Jason Uy, Douglas Anderson
In-Reply-To: <b962b00c-64d7-0b55-2fde-5a3ea04d8671@schinagl.nl>
On Fri, Mar 31, 2017 at 4:54 PM, Olliver Schinagl
<o.schinagl@ultimaker.com> wrote:
> On 30-03-17 11:56, Andy Shevchenko wrote:
>> On Wed, 2017-03-29 at 20:44 +0200, Olliver Schinagl wrote:
>> Looking to implementation I would rather go with some helper like
>>
>> int serial_in_IIR(port, [additional mask])
>> {
>> return port->serial_in(port, UART_IIR) & (_IIR_MASK [| additional
>> mask]);
>> }
> As I just wrote a simply static inline helper function in serial_core.h, I
> just figured that the helper will only work for some of the calls. All
> interrupt checks in xxx_serial_in() obviously can't rely on this. So do you
> still want this helper function added for the other cases? Or have all
> implementations do the masking manually?
You have still few places (3+ IIRC) where it makes sense.
> And then, is iir = serial_port_in(up, UART_IIR) & UART_IIR_MASK; preferred
> over splitting it over two lines, like I did?
With given indentation it might be long enough to uglify the code.
So, I would still go with one / two helpers (do your own choice), but
if you insist that is not beneficial I would not object in-place
masking.
static inline int serial_in_IIR_mask(port, mask)
{
return ... & mask;
}
static inline int serial_in_IIR(port)
{
return serial_in_IIR_mask(port, ..._IIR_MASK);
}
> Finally, why rename it to _IIR_MASK, I assume a typo here?
I usually do such to minimize characters to type (notice leading _
which means I referred to a suffix) and that's why the work "like" is
used above implying you need to modify to function correctly.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3] serdev: Add serdev_device_write subroutine
From: Andy Shevchenko @ 2017-03-31 14:35 UTC (permalink / raw)
To: Rob Herring
Cc: Andrey Smirnov, Chris Healy, Guenter Roeck,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+W3UcmmRNuTALczAHCO_wHTDcDz4jXBJgJeB0X-3oGBA@mail.gmail.com>
On Fri, Mar 31, 2017 at 4:22 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Mar 30, 2017 at 5:00 PM, Andrey Smirnov
> <andrew.smirnov@gmail.com> wrote:
>> +int serdev_device_write(struct serdev_device *serdev,
>> + const unsigned char *buf, size_t count,
>> + unsigned long timeout)
>> {
>> struct serdev_controller *ctrl = serdev->ctrl;
>> + int ret;
>>
>> + if (!ctrl || !ctrl->ops->write_buf ||
>> + (timeout && !serdev->ops->write_wakeup))
>> return -EINVAL;
>>
>> + mutex_lock(&serdev->write_lock);
>> + do {
>> + reinit_completion(&serdev->write_comp);
>> +
>> + ret = ctrl->ops->write_buf(ctrl, buf, count);
>> + if (ret < 0)
>> + break;
>> +
>> + buf += ret;
>> + count -= ret;
>> +
>> + } while (count &&
>> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
>> + timeout)));
>
> Need to test for timeout < 0 here and return timeout when < 0.
It can't be the case since the variable is of unsigned type.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] serial: Do not treat the IIR register as a bitfield
From: Olliver Schinagl @ 2017-03-31 13:54 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby, Laxman Dewangan,
Stephen Warren, Thierry Reding, Alexandre Courbot,
David S . Miller
Cc: dev-3kdeTeqwOZ9EV1b7eY7vFQ, Ed Blake, Alexander Sverdlin,
Yegor Yefremov, Wan Ahmad Zainie, Kefeng Wang, Heikki Krogerus,
Heiko Stuebner, Jason Uy, Douglas Anderson, Peter Hurley,
Tony Lindgren, Vignesh R, Thor Thayer, David Lechner, Jan Kiszka,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-tegra-u79uwXL29TY76Z2rM5mHXA,
sparclinux-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1490867763.708.62.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Hey Andy,
On 30-03-17 11:56, Andy Shevchenko wrote:
> On Wed, 2017-03-29 at 20:44 +0200, Olliver Schinagl wrote:
>> It seems that at some point, someone made the assumption that the UART
>> Interrupt ID Register was a bitfield and started to check if certain
>> bits where set.
>>
>> Actually however the register contains interrupt ID's where only the
>> MSB
>> seems to be used singular and the rest share at least one bit. Thus
>> doing bitfield operations is wrong.
>>
>> This patch cleans up the serial_reg include file by ordering it and
>> replacing the UART_IIR_ID 'mask' with a proper mask for the register.
>> The OMAP uart appears to have used the two commonly 'reserved' bits 4
>> and 5 and thus get an UART_IIR_EXT_MASK for these two bits.
>>
>> This patch then goes over all UART_IIR_* users and changes the code
>> from
>> bitfield checking, to ID checking instead.
>
>
> Looking to implementation I would rather go with some helper like
>
> int serial_in_IIR(port, [additional mask])
> {
> return port->serial_in(port, UART_IIR) & (_IIR_MASK [| additional
> mask]);
> }
As I just wrote a simply static inline helper function in serial_core.h,
I just figured that the helper will only work for some of the calls. All
interrupt checks in xxx_serial_in() obviously can't rely on this. So do
you still want this helper function added for the other cases? Or have
all implementations do the masking manually?
And then, is iir = serial_port_in(up, UART_IIR) & UART_IIR_MASK;
preferred over splitting it over two lines, like I did?
Finally, why rename it to _IIR_MASK, I assume a typo here?
Olliver
>
^ permalink raw reply
* Re: [PATCH v2 1/2] serial: samsung: Use right device for DMA-mapping calls
From: Greg Kroah-Hartman @ 2017-03-31 13:35 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Marek Szyprowski, linux-samsung-soc, linux-serial,
Sylwester Nawrocki, Bartlomiej Zolnierkiewicz, Seung-Woo Kim,
Joonyoung Shim, Inki Dae, stable
In-Reply-To: <20170323195155.u3bfvuo6rxncsgzr@kozik-lap>
On Thu, Mar 23, 2017 at 09:51:55PM +0200, Krzysztof Kozlowski wrote:
> On Thu, Mar 23, 2017 at 01:01:52PM +0100, Marek Szyprowski wrote:
> > Driver should provide its own struct device for all DMA-mapping calls instead
> > of extracting device pointer from DMA engine channel.
>
> For the purpose of cc-stable it might be useful to answer here what
> visible error is fixed (or why driver should not use DMA's device) but
> on the other hand this is actually obvious... Anyway:
No, please make it obvious on the next version of this series.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCHv3 00/10] Nokia H4+ support
From: Greg Kroah-Hartman @ 2017-03-31 13:33 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Rob Herring, Sebastian Reichel, Gustavo F. Padovan, Johan Hedberg,
Samuel Thibault, Pavel Machek, Tony Lindgren, Jiri Slaby,
Mark Rutland, open list:BLUETOOTH DRIVERS,
linux-serial@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, David S. Miller
In-Reply-To: <C1A408F2-1DF8-4F49-B399-B936542DE6BC@holtmann.org>
On Wed, Mar 29, 2017 at 11:33:26PM +0200, Marcel Holtmann wrote:
> Hi Rob,
>
> >> Here is PATCHv3 for the Nokia bluetooth patchset. I addressed all comments from
> >> Rob and Pavel regarding the serdev patches and dropped the *.dts patches, since
> >> they were queued by Tony. I also changed the patch order, so that the serdev
> >> patches come first. All of them have Acked-by from Rob, so I think it makes
> >> sense to merge them to serdev subsystem (now) and provide an immutable branch
> >> for the bluetooth subsystem.
> >
> > Greg doesn't read cover letters generally and since the serdev patches
> > are Cc rather than To him, he's probably not planning to pick them up.
>
> I wonder actually if we should merge all of these via bluetooth-next
> tree with proper Ack from Greg. However it would be good to also get
> buy in from Dave for merging this ultimately through net-next.
I don't really care where it goes. I can take the whole thing in my
tty/serial tree now if no one objects and I get an ack from the relevant
maintainers {hint...}
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 2/2] drivers/serial: Add driver for Aspeed virtual UART
From: Greg Kroah-Hartman @ 2017-03-31 13:31 UTC (permalink / raw)
To: Joel Stanley
Cc: Jiri Slaby, Mark Rutland, Rob Herring, Jeremy Kerr,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, devicetree-u79uwXL29TY76Z2rM5mHXA,
Benjamin Herrenschmidt
In-Reply-To: <20170328054458.29341-3-joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
On Tue, Mar 28, 2017 at 04:14:58PM +1030, Joel Stanley wrote:
> From: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
>
> This change adds a driver for the 16550-based Aspeed virtual UART
> device. We use a similar process to the of_serial driver for device
> probe, but expose some VUART-specific functions through sysfs too.
>
> OpenPOWER host firmware doesn't like it when the host-side of the
> VUART's FIFO is not drained. This driver only disables host TX discard
> mode when the port is in use. We set the VUART enabled bit when we bind
> to the device, and clear it on unbind.
>
> We don't want to do this on open/release, as the host may be using this
> bit to configure serial output modes, which is independent of whether
> the devices has been opened by BMC userspace.
>
> Signed-off-by: Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
> ---
> Documentation/devicetree/bindings/serial/8250.txt | 2 +
> drivers/tty/serial/Kconfig | 10 +
> drivers/tty/serial/Makefile | 1 +
> drivers/tty/serial/aspeed-vuart.c | 335 ++++++++++++++++++++++
> 4 files changed, 348 insertions(+)
> create mode 100644 drivers/tty/serial/aspeed-vuart.c
>
> diff --git a/Documentation/devicetree/bindings/serial/8250.txt b/Documentation/devicetree/bindings/serial/8250.txt
> index f86bb06c39e9..a12e9277ac5d 100644
> --- a/Documentation/devicetree/bindings/serial/8250.txt
> +++ b/Documentation/devicetree/bindings/serial/8250.txt
> @@ -19,6 +19,8 @@ Required properties:
> - "altr,16550-FIFO128"
> - "fsl,16550-FIFO64"
> - "fsl,ns16550"
> + - "aspeed,ast2400-vuart"
> + - "aspeed,ast2500-vuart"
> - "serial" if the port type is unknown.
> - reg : offset and length of the register set for the device.
> - interrupts : should contain uart interrupt.
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index e9cf5b67f1b7..758b69a51078 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1129,6 +1129,16 @@ config SERIAL_NETX_CONSOLE
> If you have enabled the serial port on the Hilscher NetX SoC
> you can make it the console by answering Y to this option.
>
> +config SERIAL_ASPEED_VUART
> + tristate "Aspeed Virtual UART"
> + depends on OF
> + depends on SERIAL_8250
> + help
> + If you want to use the virtual UART (VUART) device on Aspeed
> + BMC platforms, enable this option. This enables the 16550A-
> + compatible device on the local LPC bus, giving a UART device
> + with no physical RS232 connections.
> +
> config SERIAL_OMAP
> tristate "OMAP serial port support"
> depends on ARCH_OMAP2PLUS
> diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile
> index 2d6288bc4554..5b97b0fa29e2 100644
> --- a/drivers/tty/serial/Makefile
> +++ b/drivers/tty/serial/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_SERIAL_8250) += 8250/
>
> obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o
> obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o
> +obj-$(CONFIG_SERIAL_ASPEED_VUART) += aspeed-vuart.o
> obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o
> obj-$(CONFIG_SERIAL_PXA_NON8250) += pxa.o
> obj-$(CONFIG_SERIAL_PNX8XXX) += pnx8xxx_uart.o
> diff --git a/drivers/tty/serial/aspeed-vuart.c b/drivers/tty/serial/aspeed-vuart.c
> new file mode 100644
> index 000000000000..fc6fa6d243c8
> --- /dev/null
> +++ b/drivers/tty/serial/aspeed-vuart.c
> @@ -0,0 +1,335 @@
> +/*
> + * Serial Port driver for Aspeed VUART device
> + *
> + * Copyright (C) 2016 Jeremy Kerr <jk-mnsaURCQ41sdnm+yROfE0A@public.gmane.org>, IBM Corp.
> + * Copyright (C) 2006 Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>, IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + *
> + */
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/clk.h>
> +
> +#include "8250/8250.h"
> +
> +#define AST_VUART_GCRA 0x20
> +#define AST_VUART_GCRA_VUART_EN 0x01
> +#define AST_VUART_GCRA_HOST_TX_DISCARD 0x20
> +#define AST_VUART_GCRB 0x24
> +#define AST_VUART_GCRB_HOST_SIRQ_MASK 0xf0
> +#define AST_VUART_GCRB_HOST_SIRQ_SHIFT 4
> +#define AST_VUART_ADDRL 0x28
> +#define AST_VUART_ADDRH 0x2c
> +
> +struct ast_vuart {
> + struct platform_device *pdev;
> + void __iomem *regs;
> + struct clk *clk;
> + int line;
> +};
> +
> +static ssize_t ast_vuart_show_addr(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ast_vuart *vuart = dev_get_drvdata(dev);
> + u16 addr;
> +
> + addr = (readb(vuart->regs + AST_VUART_ADDRH) << 8) |
> + (readb(vuart->regs + AST_VUART_ADDRL));
> +
> + return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr);
> +}
> +
> +static ssize_t ast_vuart_set_addr(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ast_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + writeb((val >> 8) & 0xff, vuart->regs + AST_VUART_ADDRH);
> + writeb((val >> 0) & 0xff, vuart->regs + AST_VUART_ADDRL);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(lpc_address, 0664,
> + ast_vuart_show_addr, ast_vuart_set_addr);
DEVICE_ATTR_RW()?
And why random sysfs files for a uart? Where have you documented these?
> +
> +static ssize_t ast_vuart_show_sirq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct ast_vuart *vuart = dev_get_drvdata(dev);
> + u8 reg;
> +
> + reg = readb(vuart->regs + AST_VUART_GCRB);
> + reg &= AST_VUART_GCRB_HOST_SIRQ_MASK;
> + reg >>= AST_VUART_GCRB_HOST_SIRQ_SHIFT;
> +
> + return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg);
> +}
> +
> +static ssize_t ast_vuart_set_sirq(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct ast_vuart *vuart = dev_get_drvdata(dev);
> + unsigned long val;
> + int err;
> + u8 reg;
> +
> + err = kstrtoul(buf, 0, &val);
> + if (err)
> + return err;
> +
> + val <<= AST_VUART_GCRB_HOST_SIRQ_SHIFT;
> + val &= AST_VUART_GCRB_HOST_SIRQ_MASK;
> +
> + reg = readb(vuart->regs + AST_VUART_GCRB);
> + reg &= ~AST_VUART_GCRB_HOST_SIRQ_MASK;
> + reg |= val;
> + writeb(reg, vuart->regs + AST_VUART_GCRB);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(sirq, 0664,
> + ast_vuart_show_sirq, ast_vuart_set_sirq);
DEVICE_ATTR_RW().
> +
> +static void ast_vuart_set_enabled(struct ast_vuart *vuart, bool enabled)
> +{
> + u8 reg;
> +
> + reg = readb(vuart->regs + AST_VUART_GCRA);
> + reg &= ~AST_VUART_GCRA_VUART_EN;
> + if (enabled)
> + reg |= AST_VUART_GCRA_VUART_EN;
> + writeb(reg, vuart->regs + AST_VUART_GCRA);
> +}
> +
> +static void ast_vuart_set_host_tx_discard(struct ast_vuart *vuart, bool discard)
> +{
> + u8 reg;
> +
> + reg = readb(vuart->regs + AST_VUART_GCRA);
> +
> + /* if the HOST_TX_DISCARD bit is set, discard is *disabled* */
> + reg &= ~AST_VUART_GCRA_HOST_TX_DISCARD;
> + if (!discard)
> + reg |= AST_VUART_GCRA_HOST_TX_DISCARD;
> +
> + writeb(reg, vuart->regs + AST_VUART_GCRA);
> +}
> +
> +static int ast_vuart_startup(struct uart_port *uart_port)
> +{
> + struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> + struct ast_vuart *vuart = uart_8250_port->port.private_data;
> + int rc;
> +
> + rc = serial8250_do_startup(uart_port);
> + if (rc)
> + return rc;
> +
> + ast_vuart_set_host_tx_discard(vuart, false);
> +
> + return 0;
> +}
> +
> +static void ast_vuart_shutdown(struct uart_port *uart_port)
> +{
> + struct uart_8250_port *uart_8250_port = up_to_u8250p(uart_port);
> + struct ast_vuart *vuart = uart_8250_port->port.private_data;
> +
> + ast_vuart_set_host_tx_discard(vuart, true);
> +
> + serial8250_do_shutdown(uart_port);
> +}
> +
> +
> +/**
> + * The device tree parsing code here is heavily based on that of the of_serial
> + * driver, but we have a few core differences, as we need to use our own
> + * ioremapping for extra register support
> + */
> +static int ast_vuart_probe(struct platform_device *pdev)
> +{
> + struct uart_8250_port port;
> + struct resource resource;
> + struct ast_vuart *vuart;
> + struct device_node *np;
> + u32 clk, prop;
> + int rc;
> +
> + np = pdev->dev.of_node;
> +
> + vuart = devm_kzalloc(&pdev->dev, sizeof(*vuart), GFP_KERNEL);
> + if (!vuart)
> + return -ENOMEM;
> +
> + vuart->pdev = pdev;
> + rc = of_address_to_resource(np, 0, &resource);
> + if (rc) {
> + dev_warn(&pdev->dev, "invalid address\n");
> + return rc;
> + }
> +
> + /* create our own mapping for VUART-specific registers */
> + vuart->regs = devm_ioremap_resource(&pdev->dev, &resource);
> + if (IS_ERR(vuart->regs)) {
> + dev_warn(&pdev->dev, "failed to map registers\n");
> + return PTR_ERR(vuart->regs);
> + }
> +
> + memset(&port, 0, sizeof(port));
> + port.port.private_data = vuart;
> + port.port.membase = vuart->regs;
> + port.port.mapbase = resource.start;
> + port.port.mapsize = resource_size(&resource);
> + port.port.startup = ast_vuart_startup;
> + port.port.shutdown = ast_vuart_shutdown;
> +
> + if (of_property_read_u32(np, "clock-frequency", &clk)) {
> + vuart->clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(vuart->clk)) {
> + dev_warn(&pdev->dev,
> + "clk or clock-frequency not defined\n");
> + return PTR_ERR(vuart->clk);
> + }
> +
> + rc = clk_prepare_enable(vuart->clk);
> + if (rc < 0)
> + return rc;
> +
> + clk = clk_get_rate(vuart->clk);
> + }
> +
> + /* If current-speed was set, then try not to change it. */
> + if (of_property_read_u32(np, "current-speed", &prop) == 0)
> + port.port.custom_divisor = clk / (16 * prop);
> +
> + /* Check for shifted address mapping */
> + if (of_property_read_u32(np, "reg-offset", &prop) == 0)
> + port.port.mapbase += prop;
> +
> + /* Check for registers offset within the devices address range */
> + if (of_property_read_u32(np, "reg-shift", &prop) == 0)
> + port.port.regshift = prop;
> +
> + /* Check for fifo size */
> + if (of_property_read_u32(np, "fifo-size", &prop) == 0)
> + port.port.fifosize = prop;
> +
> + /* Check for a fixed line number */
> + rc = of_alias_get_id(np, "serial");
> + if (rc >= 0)
> + port.port.line = rc;
> +
> + port.port.irq = irq_of_parse_and_map(np, 0);
> + port.port.irqflags = IRQF_SHARED;
> + port.port.iotype = UPIO_MEM;
> + if (of_property_read_u32(np, "reg-io-width", &prop) == 0) {
> + switch (prop) {
> + case 1:
> + port.port.iotype = UPIO_MEM;
> + break;
> + case 4:
> + port.port.iotype = of_device_is_big_endian(np) ?
> + UPIO_MEM32BE : UPIO_MEM32;
> + break;
> + default:
> + dev_warn(&pdev->dev, "unsupported reg-io-width (%d)\n",
> + prop);
> + rc = -EINVAL;
> + goto err_clk_disable;
> + }
> + }
> +
> + port.port.type = PORT_16550A;
> + port.port.uartclk = clk;
> + port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF
> + | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST;
> +
> + if (of_find_property(np, "no-loopback-test", NULL))
> + port.port.flags |= UPF_SKIP_TEST;
> +
> + port.port.dev = &pdev->dev;
> +
> + if (port.port.fifosize)
> + port.capabilities = UART_CAP_FIFO;
> +
> + if (of_property_read_bool(pdev->dev.of_node,
> + "auto-flow-control"))
> + port.capabilities |= UART_CAP_AFE;
> +
> + rc = serial8250_register_8250_port(&port);
> + if (rc < 0)
> + goto err_clk_disable;
> +
> +
> + vuart->line = rc;
> + ast_vuart_set_enabled(vuart, true);
> + ast_vuart_set_host_tx_discard(vuart, true);
> + platform_set_drvdata(pdev, vuart);
> +
> + /* extra sysfs control */
> + rc = device_create_file(&pdev->dev, &dev_attr_lpc_address);
> + if (rc)
> + dev_warn(&pdev->dev, "can't create lpc_address file\n");
> + rc = device_create_file(&pdev->dev, &dev_attr_sirq);
> + if (rc)
> + dev_warn(&pdev->dev, "can't create sirq file\n");
use an attribute group? You just raced userspace and lost :(
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/2] Move uart_register_driver call to device probe for pl010 and sh-sci
From: Greg Kroah-Hartman @ 2017-03-31 13:27 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Russell King - ARM Linux, Sjoerd Simons,
linux-serial@vger.kernel.org, Linux-Renesas,
linux-kernel@vger.kernel.org, Jiri Slaby
In-Reply-To: <CAMuHMdUkfzJw6W0jQZPChN+Uqb0Ewkn86o0Pzd+9+Wi1-XZRAw@mail.gmail.com>
On Sun, Mar 26, 2017 at 11:22:57AM +0200, Geert Uytterhoeven wrote:
> Hi Russell, Sjoerd,
>
> On Fri, Mar 24, 2017 at 5:42 PM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > On Fri, Mar 24, 2017 at 05:26:32PM +0100, Sjoerd Simons wrote:
> >> When testing on a Renesas board with the PL010 serial driver enabled
> >> serial output broke. Turns out the minor device numbers for both
> >> drivers happen to overlap, causing whichever driver happened to be the
> >> second one to register to fail.
> >
> > How the **** has the SH serial driver ended up with overlapping device
> > numbers?
>
> Interesting...
>
> > What happened to our maintained list of allocated major/minor device
> > numbers, which is supposed to stop crap like this happening?
>
> AMBA PL010 has been assigned major 204, minors 16..31,
> SCI has been assigned major 204, minors 8..11.
>
> Over the years, Renesas SoCs have been gaining more and more serial
> ports, leading to
>
> #define SCI_NPORTS CONFIG_SERIAL_SH_SCI_NR_UARTS
>
> with CONFIG_SERIAL_SH_SCI_NR_UARTS=20 in shmobile_defconfig and
> multi_v7_defconfig (although the maximum value on any supported SoC is 18).
>
> But once the value becomes 5 or more, it starts overflowing into the ttyFWx
> and ttyAMx space.
>
> How to solve this?
> Time for the serial subsystem to switch to dynamic minors, and get rid of the
> what-is-your-serial-port-called-again-on-this-platform
> multi-million-euro question?
Yes, please, we need to do that. Let's provide a build option for it,
like USB has had for over a decade. If it's enabled, it's all dynamic,
if not, the "old style" ones are used. Then we don't accept any new
reservations, making new drivers use the dynamic number, and over a long
time, all should be good.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3] serdev: Add serdev_device_write subroutine
From: Rob Herring @ 2017-03-31 13:22 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Chris Healy, Guenter Roeck, Andy Shevchenko,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170330220035.26649-1-andrew.smirnov@gmail.com>
On Thu, Mar 30, 2017 at 5:00 PM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> Add serdev_device_write() a blocking call allowing to transfer
> arbitraty amount of data (potentially exceeding amount that
> serdev_device_write_buf can process in a single call)
>
> To support that, also add serdev_device_write_wakeup().
>
> Drivers wanting to use full extent of serdev_device_write
> functionality are expected to provide serdev_device_write_wakeup() as
> a sole handler of .write_wakeup event or call it as a part of driver's
> custom .write_wakeup code.
>
> Because serdev_device_write() subroutine is a superset of
> serdev_device_write_buf() the patch re-impelements latter is terms of
> the former. For drivers watning to just use serdev_device_write_buf()
typo
> .write_wakeup handler is optional.
>
> Cc: cphealy@gmail.com
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>
> Changes since v2 (see [v2]):
>
> - Changed subject and commit message wording to better reflect
> nature of the patch
>
> - Various spelling, formatting, documentation and wording
> fixes as caught/suggested by Andy
>
> Changes since v1 (see [v1]):
>
> - Make timeout to be a total(as opposed to per-iteration)
> timeout
>
> - Keep serdev_device_write_buf() as a wrapper arount
> serdev_device_write() for compatibility
>
> [v2] https://lkml.org/lkml/2017/3/28/942
> [v1] https://lkml.org/lkml/2017/3/20/650
>
> drivers/tty/serdev/core.c | 36 +++++++++++++++++++++++++++++++-----
> include/linux/serdev.h | 17 +++++++++++++++--
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f4c6c90..6701d10 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -116,17 +116,41 @@ void serdev_device_close(struct serdev_device *serdev)
> }
> EXPORT_SYMBOL_GPL(serdev_device_close);
>
> -int serdev_device_write_buf(struct serdev_device *serdev,
> - const unsigned char *buf, size_t count)
> +void serdev_device_write_wakeup(struct serdev_device *serdev)
> +{
> + complete(&serdev->write_comp);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
> +
> +int serdev_device_write(struct serdev_device *serdev,
> + const unsigned char *buf, size_t count,
> + unsigned long timeout)
> {
> struct serdev_controller *ctrl = serdev->ctrl;
> + int ret;
>
> - if (!ctrl || !ctrl->ops->write_buf)
> + if (!ctrl || !ctrl->ops->write_buf ||
> + (timeout && !serdev->ops->write_wakeup))
> return -EINVAL;
>
> - return ctrl->ops->write_buf(ctrl, buf, count);
> + mutex_lock(&serdev->write_lock);
> + do {
> + reinit_completion(&serdev->write_comp);
> +
> + ret = ctrl->ops->write_buf(ctrl, buf, count);
> + if (ret < 0)
> + break;
> +
> + buf += ret;
> + count -= ret;
> +
> + } while (count &&
> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
> + timeout)));
Need to test for timeout < 0 here and return timeout when < 0.
Rob
^ permalink raw reply
* Re: [PATCH] serial: tegra: Map the iir register to default defines
From: Greg Kroah-Hartman @ 2017-03-31 13:21 UTC (permalink / raw)
To: Olliver Schinagl
Cc: Jon Hunter, Laxman Dewangan, Jiri Slaby, Stephen Warren,
Thierry Reding, Alexandre Courbot, linux-serial, linux-tegra,
linux-kernel
In-Reply-To: <0DEBCDAD-C7D3-4DB9-B5F5-408951165662@schinagl.nl>
On Thu, Mar 30, 2017 at 05:37:41PM +0200, Olliver Schinagl wrote:
> Hey Jon,
>
> On March 30, 2017 3:42:19 PM CEST, Jon Hunter <jonathanh@nvidia.com> wrote:
> >
> >On 29/03/17 19:48, Olliver Schinagl wrote:
> >> The tegra serial IP seems to be following the common layout and the
> >> interrupt ID's match up nicely. Replace the magic values to match the
> >> common serial_reg defines, with the addition of the Tegra unique End
> >of
> >> Data interrupt.
> >>
> >> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
> >> ---
> >> Note I do not own any tegra hardware and just noticed it while
> >working on my
> >> somewhat related previous patch,
> >> "serial: Do not treat the IIR register as a bitfield"
> >>
> >> As such, this patch can only be applied after the aforementioned
> >patch or the
> >> iir variable will not have its mask applied yet.
> >
> >Nit-pick. If this is the case, then this should really be part of a
> >patch series so it is obvious to everyone that this should only be
> >applied after the other patch.
> Yes, and it was, but I did not want to have the really big list of names in this much smaller group.
Ok, this is a mess, don't send me patches that need to be applied in a
specific order, yet are not obviously linked together in a single
series.
How do you expect a maintainer to handle this type of stuff? You need
to make it _OBVIOUS_ as to what I need to do here, otherwise I will get
it wrong.
I'm going to drop all of your patches from my queue and wait for a
resend with the correct order, and ones that work properly, you can do
better than this :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v3] serdev: Add serdev_device_write subroutine
From: Andy Shevchenko @ 2017-03-31 11:43 UTC (permalink / raw)
To: Andrey Smirnov
Cc: Rob Herring, Chris Healy, Guenter Roeck,
linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20170330220035.26649-1-andrew.smirnov@gmail.com>
On Fri, Mar 31, 2017 at 1:00 AM, Andrey Smirnov
<andrew.smirnov@gmail.com> wrote:
> Add serdev_device_write() a blocking call allowing to transfer
> arbitraty amount of data (potentially exceeding amount that
> serdev_device_write_buf can process in a single call)
>
> To support that, also add serdev_device_write_wakeup().
>
> Drivers wanting to use full extent of serdev_device_write
> functionality are expected to provide serdev_device_write_wakeup() as
> a sole handler of .write_wakeup event or call it as a part of driver's
> custom .write_wakeup code.
>
> Because serdev_device_write() subroutine is a superset of
> serdev_device_write_buf() the patch re-impelements latter is terms of
> the former. For drivers watning to just use serdev_device_write_buf()
> .write_wakeup handler is optional.
>
There are still style inconveniences I notice, but let me avoid
bikeshedding here, so
FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: cphealy@gmail.com
> Cc: Guenter Roeck <linux@roeck-us.net>
> Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
> Cc: linux-serial@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
> ---
>
> Changes since v2 (see [v2]):
>
> - Changed subject and commit message wording to better reflect
> nature of the patch
>
> - Various spelling, formatting, documentation and wording
> fixes as caught/suggested by Andy
>
> Changes since v1 (see [v1]):
>
> - Make timeout to be a total(as opposed to per-iteration)
> timeout
>
> - Keep serdev_device_write_buf() as a wrapper arount
> serdev_device_write() for compatibility
>
> [v2] https://lkml.org/lkml/2017/3/28/942
> [v1] https://lkml.org/lkml/2017/3/20/650
>
> drivers/tty/serdev/core.c | 36 +++++++++++++++++++++++++++++++-----
> include/linux/serdev.h | 17 +++++++++++++++--
> 2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index f4c6c90..6701d10 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -116,17 +116,41 @@ void serdev_device_close(struct serdev_device *serdev)
> }
> EXPORT_SYMBOL_GPL(serdev_device_close);
>
> -int serdev_device_write_buf(struct serdev_device *serdev,
> - const unsigned char *buf, size_t count)
> +void serdev_device_write_wakeup(struct serdev_device *serdev)
> +{
> + complete(&serdev->write_comp);
> +}
> +EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
> +
> +int serdev_device_write(struct serdev_device *serdev,
> + const unsigned char *buf, size_t count,
> + unsigned long timeout)
> {
> struct serdev_controller *ctrl = serdev->ctrl;
> + int ret;
>
> - if (!ctrl || !ctrl->ops->write_buf)
> + if (!ctrl || !ctrl->ops->write_buf ||
> + (timeout && !serdev->ops->write_wakeup))
> return -EINVAL;
>
> - return ctrl->ops->write_buf(ctrl, buf, count);
> + mutex_lock(&serdev->write_lock);
> + do {
> + reinit_completion(&serdev->write_comp);
> +
> + ret = ctrl->ops->write_buf(ctrl, buf, count);
> + if (ret < 0)
> + break;
> +
> + buf += ret;
> + count -= ret;
> +
> + } while (count &&
> + (timeout = wait_for_completion_timeout(&serdev->write_comp,
> + timeout)));
> + mutex_unlock(&serdev->write_lock);
> + return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
> }
> -EXPORT_SYMBOL_GPL(serdev_device_write_buf);
> +EXPORT_SYMBOL_GPL(serdev_device_write);
>
> void serdev_device_write_flush(struct serdev_device *serdev)
> {
> @@ -232,6 +256,8 @@ struct serdev_device *serdev_device_alloc(struct serdev_controller *ctrl)
> serdev->dev.parent = &ctrl->dev;
> serdev->dev.bus = &serdev_bus_type;
> serdev->dev.type = &serdev_device_type;
> + init_completion(&serdev->write_comp);
> + mutex_init(&serdev->write_lock);
> return serdev;
> }
> EXPORT_SYMBOL_GPL(serdev_device_alloc);
> diff --git a/include/linux/serdev.h b/include/linux/serdev.h
> index 5176cdc..0beaff8 100644
> --- a/include/linux/serdev.h
> +++ b/include/linux/serdev.h
> @@ -39,12 +39,16 @@ struct serdev_device_ops {
> * @nr: Device number on serdev bus.
> * @ctrl: serdev controller managing this device.
> * @ops: Device operations.
> + * @write_comp Completion used by serdev_device_write() internally
> + * @write_lock Lock to serialize access when writing data
> */
> struct serdev_device {
> struct device dev;
> int nr;
> struct serdev_controller *ctrl;
> const struct serdev_device_ops *ops;
> + struct completion write_comp;
> + struct mutex write_lock;
> };
>
> static inline struct serdev_device *to_serdev_device(struct device *d)
> @@ -186,7 +190,8 @@ int serdev_device_open(struct serdev_device *);
> void serdev_device_close(struct serdev_device *);
> unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
> void serdev_device_set_flow_control(struct serdev_device *, bool);
> -int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
> +void serdev_device_write_wakeup(struct serdev_device *);
> +int serdev_device_write(struct serdev_device *, const unsigned char *, size_t, unsigned long);
> void serdev_device_write_flush(struct serdev_device *);
> int serdev_device_write_room(struct serdev_device *);
>
> @@ -223,7 +228,8 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
> return 0;
> }
> static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
> -static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
> +static inline int serdev_device_write(struct serdev_device *sdev, const unsigned char *buf,
> + size_t count, unsigned long timeout)
> {
> return -ENODEV;
> }
> @@ -259,4 +265,11 @@ static inline struct device *serdev_tty_port_register(struct tty_port *port,
> static inline void serdev_tty_port_unregister(struct tty_port *port) {}
> #endif /* CONFIG_SERIAL_DEV_CTRL_TTYPORT */
>
> +static inline int serdev_device_write_buf(struct serdev_device *serdev,
> + const unsigned char *data,
> + size_t count)
> +{
> + return serdev_device_write(serdev, data, count, 0);
> +}
> +
> #endif /*_LINUX_SERDEV_H */
> --
> 2.9.3
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH] serial: tegra: Map the iir register to default defines
From: Olliver Schinagl @ 2017-03-31 11:32 UTC (permalink / raw)
To: Shardar Mohammed, Jonathan Hunter, Laxman Dewangan,
Greg Kroah-Hartman, Jiri Slaby, Stephen Warren, Thierry Reding,
Alexandre Courbot
Cc: linux-serial@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <79efc1286adc4119be65e0b237aebbc3@bgmail102.nvidia.com>
Hey Shadar,
On 31-03-17 12:42, Shardar Mohammed wrote:
>> On 31/03/17 11:07, Shardar Mohammed wrote:
>>> Verification failed on Tegra.
>>> Fix here is, IIR should be masked with UART_IIR_MASK after reading the IIR
>> register as on Tegra bit-6 is used for internal usage to know if FIFO mode is
>> enabled.
>>> while (1) {
>>> iir = tegra_uart_read(tup, UART_IIR);
>>> +iir &= UART_IIR_MASK;
>>
>> Per Olliver's original email did you pick up the other patch [0] before applying
>> this because that does apply the mask. I mentioned to Olliver that this should
>> really be a series, so it is clear that this patch is dependent upon the other.
>
> For UART_IIR_MASK macro, dependent patch is required if we add this required line.
> But here to fix the issue with the current patch above masking step is necessary.
Yeah the other big patch adds this line (and does some other fixes to
the tegra driver with regards to the UART_IIR_MASK).
I did not make it a 'set' because the other patch has a big big list of
names and touches a lot of serial devices. So both 'groups' would get a
lot of noise. I'm sorry if this caused confusion. If you guys prefer,
I'll turn it into a set.
Again, sorry for the inconvenience,
Olliver
>
>>
>>> -----Original Message-----
>>> From: Laxman Dewangan
>>> Sent: Thursday, March 30, 2017 3:48 PM
>>> To: Olliver Schinagl <oliver@schinagl.nl>; Greg Kroah-Hartman
>>> <gregkh@linuxfoundation.org>; Jiri Slaby <jslaby@suse.com>; Stephen
>>> Warren <swarren@wwwdotorg.org>; Thierry Reding
>>> <thierry.reding@gmail.com>; Alexandre Courbot <gnurou@gmail.com>
>>> Cc: linux-serial@vger.kernel.org; linux-tegra@vger.kernel.org;
>>> linux-kernel@vger.kernel.org; Shardar Mohammed
>> <smohammed@nvidia.com>
>>> Subject: Re: [PATCH] serial: tegra: Map the iir register to default
>>> defines
>>>
>>>
>>> On Thursday 30 March 2017 12:18 AM, Olliver Schinagl wrote:
>>>> The tegra serial IP seems to be following the common layout and the
>>>> interrupt ID's match up nicely. Replace the magic values to match the
>>>> common serial_reg defines, with the addition of the Tegra unique End
>>>> of Data interrupt.
>>>>
>>>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>>>> ---
>>>
>>> Adding Shardar for verifications.
>>>
>>> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
>>
>> Furthermore does this ACK imply that you have reviewed the other patch this
>> one is dependent upon?
>>
> Yes dependent change looks fine, but I have not tested it.
>
>> Cheers
>> Jon
>>
>> [0] http://marc.info/?l=linux-serial&m=149081309627392&w=2
>>
>> --
>> nvpublic
^ permalink raw reply
* RE: [PATCH] serial: tegra: Map the iir register to default defines
From: Shardar Mohammed @ 2017-03-31 10:42 UTC (permalink / raw)
To: Jonathan Hunter, Laxman Dewangan, Olliver Schinagl,
Greg Kroah-Hartman, Jiri Slaby, Stephen Warren, Thierry Reding,
Alexandre Courbot
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <c904007d-af59-c1fa-d11f-a69c4609ff84-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> On 31/03/17 11:07, Shardar Mohammed wrote:
> > Verification failed on Tegra.
> > Fix here is, IIR should be masked with UART_IIR_MASK after reading the IIR
> register as on Tegra bit-6 is used for internal usage to know if FIFO mode is
> enabled.
> > while (1) {
> > iir = tegra_uart_read(tup, UART_IIR);
> > +iir &= UART_IIR_MASK;
>
> Per Olliver's original email did you pick up the other patch [0] before applying
> this because that does apply the mask. I mentioned to Olliver that this should
> really be a series, so it is clear that this patch is dependent upon the other.
For UART_IIR_MASK macro, dependent patch is required if we add this required line.
But here to fix the issue with the current patch above masking step is necessary.
>
> > -----Original Message-----
> > From: Laxman Dewangan
> > Sent: Thursday, March 30, 2017 3:48 PM
> > To: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>; Greg Kroah-Hartman
> > <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>; Jiri Slaby <jslaby-IBi9RG/b67k@public.gmane.org>; Stephen
> > Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>; Thierry Reding
> > <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Shardar Mohammed
> <smohammed-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > Subject: Re: [PATCH] serial: tegra: Map the iir register to default
> > defines
> >
> >
> > On Thursday 30 March 2017 12:18 AM, Olliver Schinagl wrote:
> >> The tegra serial IP seems to be following the common layout and the
> >> interrupt ID's match up nicely. Replace the magic values to match the
> >> common serial_reg defines, with the addition of the Tegra unique End
> >> of Data interrupt.
> >>
> >> Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
> >> ---
> >
> > Adding Shardar for verifications.
> >
> > Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> Furthermore does this ACK imply that you have reviewed the other patch this
> one is dependent upon?
>
Yes dependent change looks fine, but I have not tested it.
> Cheers
> Jon
>
> [0] http://marc.info/?l=linux-serial&m=149081309627392&w=2
>
> --
> nvpublic
^ permalink raw reply
* Re: [PATCH] serial: tegra: Map the iir register to default defines
From: Jon Hunter @ 2017-03-31 10:28 UTC (permalink / raw)
To: Shardar Mohammed, Laxman Dewangan, Olliver Schinagl,
Greg Kroah-Hartman, Jiri Slaby, Stephen Warren, Thierry Reding,
Alexandre Courbot
Cc: linux-serial@vger.kernel.org, linux-tegra@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <83c51151685d4be7926f7f3982c59db8@bgmail102.nvidia.com>
On 31/03/17 11:07, Shardar Mohammed wrote:
> Verification failed on Tegra.
> Fix here is, IIR should be masked with UART_IIR_MASK after reading the IIR register as on Tegra bit-6 is used for internal usage to know if FIFO mode is enabled.
> while (1) {
> iir = tegra_uart_read(tup, UART_IIR);
> +iir &= UART_IIR_MASK;
Per Olliver's original email did you pick up the other patch [0] before
applying this because that does apply the mask. I mentioned to Olliver
that this should really be a series, so it is clear that this patch is
dependent upon the other.
> -----Original Message-----
> From: Laxman Dewangan
> Sent: Thursday, March 30, 2017 3:48 PM
> To: Olliver Schinagl <oliver@schinagl.nl>; Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Jiri Slaby <jslaby@suse.com>; Stephen Warren <swarren@wwwdotorg.org>; Thierry Reding <thierry.reding@gmail.com>; Alexandre Courbot <gnurou@gmail.com>
> Cc: linux-serial@vger.kernel.org; linux-tegra@vger.kernel.org; linux-kernel@vger.kernel.org; Shardar Mohammed <smohammed@nvidia.com>
> Subject: Re: [PATCH] serial: tegra: Map the iir register to default defines
>
>
> On Thursday 30 March 2017 12:18 AM, Olliver Schinagl wrote:
>> The tegra serial IP seems to be following the common layout and the
>> interrupt ID's match up nicely. Replace the magic values to match the
>> common serial_reg defines, with the addition of the Tegra unique End
>> of Data interrupt.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>
> Adding Shardar for verifications.
>
> Acked-by: Laxman Dewangan <ldewangan@nvidia.com>
Furthermore does this ACK imply that you have reviewed the other patch
this one is dependent upon?
Cheers
Jon
[0] http://marc.info/?l=linux-serial&m=149081309627392&w=2
--
nvpublic
^ permalink raw reply
* RE: [PATCH] serial: tegra: Map the iir register to default defines
From: Shardar Mohammed @ 2017-03-31 10:07 UTC (permalink / raw)
To: Laxman Dewangan, Olliver Schinagl, Greg Kroah-Hartman, Jiri Slaby,
Stephen Warren, Thierry Reding, Alexandre Courbot
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <58DCDB54.5040005-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Verification failed on Tegra.
Fix here is, IIR should be masked with UART_IIR_MASK after reading the IIR register as on Tegra bit-6 is used for internal usage to know if FIFO mode is enabled.
while (1) {
iir = tegra_uart_read(tup, UART_IIR);
+iir &= UART_IIR_MASK;
Thanks,
Shardar
-----Original Message-----
From: Laxman Dewangan
Sent: Thursday, March 30, 2017 3:48 PM
To: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>; Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>; Jiri Slaby <jslaby-IBi9RG/b67k@public.gmane.org>; Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>; Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>; Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Shardar Mohammed <smohammed-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Subject: Re: [PATCH] serial: tegra: Map the iir register to default defines
On Thursday 30 March 2017 12:18 AM, Olliver Schinagl wrote:
> The tegra serial IP seems to be following the common layout and the
> interrupt ID's match up nicely. Replace the magic values to match the
> common serial_reg defines, with the addition of the Tegra unique End
> of Data interrupt.
>
> Signed-off-by: Olliver Schinagl <oliver-dxLnbx3+1qmEVqv0pETR8A@public.gmane.org>
> ---
Adding Shardar for verifications.
Acked-by: Laxman Dewangan <ldewangan-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply
* [PATCH v3] serdev: Add serdev_device_write subroutine
From: Andrey Smirnov @ 2017-03-30 22:00 UTC (permalink / raw)
To: Rob Herring
Cc: Andrey Smirnov, cphealy, Guenter Roeck, Andy Shevchenko,
linux-serial, linux-kernel
Add serdev_device_write() a blocking call allowing to transfer
arbitraty amount of data (potentially exceeding amount that
serdev_device_write_buf can process in a single call)
To support that, also add serdev_device_write_wakeup().
Drivers wanting to use full extent of serdev_device_write
functionality are expected to provide serdev_device_write_wakeup() as
a sole handler of .write_wakeup event or call it as a part of driver's
custom .write_wakeup code.
Because serdev_device_write() subroutine is a superset of
serdev_device_write_buf() the patch re-impelements latter is terms of
the former. For drivers watning to just use serdev_device_write_buf()
.write_wakeup handler is optional.
Cc: cphealy@gmail.com
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: linux-serial@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
---
Changes since v2 (see [v2]):
- Changed subject and commit message wording to better reflect
nature of the patch
- Various spelling, formatting, documentation and wording
fixes as caught/suggested by Andy
Changes since v1 (see [v1]):
- Make timeout to be a total(as opposed to per-iteration)
timeout
- Keep serdev_device_write_buf() as a wrapper arount
serdev_device_write() for compatibility
[v2] https://lkml.org/lkml/2017/3/28/942
[v1] https://lkml.org/lkml/2017/3/20/650
drivers/tty/serdev/core.c | 36 +++++++++++++++++++++++++++++++-----
include/linux/serdev.h | 17 +++++++++++++++--
2 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
index f4c6c90..6701d10 100644
--- a/drivers/tty/serdev/core.c
+++ b/drivers/tty/serdev/core.c
@@ -116,17 +116,41 @@ void serdev_device_close(struct serdev_device *serdev)
}
EXPORT_SYMBOL_GPL(serdev_device_close);
-int serdev_device_write_buf(struct serdev_device *serdev,
- const unsigned char *buf, size_t count)
+void serdev_device_write_wakeup(struct serdev_device *serdev)
+{
+ complete(&serdev->write_comp);
+}
+EXPORT_SYMBOL_GPL(serdev_device_write_wakeup);
+
+int serdev_device_write(struct serdev_device *serdev,
+ const unsigned char *buf, size_t count,
+ unsigned long timeout)
{
struct serdev_controller *ctrl = serdev->ctrl;
+ int ret;
- if (!ctrl || !ctrl->ops->write_buf)
+ if (!ctrl || !ctrl->ops->write_buf ||
+ (timeout && !serdev->ops->write_wakeup))
return -EINVAL;
- return ctrl->ops->write_buf(ctrl, buf, count);
+ mutex_lock(&serdev->write_lock);
+ do {
+ reinit_completion(&serdev->write_comp);
+
+ ret = ctrl->ops->write_buf(ctrl, buf, count);
+ if (ret < 0)
+ break;
+
+ buf += ret;
+ count -= ret;
+
+ } while (count &&
+ (timeout = wait_for_completion_timeout(&serdev->write_comp,
+ timeout)));
+ mutex_unlock(&serdev->write_lock);
+ return ret < 0 ? ret : (count ? -ETIMEDOUT : 0);
}
-EXPORT_SYMBOL_GPL(serdev_device_write_buf);
+EXPORT_SYMBOL_GPL(serdev_device_write);
void serdev_device_write_flush(struct serdev_device *serdev)
{
@@ -232,6 +256,8 @@ struct serdev_device *serdev_device_alloc(struct serdev_controller *ctrl)
serdev->dev.parent = &ctrl->dev;
serdev->dev.bus = &serdev_bus_type;
serdev->dev.type = &serdev_device_type;
+ init_completion(&serdev->write_comp);
+ mutex_init(&serdev->write_lock);
return serdev;
}
EXPORT_SYMBOL_GPL(serdev_device_alloc);
diff --git a/include/linux/serdev.h b/include/linux/serdev.h
index 5176cdc..0beaff8 100644
--- a/include/linux/serdev.h
+++ b/include/linux/serdev.h
@@ -39,12 +39,16 @@ struct serdev_device_ops {
* @nr: Device number on serdev bus.
* @ctrl: serdev controller managing this device.
* @ops: Device operations.
+ * @write_comp Completion used by serdev_device_write() internally
+ * @write_lock Lock to serialize access when writing data
*/
struct serdev_device {
struct device dev;
int nr;
struct serdev_controller *ctrl;
const struct serdev_device_ops *ops;
+ struct completion write_comp;
+ struct mutex write_lock;
};
static inline struct serdev_device *to_serdev_device(struct device *d)
@@ -186,7 +190,8 @@ int serdev_device_open(struct serdev_device *);
void serdev_device_close(struct serdev_device *);
unsigned int serdev_device_set_baudrate(struct serdev_device *, unsigned int);
void serdev_device_set_flow_control(struct serdev_device *, bool);
-int serdev_device_write_buf(struct serdev_device *, const unsigned char *, size_t);
+void serdev_device_write_wakeup(struct serdev_device *);
+int serdev_device_write(struct serdev_device *, const unsigned char *, size_t, unsigned long);
void serdev_device_write_flush(struct serdev_device *);
int serdev_device_write_room(struct serdev_device *);
@@ -223,7 +228,8 @@ static inline unsigned int serdev_device_set_baudrate(struct serdev_device *sdev
return 0;
}
static inline void serdev_device_set_flow_control(struct serdev_device *sdev, bool enable) {}
-static inline int serdev_device_write_buf(struct serdev_device *sdev, const unsigned char *buf, size_t count)
+static inline int serdev_device_write(struct serdev_device *sdev, const unsigned char *buf,
+ size_t count, unsigned long timeout)
{
return -ENODEV;
}
@@ -259,4 +265,11 @@ static inline struct device *serdev_tty_port_register(struct tty_port *port,
static inline void serdev_tty_port_unregister(struct tty_port *port) {}
#endif /* CONFIG_SERIAL_DEV_CTRL_TTYPORT */
+static inline int serdev_device_write_buf(struct serdev_device *serdev,
+ const unsigned char *data,
+ size_t count)
+{
+ return serdev_device_write(serdev, data, count, 0);
+}
+
#endif /*_LINUX_SERDEV_H */
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] serial: Do not treat the IIR register as a bitfield
From: Olliver Schinagl @ 2017-03-30 15:39 UTC (permalink / raw)
To: Vignesh R, Greg Kroah-Hartman, Jiri Slaby, Laxman Dewangan,
Stephen Warren, Thierry Reding, Alexandre Courbot,
David S . Miller
Cc: dev@linux-sunxi.org, Ed Blake, Andy Shevchenko,
Alexander Sverdlin, Yegor Yefremov, Wan Ahmad Zainie, Kefeng Wang,
Heikki Krogerus, Heiko Stuebner, Jason Uy, Douglas Anderson,
Peter Hurley, Tony Lindgren, Thor Thayer, David Lechner,
Jan Kiszka, linux-serial@vger.kernel.org, linux-kernel@
In-Reply-To: <2faa7420-0a54-e77f-0da2-80f6dfcf9710@ti.com>
Hey Vignesh,
On March 30, 2017 9:57:19 AM CEST, Vignesh R <vigneshr@ti.com> wrote:
>
>
>On Thursday 30 March 2017 12:13 PM, Olliver Schinagl wrote:
>>
>>
>> On March 30, 2017 8:15:29 AM CEST, Vignesh R <vigneshr@ti.com> wrote:
>>> Hi,
>>>
>>> On Thursday 30 March 2017 12:14 AM, Olliver Schinagl wrote:
>>>> diff --git a/include/uapi/linux/serial_reg.h
>>> b/include/uapi/linux/serial_reg.h
>>>> index 5db76880b4ad..489522389a10 100644
>>>> --- a/include/uapi/linux/serial_reg.h
>>>> +++ b/include/uapi/linux/serial_reg.h
>>>> @@ -31,18 +31,18 @@
>>>> #define UART_IERX_SLEEP 0x10 /* Enable sleep mode */
>>>>
>>>> #define UART_IIR 2 /* In: Interrupt ID Register */
>>>> -#define UART_IIR_NO_INT 0x01 /* No interrupts pending */
>>>> -#define UART_IIR_ID 0x0e /* Mask for the interrupt ID */
>>>> #define UART_IIR_MSI 0x00 /* Modem status interrupt */
>>>> +#define UART_IIR_NO_INT 0x01 /* No interrupts pending */
>>>> #define UART_IIR_THRI 0x02 /* Transmitter holding register empty
>*/
>>>> #define UART_IIR_RDI 0x04 /* Receiver data interrupt */
>>>> #define UART_IIR_RLSI 0x06 /* Receiver line status interrupt */
>>>> -
>>>> #define UART_IIR_BUSY 0x07 /* DesignWare APB Busy Detect */
>>>> +#define UART_IIR_RX_TIMEOUT 0x0c /* DesignWare RX Timeout
>interrupt
>>> */
>> It was moved due to sorting. The comment could be changed maybe? I
>renamed it from omap to dw as i believe the omap also uses the dw ip.
>But i think it is a defacto standard mapping of the irr register?
>
>AFAIK, OMAP UART does not use DW core, please make these IDs generic to
>avoid confusion.
The ids are genetic, the comments are not. Ill update the comments to be generic, with the exception of the omap specific extended ones?
Olliver
>
>>
>>>> +#define UART_IIR_MASK 0x0f /* DesignWare IIR mask */
>>>>
>>>> -#define UART_IIR_RX_TIMEOUT 0x0c /* OMAP RX Timeout interrupt */
>>>
>>> You are removing UART_IIR_RX_TIMEOUT? Is this intended?
>>>
>>>> #define UART_IIR_XOFF 0x10 /* OMAP XOFF/Special Character */
>>>> #define UART_IIR_CTS_RTS_DSR 0x20 /* OMAP CTS/RTS/DSR Change */
>>>> +#define UART_IIR_EXT_MASK 0x30 /* OMAP extended IIR mask */
>>
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH] serial: tegra: Map the iir register to default defines
From: Olliver Schinagl @ 2017-03-30 15:37 UTC (permalink / raw)
To: Jon Hunter, Laxman Dewangan, Greg Kroah-Hartman, Jiri Slaby,
Stephen Warren, Thierry Reding, Alexandre Courbot
Cc: linux-serial, linux-tegra, linux-kernel
In-Reply-To: <9eed3c8e-a7af-1e1f-659a-a092a89d1679@nvidia.com>
Hey Jon,
On March 30, 2017 3:42:19 PM CEST, Jon Hunter <jonathanh@nvidia.com> wrote:
>
>On 29/03/17 19:48, Olliver Schinagl wrote:
>> The tegra serial IP seems to be following the common layout and the
>> interrupt ID's match up nicely. Replace the magic values to match the
>> common serial_reg defines, with the addition of the Tegra unique End
>of
>> Data interrupt.
>>
>> Signed-off-by: Olliver Schinagl <oliver@schinagl.nl>
>> ---
>> Note I do not own any tegra hardware and just noticed it while
>working on my
>> somewhat related previous patch,
>> "serial: Do not treat the IIR register as a bitfield"
>>
>> As such, this patch can only be applied after the aforementioned
>patch or the
>> iir variable will not have its mask applied yet.
>
>Nit-pick. If this is the case, then this should really be part of a
>patch series so it is obvious to everyone that this should only be
>applied after the other patch.
Yes, and it was, but I did not want to have the really big list of names in this much smaller group.
>
>Cheers
>Jon
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ 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