* Re: [RFC PATCH v1 13/25] printk: track seq per console
From: Petr Mladek @ 2019-02-25 14:59 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-14-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:51, John Ogness wrote:
> Allow each console to track which seq record was last printed. This
> simplifies identifying dropped records.
I would like to see a better justification here. And I think
that it is part of this patchset.
One reason is to implement console reply a cleaner way.
Another reason is to allows calling lockless consoles
directly in emergency situations.
Both reasons are independent on the log buffer implementation.
Therefore I suggest to move it into another patchset.
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index ece54c24ea0d..ebd9aac06323 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1504,6 +1514,19 @@ static void call_console_drivers(const char *ext_text, size_t ext_len,
> if (!cpu_online(raw_smp_processor_id()) &&
> !(con->flags & CON_ANYTIME))
> continue;
> + if (con->printk_seq >= seq)
> + continue;
> +
> + con->printk_seq++;
> + if (con->printk_seq < seq) {
> + print_console_dropped(con, seq - con->printk_seq);
> + con->printk_seq = seq;
It would be great to print this message only when the real one
is not superseded.
The suppressed messages can be proceed quickly. They allow consoles
to catch up with the message producers. But if the spend time
with printing the warning, we just risk loosing more messages
again and again.
Heh, there is a bug in the current code. The warning is not
printed when the currently proceed message is superseded.
I am going to prepare a patch for this.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 12/25] printk: minimize console locking implementation
From: Petr Mladek @ 2019-02-25 13:44 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-13-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:50, John Ogness wrote:
> Since printing of the printk buffer is now handled by the printk
> kthread, minimize the console locking functions to just handle
> locking of the console.
>
> NOTE: With this console_flush_on_panic will no longer flush.
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> kernel/printk/printk.c | 255 +------------------------------------------------
> 1 file changed, 1 insertion(+), 254 deletions(-)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 073ff9fd6872..ece54c24ea0d 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -209,19 +209,7 @@ static int nr_ext_console_drivers;
>
> static int __down_trylock_console_sem(unsigned long ip)
> {
> - int lock_failed;
> - unsigned long flags;
> -
> - /*
> - * Here and in __up_console_sem() we need to be in safe mode,
> - * because spindump/WARN/etc from under console ->lock will
> - * deadlock in printk()->down_trylock_console_sem() otherwise.
> - */
> - printk_safe_enter_irqsave(flags);
> - lock_failed = down_trylock(&console_sem);
> - printk_safe_exit_irqrestore(flags);
> -
> - if (lock_failed)
> + if (down_trylock(&console_sem))
> return 1;
> mutex_acquire(&console_lock_dep_map, 0, 1, ip);
> return 0;
> @@ -230,13 +218,9 @@ static int __down_trylock_console_sem(unsigned long ip)
>
> static void __up_console_sem(unsigned long ip)
> {
> - unsigned long flags;
> -
> mutex_release(&console_lock_dep_map, 1, ip);
>
> - printk_safe_enter_irqsave(flags);
> up(&console_sem);
> - printk_safe_exit_irqrestore(flags);
> }
> #define up_console_sem() __up_console_sem(_RET_IP_)
>
It might be obvious from the previous mails. But just to be sure.
I would remove printk_safe stuff in one patch after switching
to the new ring buffer implementation.
> @@ -1498,82 +1482,6 @@ static void format_text(struct printk_log *msg, u64 seq,
> }
>
> /*
> - * Special console_lock variants that help to reduce the risk of soft-lockups.
> - * They allow to pass console_lock to another printk() call using a busy wait.
> - */
[...]
> -static void console_lock_spinning_enable(void)
The console waiter logic is another story. It can get removed only
after we have a reasonable alternative. That means an acceptable
offload that handles emergency situations and sudden death
reasonable well.
I would move this into a separate patchset.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH v3 7/9] dt-bindings: serial: Add Milbeaut serial driver description
From: Sugaya, Taichi @ 2019-02-25 12:13 UTC (permalink / raw)
To: Rob Herring
Cc: linux-serial, devicetree, linux-kernel, linux-arm-kernel, soc,
Greg Kroah-Hartman, Mark Rutland, Takao Orito, Kazuhiro Kasai,
Shinji Kanematsu, Jassi Brar, Masami Hiramatsu
In-Reply-To: <20190222183812.GA20616@bogus>
Hi Rob,
On 2019/02/23 3:38, Rob Herring wrote:
> On Wed, 20 Feb 2019 16:44:37 +0900, Sugaya Taichi wrote:
>> Add DT bindings document for Milbeaut serial driver.
>>
>> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
>> ---
>> .../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>>
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>
Thank you!
^ permalink raw reply
* Re: [RFC PATCH v1 10/25] printk: redirect emit/store to new ringbuffer
From: Petr Mladek @ 2019-02-25 12:11 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <8736oijgpf.fsf@linutronix.de>
On Wed 2019-02-20 22:25:00, John Ogness wrote:
> On 2019-02-20, Petr Mladek <pmladek@suse.com> wrote:
> >> vprintk_emit and vprintk_store are the main functions that all printk
> >> variants eventually go through. Change these to store the message in
> >> the new printk ring buffer that the printk kthread is reading.
> >
> > Please, are there any candidates or plans to reuse the new ring
> > buffer implementation?
>
> As you pointed out below, this patch already uses the ring buffer
> implementation for a totally different purpose: NMI safe dynamic memory
> allocation.
I have found an alternative solution. We could calculate the length
of the formatted string without any buffer:
va_list args_copy;
va_copy(args_copy, args);
len = vscprintf(NULL, fmt, args_copy);
va_end(args_copy);
This vsprintf() mode was implemented for exactly this purpose.
Best Regards,
Petr
^ permalink raw reply
* [PATCH] serial: 8250_of: assume reg-shift of 2 for mrvl,mmp-uart
From: Lubomir Rintel @ 2019-02-24 12:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Lubomir Rintel, stable
There are two other drivers that bind to mrvl,mmp-uart and both of them
assume register shift of 2 bits. There are device trees that lack the
property and rely on that assumption.
If this driver wins the race to bind to those devices, it should behave
the same as the older deprecated driver.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
Cc: stable@vger.kernel.org
---
drivers/tty/serial/8250/8250_of.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c
index a1a85805d010..2488de1c4bc4 100644
--- a/drivers/tty/serial/8250/8250_of.c
+++ b/drivers/tty/serial/8250/8250_of.c
@@ -130,6 +130,10 @@ static int of_platform_serial_setup(struct platform_device *ofdev,
port->flags |= UPF_IOREMAP;
}
+ /* Compatibility with the deprecated pxa driver and 8250_pxa drivers. */
+ if (of_device_is_compatible(np, "mrvl,mmp-uart"))
+ port->regshift = 2;
+
/* Check for registers offset within the devices address range */
if (of_property_read_u32(np, "reg-shift", &prop) == 0)
port->regshift = prop;
--
2.20.1
^ permalink raw reply related
* [PATCH] serial: 8250_pxa: honor the port number from devicetree
From: Lubomir Rintel @ 2019-02-24 11:58 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Jiri Slaby, linux-serial, linux-kernel, Lubomir Rintel
Like the other OF-enabled drivers, use the port number from the firmware if
the devicetree specifies an alias:
aliases {
...
serial2 = &uart2; /* Should be ttyS2 */
}
This is how the deprecated pxa.c driver behaved, switching to 8250_pxa
messes up the numbering.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/tty/serial/8250/8250_pxa.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/tty/serial/8250/8250_pxa.c b/drivers/tty/serial/8250/8250_pxa.c
index b9bcbe20a2be..c47188860e32 100644
--- a/drivers/tty/serial/8250/8250_pxa.c
+++ b/drivers/tty/serial/8250/8250_pxa.c
@@ -113,6 +113,10 @@ static int serial_pxa_probe(struct platform_device *pdev)
if (ret)
return ret;
+ ret = of_alias_get_id(pdev->dev.of_node, "serial");
+ if (ret >= 0)
+ uart.port.line = ret;
+
uart.port.type = PORT_XSCALE;
uart.port.iotype = UPIO_MEM32;
uart.port.mapbase = mmres->start;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/6] dt-bindings: soc: qcom: Add interconnect binding for GENI QUP
From: Rob Herring @ 2019-02-23 0:26 UTC (permalink / raw)
To: Georgi Djakov
Cc: Bjorn Andersson, Alok Chauhan, linux-arm-msm, devicetree,
linux-kernel, linux-i2c, linux-spi, linux-serial, Andy Gross,
David Brown, Mark Rutland, dianders, swboyd
In-Reply-To: <5ca1ce61-c458-5550-f89d-ab361f1ec456@linaro.org>
On Wed, Jan 23, 2019 at 08:41:20PM +0200, Georgi Djakov wrote:
> Hi,
>
> On 1/23/19 19:07, Bjorn Andersson wrote:
> > On Mon 21 Jan 22:33 PST 2019, Alok Chauhan wrote:
> >
> >> Add documentation for the interconnect and interconnect-names bindings
> >> for the GENI QUP as detailed by bindings/interconnect/interconnect.txt.
> >>
> >> Signed-off-by: Alok Chauhan <alokc@codeaurora.org>
> >> ---
> >> Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt | 10 ++++++++++
> >> 1 file changed, 10 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >> index dab7ca9..44d7e02 100644
> >> --- a/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.txt
> >> @@ -17,6 +17,12 @@ Required properties if child node exists:
> >> - #address-cells: Must be <1> for Serial Engine Address
> >> - #size-cells: Must be <1> for Serial Engine Address Size
> >> - ranges: Must be present
> >> +- interconnects: phandle to a interconnect provider. Please refer
> >> + ../interconnect/interconnect.txt for details.
> >> + Must be 2 paths corresponding to 2 AXI ports.
> >> +- interconnect-names: Port names to differentiate between the
> >
> > s/Port names/Path names/
> >
> >> + 2 interconnect paths defined with interconnect
> >> + specifier.
> >
> > These two names are significant in that they must match what the driver
> > expects, hence you must actually specify them here.
> >
> > And as the scope of these strings are local to the QUP node you can omit
> > "qup" from them, so make them "memory" and "config" (or perhaps iface,
> > to match the clock naming?).
>
> Actually there was a discussion in the past where we decided include
> both the src and dst endpoint names in this property so that there is
> some symmetry with the "interconnects" property. It would be nice to be
> consistent across different drivers at least for now.
> If we want to denote the master and slave ports here, my two cents would
> be for "qup-mem" and "cpu-qup" or something similar?
Well, there's a proposal from Maxime to add 'dma-memory' or something.
You all need to sort this out.
I assume config or cpu-qup is for register access? Why is this needed?
That should get described thru the DT tree. The interconnect stuff was
supposed to be for the non-cpu centric view (i.e. DMA masters). Maybe
it's fine, but that's not my initial reaction.
Rob
^ permalink raw reply
* Re: [PATCH v2 3/9] serial: core: add rs485-rts-delay-us devicetree property for RS485
From: Rob Herring @ 2019-02-22 23:44 UTC (permalink / raw)
To: Martin Kepplinger
Cc: gregkh, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32, linux-kernel
In-Reply-To: <20190221171758.10322-3-martin.kepplinger@ginzinger.com>
On Thu, Feb 21, 2019 at 06:17:52PM +0100, Martin Kepplinger wrote:
> struct serial_rs485 now optionally holds the rts delay values in
> microseconds. Users can set these delays in their devicetree descriptions,
> so this adds the microseconds-option with the "rs485-rts-delay-us" boolean
> property.
If it has a value, it's not boolean.
Should the old prop be deprecated?
>
> Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
> ---
> Documentation/devicetree/bindings/serial/rs485.txt | 1 +
> drivers/tty/serial/serial_core.c | 11 +++++++++++
> 2 files changed, 12 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/serial/rs485.txt b/Documentation/devicetree/bindings/serial/rs485.txt
> index b92592dff6dd..77396c62b383 100644
> --- a/Documentation/devicetree/bindings/serial/rs485.txt
> +++ b/Documentation/devicetree/bindings/serial/rs485.txt
> @@ -12,6 +12,7 @@ Optional properties:
> * b is the delay between end of data sent and rts signal in milliseconds
> it corresponds to the delay after sending data and actual release of the line.
> If this property is not specified, <0 0> is assumed.
> +- rs485-rts-delay-us: the same as rs485-rts-delay, but in microseconds.
> - rs485-rts-active-low: drive RTS low when sending (default is high).
> - linux,rs485-enabled-at-boot-time: empty property telling to enable the rs485
> feature at boot time. It can be disabled later with proper ioctl.
^ permalink raw reply
* Re: [PATCH] tty/serial/imx.c: Rename URTX0 to UTXD0
From: Uwe Kleine-König @ 2019-02-22 20:55 UTC (permalink / raw)
To: Christina Quast; +Cc: linux-serial, gregkh, jslaby, linux-kernel, kernel
In-Reply-To: <20190222163127.10172-1-cquast@hanoverdisplays.com>
On Fri, Feb 22, 2019 at 04:31:27PM +0000, Christina Quast wrote:
> Rename to be coherent with the datasheet.
>
> Signed-off-by: Christina Quast <cquast@hanoverdisplays.com>
> ---
> drivers/tty/serial/imx.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index dff75dc94731..a405e0d21ec9 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -40,7 +40,7 @@
>
> /* Register definitions */
> #define URXD0 0x0 /* Receiver Register */
> -#define URTX0 0x40 /* Transmitter Register */
> +#define UTXD0 0x40 /* Transmitter Register */
Oh wow, I never noticed this and it was wrong ever since the kernel is
tracked in git (back in 1da177e4c3f4 the definition was in
include/asm-arm/arch-imx/imx-regs.h)---I didn't check further.
Also I checked the i.MX1, i.MX25 and i.MX6 manuals and the transmit
register was never called RTX, in all three manuals it is named UTXD.
I suggest to also drop the 0 from both URXD0 and UTXD0.
If you send a v2 I recommend adding the imx people (from MAINTAINERS) to
Cc.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH v3 7/9] dt-bindings: serial: Add Milbeaut serial driver description
From: Rob Herring @ 2019-02-22 18:38 UTC (permalink / raw)
Cc: linux-serial, devicetree, linux-kernel, linux-arm-kernel, soc,
Greg Kroah-Hartman, Mark Rutland, Takao Orito, Kazuhiro Kasai,
Shinji Kanematsu, Jassi Brar, Masami Hiramatsu, Sugaya Taichi
In-Reply-To: <1550648677-11164-1-git-send-email-sugaya.taichi@socionext.com>
On Wed, 20 Feb 2019 16:44:37 +0900, Sugaya Taichi wrote:
> Add DT bindings document for Milbeaut serial driver.
>
> Signed-off-by: Sugaya Taichi <sugaya.taichi@socionext.com>
> ---
> .../devicetree/bindings/serial/milbeaut-uart.txt | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/serial/milbeaut-uart.txt
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* [PATCH] tty/serial/imx.c: Rename URTX0 to UTXD0
From: Christina Quast @ 2019-02-22 16:31 UTC (permalink / raw)
To: linux-serial; +Cc: gregkh, jslaby, linux-kernel, cquast
Rename to be coherent with the datasheet.
Signed-off-by: Christina Quast <cquast@hanoverdisplays.com>
---
drivers/tty/serial/imx.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index dff75dc94731..a405e0d21ec9 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -40,7 +40,7 @@
/* Register definitions */
#define URXD0 0x0 /* Receiver Register */
-#define URTX0 0x40 /* Transmitter Register */
+#define UTXD0 0x40 /* Transmitter Register */
#define UCR1 0x80 /* Control Register 1 */
#define UCR2 0x84 /* Control Register 2 */
#define UCR3 0x88 /* Control Register 3 */
@@ -502,7 +502,7 @@ static inline void imx_uart_transmit_buffer(struct imx_port *sport)
if (sport->port.x_char) {
/* Send next char */
- imx_uart_writel(sport, sport->port.x_char, URTX0);
+ imx_uart_writel(sport, sport->port.x_char, UTXD0);
sport->port.icount.tx++;
sport->port.x_char = 0;
return;
@@ -536,7 +536,7 @@ static inline void imx_uart_transmit_buffer(struct imx_port *sport)
!(imx_uart_readl(sport, imx_uart_uts_reg(sport)) & UTS_TXFULL)) {
/* send xmit->buf[xmit->tail]
* out the port here */
- imx_uart_writel(sport, xmit->buf[xmit->tail], URTX0);
+ imx_uart_writel(sport, xmit->buf[xmit->tail], UTXD0);
xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
sport->port.icount.tx++;
}
@@ -1257,7 +1257,7 @@ static int imx_uart_dma_init(struct imx_port *sport)
}
slave_config.direction = DMA_MEM_TO_DEV;
- slave_config.dst_addr = sport->port.mapbase + URTX0;
+ slave_config.dst_addr = sport->port.mapbase + UTXD0;
slave_config.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
slave_config.dst_maxburst = TXTL_DMA;
ret = dmaengine_slave_config(sport->dma_chan_tx, &slave_config);
@@ -1812,7 +1812,7 @@ static void imx_uart_poll_put_char(struct uart_port *port, unsigned char c)
} while (~status & USR1_TRDY);
/* write */
- imx_uart_writel(sport, c, URTX0);
+ imx_uart_writel(sport, c, UTXD0);
/* flush */
do {
@@ -1894,7 +1894,7 @@ static void imx_uart_console_putchar(struct uart_port *port, int ch)
while (imx_uart_readl(sport, imx_uart_uts_reg(sport)) & UTS_TXFULL)
barrier();
- imx_uart_writel(sport, ch, URTX0);
+ imx_uart_writel(sport, ch, UTXD0);
}
/*
@@ -2091,7 +2091,7 @@ static void imx_uart_console_early_putchar(struct uart_port *port, int ch)
while (imx_uart_readl(sport, IMX21_UTS) & UTS_TXFULL)
cpu_relax();
- imx_uart_writel(sport, ch, URTX0);
+ imx_uart_writel(sport, ch, UTXD0);
}
static void imx_uart_console_early_write(struct console *con, const char *s,
--
2.20.1
Please consider the environment before printing this email
The information transmitted is intended only for the person or entity to which it is addressed and may contain confidential and/or privileged material. Any review, retransmission, dissemination or other use of, or taking of any action in reliance upon, this information by persons or entities other than the intended recipient is prohibited.
If you received this in error, please contact the sender or postmaster (postmaster@hanoverdisplays.com) and delete the material from any computer.
Although we routinely screen for viruses, addressees should check this e-mail and any attachment for viruses. We make no warranty as to absence of viruses in this e-mail or any attachments.
Our Company's email policy is to permit incidental personal use. If this email is of a personal nature, it must not be relied upon as expressing the views or opinions of the company.
^ permalink raw reply related
* Re: [RFC PATCH v1 10/25] printk: redirect emit/store to new ringbuffer
From: Petr Mladek @ 2019-02-22 15:25 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <87va1byia5.fsf@linutronix.de>
On Fri 2019-02-22 16:06:26, John Ogness wrote:
> On 2019-02-22, Petr Mladek <pmladek@suse.com> wrote:
> >>>> + rbuf = prb_reserve(&h, &sprint_rb, PRINTK_SPRINT_MAX);
> >>>
> >>> The second ring buffer for temporary buffers is really interesting
> >>> idea.
> >>>
> >>> Well, it brings some questions. For example, how many users might
> >>> need a reservation in parallel. Or if the nested use might cause
> >>> some problems when we decide to use printk-specific ring buffer
> >>> implementation. I still have to think about it.
> >>
> >> Keep in mind that it is only used by the writers, which have the
> >> prb_cpulock. Typically there would only be 2 max users: a non-NMI
> >> writer that was interrupted during the reserve/commit window and the
> >> interrupting NMI that does printk. The only exception would be if the
> >> printk-code code itself triggers a BUG_ON or WARN_ON within the
> >> reserve/commit window. Then you will have an additional user per
> >> recursion level.
> >
> > I am not sure it is worth to call the ring buffer machinery just
> > to handle 2-3 buffers.
>
> It may be slightly overkill, but:
>
> 1. We have the prb_cpulock at this point anyway, so it will be
> fast. (Both ring buffers share the same prb_cpulock.)
I am still not persuaded that we really need the lock. The
implementation looks almost ready for a fully lockless
writers. But I might be wrong.
The lock might be fine when it makes the code easier and does
not bring any deadlocks.
> 2. Getting a safe buffer is just 1 line of code: prb_reserve()
The problem is how complicated code is hidden behind
this 1 line of code.
> 3. Why should we waste _any_ lines of code implementing the handling of
> these special 3-4 buffers?
It might be worth if it makes the code more strighforward
and less prone to bugs.
> > Well, it might be just my mental block. We need to be really careful
> > to avoid infinite recursion when storing messages into the log
> > buffer.
>
> The recursion works well. I inserted a triggerable BUG_ON() in
> vprintk_emit() _within_ the reserve/commit window and I see a clean
> backtrace on the emergency console.
Have you tested all possible error situations that might happen?
Testing helps a lot. But the real life often brings surprises.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 11/25] printk_safe: remove printk safe code
From: Petr Mladek @ 2019-02-22 15:15 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <87h8cwymcr.fsf@linutronix.de>
On Fri 2019-02-22 14:38:28, John Ogness wrote:
> On 2019-02-22, Petr Mladek <pmladek@suse.com> wrote:
> >> diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
> >> index 15ca78e1c7d4..77bf84987cda 100644
> >> --- a/lib/nmi_backtrace.c
> >> +++ b/lib/nmi_backtrace.c
> >> @@ -75,12 +75,6 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
> >> touch_softlockup_watchdog();
> >> }
> >>
> >> - /*
> >> - * Force flush any remote buffers that might be stuck in IRQ context
> >> - * and therefore could not run their irq_work.
> >> - */
> >> - printk_safe_flush();
> >> -
> >> clear_bit_unlock(0, &backtrace_flag);
> >> put_cpu();
> >> }
> >
> > This reminds me that we need to add back the locking that was
> > removed in the commit 03fc7f9c99c1e7ae2925d45 ("printk/nmi:
> > Prevent deadlock when accessing the main log buffer in NMI").
>
> No, that commit is needed. You cannot have NMIs waiting on other CPUs.
It sounds weird. But it is safe to use a lock when it is used only
in the NMI context.
The lock has always been there. For example, I found it in
the commit 1fb9d6ad2766a1dd70 ("nmi_watchdog: Add new,
generic implementation, using perf events") from v2.6.36.
It could get removed only because we switched to the per-CPU
buffers.
> > Otherwise, backtraces from different CPUs would get mixed.
>
> A later patch (#17) adds CPU IDs to the printk messages so that this
> isn't a problem. (That patch is actually obsolete now because Sergey has
> already merged work for linux-next that includes this information.)
No this is not enough. First, the CPU-ids were primary added for
kernel testers (0-day robot, kernel-ci, syzcaller). It is not
enabled by default. It is not handled by kmsg interface.
Also sorting the messages is not much user friendly.
It should be the last resort when no other solution
is possible.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 10/25] printk: redirect emit/store to new ringbuffer
From: John Ogness @ 2019-02-22 15:06 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190222144302.44zl37p75qgaixf3@pathway.suse.cz>
On 2019-02-22, Petr Mladek <pmladek@suse.com> wrote:
>>>> + rbuf = prb_reserve(&h, &sprint_rb, PRINTK_SPRINT_MAX);
>>>
>>> The second ring buffer for temporary buffers is really interesting
>>> idea.
>>>
>>> Well, it brings some questions. For example, how many users might
>>> need a reservation in parallel. Or if the nested use might cause
>>> some problems when we decide to use printk-specific ring buffer
>>> implementation. I still have to think about it.
>>
>> Keep in mind that it is only used by the writers, which have the
>> prb_cpulock. Typically there would only be 2 max users: a non-NMI
>> writer that was interrupted during the reserve/commit window and the
>> interrupting NMI that does printk. The only exception would be if the
>> printk-code code itself triggers a BUG_ON or WARN_ON within the
>> reserve/commit window. Then you will have an additional user per
>> recursion level.
>
> I am not sure it is worth to call the ring buffer machinery just
> to handle 2-3 buffers.
It may be slightly overkill, but:
1. We have the prb_cpulock at this point anyway, so it will be
fast. (Both ring buffers share the same prb_cpulock.)
2. Getting a safe buffer is just 1 line of code: prb_reserve()
3. Why should we waste _any_ lines of code implementing the handling of
these special 3-4 buffers?
> Well, it might be just my mental block. We need to be really careful
> to avoid infinite recursion when storing messages into the log
> buffer.
The recursion works well. I inserted a triggerable BUG_ON() in
vprintk_emit() _within_ the reserve/commit window and I see a clean
backtrace on the emergency console.
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 10/25] printk: redirect emit/store to new ringbuffer
From: Petr Mladek @ 2019-02-22 14:43 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <8736oijgpf.fsf@linutronix.de>
On Wed 2019-02-20 22:25:00, John Ogness wrote:
> On 2019-02-20, Petr Mladek <pmladek@suse.com> wrote:
> >> vprintk_emit and vprintk_store are the main functions that all printk
> >> variants eventually go through. Change these to store the message in
> >> the new printk ring buffer that the printk kthread is reading.
> >
> > We need to switch the two buffers in a single commit
> > without disabling important functionality.
> >
> > By other words, we need to change vprintk_emit(), vprintk_store(),
> > console_unlock(), syslog(), devkmsg(), and syslog in one patch.
>
> Agreed. But for the review process I expect it makes things much easier
> to change them one at a time. Patch-squashing is not a problem once all
> the individuals have been ack'd.
Good question. I would personally prefer to keep it in a single patch
even for review.
It would help me to see what the different readers have in common
and what can get optimized. Anyway, we should prepare the patch
a way so that it can get understand also by git history readers.
> >> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> >> index 5a5a685bb128..b6a6f1002741 100644
> >> --- a/kernel/printk/printk.c
> >> +++ b/kernel/printk/printk.c
> >> @@ -584,54 +500,36 @@ static int log_store(int facility, int level,
> >> const char *text, u16 text_len)
> >
> >> memcpy(log_dict(msg), dict, dict_len);
> >> msg->dict_len = dict_len;
> >> msg->facility = facility;
> >> msg->level = level & 7;
> >> msg->flags = flags & 0x1f;
> >
> > The existing struct printk_log is stored into the data field
> > of struct prb_entry. It is because printk_ring_buffer is supposed
> > to be a generic ring buffer.
>
> Yes.
>
> > It makes the code more complicated. Also it needs more space for
> > the size and seq items from struct prb_entry.
> >
> > printk() is already very complicated code. We should not make
> > it unnecessarily worse.
>
> In my opinion it makes things considerably easier. My experience with
> printk-code is that it is so complicated because it is mixing
> printk-features with ring buffer handling code. By providing a strict
> API (and hiding the details) of the ring buffer, the implementation of
> the printk-features became pretty straight forward.
It sounds reasonable. Well, the separation is not completely clear.
We have tree layers:
+ struct prb_entry: ring buffer metadata
+ struct printk_log: message metadata
+ text, dict: message strings
Where sequence number is part of prb_entry but it is too important
part of the printk logic.
Also it does not make sense to read text and dict when we just
calculate the space taken by the messages.
That said, I agree that printk code is complicated and we should
do better. This patchset goes in the right direction.
I personally hate the following things:
+ There are too many global values. Many of them are related,
e.g. first_idx, next_idx, first_seq, next_seq. They should
be in some structures.
+ Too many variables are passed by parameters. They should be
in some structures as well.
+ The name of struct printk_log is really confusing. It
should have been printk_msg or so.
+ Continuous lines buffer makes the buffer-related code much
more complicated.
+ Especially the console-related code is full of hacks.
printk code was not really maintained most of the history.
Random people just fixed/extended the code for their needs.
> > Please, are there any candidates or plans to reuse the new ring
> > buffer implementation?
>
> As you pointed out below, this patch already uses the ring buffer
> implementation for a totally different purpose: NMI safe dynamic memory
> allocation.
I am not sure that this 2nd usage is worth it, see below.
> >> int vprintk_store(int facility, int level,
> >> const char *dict, size_t dictlen,
> >> const char *fmt, va_list args)
> >> {
> >> - static char textbuf[LOG_LINE_MAX];
> >> - char *text = textbuf;
> >> - size_t text_len;
> >> + return vprintk_emit(facility, level, dict, dictlen, fmt, args);
> >> +}
> >> +
> >> +/* ring buffer used as memory allocator for temporary sprint buffers */
> >> +DECLARE_STATIC_PRINTKRB(sprint_rb,
> >> + ilog2(PRINTK_RECORD_MAX + sizeof(struct prb_entry) +
> >> + sizeof(long)) + 2, &printk_cpulock);
> >> +
> >> +asmlinkage int vprintk_emit(int facility, int level,
> >> + const char *dict, size_t dictlen,
> >> + const char *fmt, va_list args)
> >
> > [...]
> >
> >> + rbuf = prb_reserve(&h, &sprint_rb, PRINTK_SPRINT_MAX);
> >
> > The second ring buffer for temporary buffers is really interesting
> > idea.
> >
> > Well, it brings some questions. For example, how many users might
> > need a reservation in parallel. Or if the nested use might cause
> > some problems when we decide to use printk-specific ring buffer
> > implementation. I still have to think about it.
>
> Keep in mind that it is only used by the writers, which have the
> prb_cpulock. Typically there would only be 2 max users: a non-NMI writer
> that was interrupted during the reserve/commit window and the
> interrupting NMI that does printk. The only exception would be if the
> printk-code code itself triggers a BUG_ON or WARN_ON within the
> reserve/commit window. Then you will have an additional user per
> recursion level.
I am not sure it is worth to call the ring buffer machinery just
to handle 2-3 buffers.
Well, it might be just my mental block. We need to be really
careful to avoid infinite recursion when storing messages
into the log buffer. The nested reserve/commit calls provoke
my brain to spin around. It is possible that I would love
this idea once my brain stops spinning ;-)
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 11/25] printk_safe: remove printk safe code
From: John Ogness @ 2019-02-22 13:38 UTC (permalink / raw)
To: Petr Mladek
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190222103732.zkcvjijtdcfu4vbt@pathway.suse.cz>
On 2019-02-22, Petr Mladek <pmladek@suse.com> wrote:
>> diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
>> index 15ca78e1c7d4..77bf84987cda 100644
>> --- a/lib/nmi_backtrace.c
>> +++ b/lib/nmi_backtrace.c
>> @@ -75,12 +75,6 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
>> touch_softlockup_watchdog();
>> }
>>
>> - /*
>> - * Force flush any remote buffers that might be stuck in IRQ context
>> - * and therefore could not run their irq_work.
>> - */
>> - printk_safe_flush();
>> -
>> clear_bit_unlock(0, &backtrace_flag);
>> put_cpu();
>> }
>
> This reminds me that we need to add back the locking that was
> removed in the commit 03fc7f9c99c1e7ae2925d45 ("printk/nmi:
> Prevent deadlock when accessing the main log buffer in NMI").
No, that commit is needed. You cannot have NMIs waiting on other CPUs.
> Otherwise, backtraces from different CPUs would get mixed.
A later patch (#17) adds CPU IDs to the printk messages so that this
isn't a problem. (That patch is actually obsolete now because Sergey has
already merged work for linux-next that includes this information.)
John Ogness
^ permalink raw reply
* Re: [RFC PATCH v1 11/25] printk_safe: remove printk safe code
From: Petr Mladek @ 2019-02-22 10:37 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20190212143003.48446-12-john.ogness@linutronix.de>
On Tue 2019-02-12 15:29:49, John Ogness wrote:
> vprintk variants are now NMI-safe so there is no longer a need for
> the "safe" calls.
>
> NOTE: This also removes printk flushing functionality.
>
> Signed-off-by: John Ogness <john.ogness@linutronix.de>
> ---
> include/linux/hardirq.h | 2 -
> include/linux/printk.h | 27 ---
> init/main.c | 1 -
> kernel/kexec_core.c | 1 -
> kernel/panic.c | 3 -
> kernel/printk/Makefile | 1 -
> kernel/printk/internal.h | 30 +---
> kernel/printk/printk.c | 13 +-
> kernel/printk/printk_safe.c | 427 --------------------------------------------
> kernel/trace/trace.c | 2 -
> lib/nmi_backtrace.c | 6 -
> 11 files changed, 7 insertions(+), 506 deletions(-)
> delete mode 100644 kernel/printk/printk_safe.c
>From my POV, this is the primary selling argument for the new
ring buffer.
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index b6a6f1002741..073ff9fd6872 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -1752,6 +1745,11 @@ asmlinkage int vprintk_emit(int facility, int level,
> }
> EXPORT_SYMBOL(vprintk_emit);
>
> +__printf(1, 0) int vprintk_func(const char *fmt, va_list args)
> +{
> + return vprintk_emit(0, LOGLEVEL_DEFAULT, NULL, 0, fmt, args);
> +}
All vprintk_func() calls should get replaced with vprintk_default().
It includes a crazy hack to reuse some kernel code (that calls
printk() in kdb code.
> asmlinkage int vprintk(const char *fmt, va_list args)
> {
> return vprintk_func(fmt, args);
> diff --git a/lib/nmi_backtrace.c b/lib/nmi_backtrace.c
> index 15ca78e1c7d4..77bf84987cda 100644
> --- a/lib/nmi_backtrace.c
> +++ b/lib/nmi_backtrace.c
> @@ -75,12 +75,6 @@ void nmi_trigger_cpumask_backtrace(const cpumask_t *mask,
> touch_softlockup_watchdog();
> }
>
> - /*
> - * Force flush any remote buffers that might be stuck in IRQ context
> - * and therefore could not run their irq_work.
> - */
> - printk_safe_flush();
> -
> clear_bit_unlock(0, &backtrace_flag);
> put_cpu();
> }
This reminds me that we need to add back the locking that was
removed in the commit 03fc7f9c99c1e7ae2925d45 ("printk/nmi:
Prevent deadlock when accessing the main log buffer in NMI").
Otherwise, backtraces from different CPUs would get mixed.
We need to add this before redirecting printk() to
the new ring buffer.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC PATCH v1 07/25] printk-rb: add functionality required by printk
From: Petr Mladek @ 2019-02-22 9:58 UTC (permalink / raw)
To: John Ogness
Cc: linux-kernel, Peter Zijlstra, Sergey Senozhatsky, Steven Rostedt,
Daniel Wang, Andrew Morton, Linus Torvalds, Greg Kroah-Hartman,
Alan Cox, Jiri Slaby, Peter Feiner, linux-serial,
Sergey Senozhatsky
In-Reply-To: <87ftsjv3cb.fsf@linutronix.de>
On Tue 2019-02-19 23:08:20, John Ogness wrote:
> On 2019-02-18, Petr Mladek <pmladek@suse.com> wrote:
> >> The printk subsystem needs to be able to query the size of the ring
> >> buffer, seek to specific entries within the ring buffer, and track
> >> if records could not be stored in the ring buffer.
> >>
> >> diff --git a/lib/printk_ringbuffer.c b/lib/printk_ringbuffer.c
> >> index c2ddf4cb9f92..ce33b5add5a1 100644
> >> --- a/lib/printk_ringbuffer.c
> >> +++ b/lib/printk_ringbuffer.c
> >> @@ -175,11 +175,16 @@ void prb_commit(struct prb_handle *h)
> >> head = PRB_WRAP_LPOS(rb, head, 1);
> >> continue;
> >> }
> >> + while (atomic_long_read(&rb->lost)) {
> >> + atomic_long_dec(&rb->lost);
> >> + rb->seq++;
>
> > On the contrary, the patch adding support for lost messages
> > should implement a way how to inform the user about lost messages.
> > E.g. to add a warning when some space becomes available again.
>
> The readers will see that messages were lost. I think that is enough. I
> don't know how useful it would be to notify writers that space is
> available. The writers are holding the prb_cpulock, so they definitely
> shouldn't be waiting around for anything.
I see your intention. Well, it forces all readers to implement the
check and write a message. It might be fine if the code can be shared.
My original idea was the following. If any next writer succeeds
to reserve space for its own message. It would try to reserve
space also for the warning. If it succeeds, it would just
write the warning there (like a nested context or so). Then
all readers would get the warning for free.
But you inspired me to antoher idea. We could hadle this in
the krb_iter_get_next_entry() calls. They could fill
the given buffer with the warning message when they
detect missed messages. The real message might get
added into the same buffer. Or we might add a flag
into struct prb_iter so that the reader would need
to call krb_iter_get_next_entry() to get the real
message on the current lpos.
Both solutions allow to get the warnings trasparently.
There will be no duplicated extra code. Also all readers
would handle it consistently.
But there is a difference:
If we store the warning into the ring buffer directly
then we do not need to store the seq number. I mean
that we would not need to bump seq when reservation failed.
The amount of lost messages is handled by another
counter anyway.
On the other hand, using the fake messages would allow
to handle transparently even the messages lost by readers.
I mean that krb_iter_get_next_valid_entry() might fill
the given buffer with a warning when the next message
was overwriten and it had to reset lpos to tail.
I am not sure what is better out of my head. I would need
to play with the code.
> This situation should be quite rare because it means the _entire_ ring
> buffer was filled up by an NMI context that interrupted a context that
> was in the reserve/commit window. NMI contexts probably should not be
> doing _so_ much printk'ing within a single NMI.
Sure.
Best Regards,
Petr
^ permalink raw reply
* [PATCH v2 9/9] serial: st32-usart: set SER_RS485_DELAY_IN_USEC accordingly
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 673 bytes --]
Unset SER_RS485_DELAY_IN_USEC for userspace to get correct settings.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/tty/serial/stm32-usart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/stm32-usart.c b/drivers/tty/serial/stm32-usart.c
index e8d7a7bb4339..4daf5fc71644 100644
--- a/drivers/tty/serial/stm32-usart.c
+++ b/drivers/tty/serial/stm32-usart.c
@@ -112,6 +112,7 @@ static int stm32_config_rs485(struct uart_port *port,
port->rs485 = *rs485conf;
+ rs485conf->flags &= ~SER_RS485_DELAY_IN_USEC;
rs485conf->flags |= SER_RS485_RX_DURING_TX;
if (rs485conf->flags & SER_RS485_ENABLED) {
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
* [PATCH v2 8/9] serial: fsl_lpuart: set SER_RS485_DELAY_IN_USEC accordingly
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 736 bytes --]
Clear SER_RS485_DELAY_IN_USEC for userspace to get correct settings.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/tty/serial/fsl_lpuart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index ea1c85e3b432..a63aa22e3a25 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -1124,6 +1124,7 @@ static int lpuart_config_rs485(struct uart_port *port,
rs485->delay_rts_before_send = 0;
rs485->delay_rts_after_send = 0;
rs485->flags &= ~SER_RS485_RX_DURING_TX;
+ rs485->flags &= ~SER_RS485_DELAY_IN_USEC;
if (rs485->flags & SER_RS485_ENABLED) {
/* Enable auto RS-485 RTS mode */
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
* [PATCH v2 7/9] serial: atmel_serial: set SER_RS485_DELAY_IN_USEC accordingly
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 800 bytes --]
Unset the SER_RS485_DELAY_IN_USEC flag during rs485 config for
userspace to get the correct setting.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/tty/serial/atmel_serial.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 05147fe24343..36ab1c131d36 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -346,6 +346,9 @@ static int atmel_config_rs485(struct uart_port *port,
port->rs485 = *rs485conf;
+ /* delays are in milliseconds */
+ rs485conf->flags &= ~SER_RS485_DELAY_IN_USEC;
+
if (rs485conf->flags & SER_RS485_ENABLED) {
dev_dbg(port->dev, "Setting UART to RS485\n");
atmel_port->tx_done_mask = ATMEL_US_TXEMPTY;
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
* [PATCH v2 6/9] serial: sc16is7xx: add support for rs485 RTS delays in microseconds
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]
Read struct serial_rs485's flag SER_RS485_DELAY_IN_USEC and apply the delay
accordingly.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/tty/serial/sc16is7xx.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 635178cf3eed..b0e00b9fb177 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -743,7 +743,13 @@ static void sc16is7xx_tx_proc(struct kthread_work *ws)
struct uart_port *port = &(to_sc16is7xx_one(ws, tx_work)->port);
if ((port->rs485.flags & SER_RS485_ENABLED) &&
- (port->rs485.delay_rts_before_send > 0))
+ (port->rs485.delay_rts_before_send > 0) &&
+ (port->rs485.flags & SER_RS485_DELAY_IN_USEC))
+ usleep_range(port->rs485.delay_rts_before_send,
+ port->rs485.delay_rts_before_send);
+ else if ((port->rs485.flags & SER_RS485_ENABLED) &&
+ (port->rs485.delay_rts_before_send > 0) &&
+ !(port->rs485.flags & SER_RS485_DELAY_IN_USEC))
msleep(port->rs485.delay_rts_before_send);
sc16is7xx_handle_tx(port);
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
* [PATCH v2 5/9] serial: omap-serial: add support for rs485 RTS delays in microseconds
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 2402 bytes --]
Read struct serial_rs485's flag SER_RS485_DELAY_IN_USEC and apply the delay
accordingly.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/tty/serial/omap-serial.c | 25 +++++++++++++++++++++----
1 file changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 6420ae581a80..adcd75ce5112 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -310,7 +310,11 @@ static void serial_omap_stop_tx(struct uart_port *port)
res = (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) ?
1 : 0;
if (gpio_get_value(up->rts_gpio) != res) {
- if (port->rs485.delay_rts_after_send > 0)
+ if (port->rs485.delay_rts_after_send > 0 &&
+ port->rs485.flags & SER_RS485_DELAY_IN_USEC)
+ udelay(
+ port->rs485.delay_rts_after_send);
+ else if (port->rs485.delay_rts_after_send > 0)
mdelay(
port->rs485.delay_rts_after_send);
gpio_set_value(up->rts_gpio, res);
@@ -420,7 +424,11 @@ static void serial_omap_start_tx(struct uart_port *port)
res = (port->rs485.flags & SER_RS485_RTS_ON_SEND) ? 1 : 0;
if (gpio_get_value(up->rts_gpio) != res) {
gpio_set_value(up->rts_gpio, res);
- if (port->rs485.delay_rts_before_send > 0)
+ if (port->rs485.delay_rts_before_send > 0 &&
+ port->rs485.flags & SER_RS485_DELAY_IN_USEC)
+ udelay(port->rs485.delay_rts_before_send);
+ else if (port->rs485.delay_rts_before_send > 0 &&
+ !(port->rs485.flags & SER_RS485_DELAY_IN_USEC)
mdelay(port->rs485.delay_rts_before_send);
}
}
@@ -1407,8 +1415,17 @@ serial_omap_config_rs485(struct uart_port *port, struct serial_rs485 *rs485)
serial_out(up, UART_IER, 0);
/* Clamp the delays to [0, 100ms] */
- rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 100U);
- rs485->delay_rts_after_send = min(rs485->delay_rts_after_send, 100U);
+ if (port->rs485.flags & SER_RS485_DELAY_IN_USEC) {
+ rs485->delay_rts_before_send = min(rs485->delay_rts_before_send,
+ 100000U);
+ rs485->delay_rts_after_send = min(rs485->delay_rts_after_send,
+ 100000U);
+ } else {
+ rs485->delay_rts_before_send = min(rs485->delay_rts_before_send,
+ 100);
+ rs485->delay_rts_after_send = min(rs485->delay_rts_after_send,
+ 100U);
+ }
/* store new config */
port->rs485 = *rs485;
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
* [PATCH v2 4/9] serial: 8250: add support for rs485 RTS delays in microseconds
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]
Read struct serial_rs485's flag SER_RS485_DELAY_IN_USEC and apply the delay
accordingly.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
drivers/tty/serial/8250/8250_omap.c | 13 +++++++++++--
drivers/tty/serial/8250/8250_port.c | 25 +++++++++++++++++++++----
2 files changed, 32 insertions(+), 6 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 0a8316632d75..cbce43ac60b1 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -720,8 +720,17 @@ static int omap_8250_rs485_config(struct uart_port *port,
struct uart_8250_port *up = up_to_u8250p(port);
/* Clamp the delays to [0, 100ms] */
- rs485->delay_rts_before_send = min(rs485->delay_rts_before_send, 100U);
- rs485->delay_rts_after_send = min(rs485->delay_rts_after_send, 100U);
+ if (rs485->flags & SER_RS485_DELAY_IN_USEC) {
+ rs485->delay_rts_before_send = min(rs485->delay_rts_before_send,
+ 100000U);
+ rs485->delay_rts_after_send = min(rs485->delay_rts_after_send,
+ 100000U);
+ } else {
+ rs485->delay_rts_before_send = min(rs485->delay_rts_before_send,
+ 100U);
+ rs485->delay_rts_after_send = min(rs485->delay_rts_after_send,
+ 100U);
+ }
port->rs485 = *rs485;
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index d2f3310abe54..0cee4aa8323d 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -1483,6 +1483,15 @@ static void start_hrtimer_ms(struct hrtimer *hrt, unsigned long msec)
hrtimer_start(hrt, t, HRTIMER_MODE_REL);
}
+static void start_hrtimer_us(struct hrtimer *hrt, unsigned long usec)
+{
+ long sec = usec / 1000000;
+ long nsec = (usec % 1000000) * 1000000000;
+ ktime_t t = ktime_set(sec, nsec);
+
+ hrtimer_start(hrt, t, HRTIMER_MODE_REL);
+}
+
static void __stop_tx_rs485(struct uart_8250_port *p)
{
struct uart_8250_em485 *em485 = p->em485;
@@ -1493,8 +1502,12 @@ static void __stop_tx_rs485(struct uart_8250_port *p)
*/
if (p->port.rs485.delay_rts_after_send > 0) {
em485->active_timer = &em485->stop_tx_timer;
- start_hrtimer_ms(&em485->stop_tx_timer,
- p->port.rs485.delay_rts_after_send);
+ if (p->port.rs485.flags & SER_RS485_DELAY_IN_USEC)
+ start_hrtimer_us(&em485->stop_tx_timer,
+ p->port.rs485.delay_rts_after_send);
+ else
+ start_hrtimer_ms(&em485->stop_tx_timer,
+ p->port.rs485.delay_rts_after_send);
} else {
__do_stop_tx_rs485(p);
}
@@ -1600,8 +1613,12 @@ static inline void start_tx_rs485(struct uart_port *port)
if (up->port.rs485.delay_rts_before_send > 0) {
em485->active_timer = &em485->start_tx_timer;
- start_hrtimer_ms(&em485->start_tx_timer,
- up->port.rs485.delay_rts_before_send);
+ if (up->port.rs485.flags & SER_RS485_DELAY_IN_USEC)
+ start_hrtimer_us(&em485->start_tx_timer,
+ up->port.rs485.delay_rts_before_send);
+ else
+ start_hrtimer_ms(&em485->start_tx_timer,
+ up->port.rs485.delay_rts_before_send);
return;
}
}
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
* [PATCH v2 3/9] serial: core: add rs485-rts-delay-us devicetree property for RS485
From: Martin Kepplinger @ 2019-02-21 17:17 UTC (permalink / raw)
To: gregkh, robh+dt, mark.rutland, jslaby, corbet, richard.genoud,
nicolas.ferre, alexandre.belloni, ludovic.desroches,
mcoquelin.stm32, alexandre.torgue, linux-serial, devicetree,
linux-arm-kernel, linux-stm32
Cc: linux-kernel, Martin Kepplinger
In-Reply-To: <20190221171758.10322-1-martin.kepplinger@ginzinger.com>
[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]
struct serial_rs485 now optionally holds the rts delay values in
microseconds. Users can set these delays in their devicetree descriptions,
so this adds the microseconds-option with the "rs485-rts-delay-us" boolean
property.
Signed-off-by: Martin Kepplinger <martin.kepplinger@ginzinger.com>
---
Documentation/devicetree/bindings/serial/rs485.txt | 1 +
drivers/tty/serial/serial_core.c | 11 +++++++++++
2 files changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/rs485.txt b/Documentation/devicetree/bindings/serial/rs485.txt
index b92592dff6dd..77396c62b383 100644
--- a/Documentation/devicetree/bindings/serial/rs485.txt
+++ b/Documentation/devicetree/bindings/serial/rs485.txt
@@ -12,6 +12,7 @@ Optional properties:
* b is the delay between end of data sent and rts signal in milliseconds
it corresponds to the delay after sending data and actual release of the line.
If this property is not specified, <0 0> is assumed.
+- rs485-rts-delay-us: the same as rs485-rts-delay, but in microseconds.
- rs485-rts-active-low: drive RTS low when sending (default is high).
- linux,rs485-enabled-at-boot-time: empty property telling to enable the rs485
feature at boot time. It can be disabled later with proper ioctl.
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 556f50aa1b58..4fb265b2c0fe 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3097,6 +3097,17 @@ void uart_get_rs485_mode(struct device *dev, struct serial_rs485 *rs485conf)
rs485conf->delay_rts_after_send = 0;
}
+ ret = device_property_read_u32_array(dev, "rs485-rts-delay-us",
+ rs485_delay, 2);
+ if (!ret) {
+ rs485conf->delay_rts_before_send = rs485_delay[0];
+ rs485conf->delay_rts_after_send = rs485_delay[1];
+ rs485conf->flags |= SER_RS485_DELAY_IN_USEC;
+ } else {
+ rs485conf->delay_rts_before_send = 0;
+ rs485conf->delay_rts_after_send = 0;
+ }
+
/*
* Clear full-duplex and enabled flags, set RTS polarity to active high
* to get to a defined state with the following properties:
--
2.20.1
[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3616 bytes --]
^ permalink raw reply related
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