Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v3 0/3] serial: xuartps: hardware race condition and cleanup
From: Helmut Grohne @ 2018-06-04 10:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Simek, linux-serial
  Cc: linux-arm-kernel, Uwe Kleine-König

The character transmission in xuartps is racy. If the transmitter is disabled
early, the device is confused and produces a desync. Garbage on the remote end
can be a visible symptom. The second patch in this series tries to reduce that
race condition in accordance with the hardware documentation, but it cannot
remove it entirely. The first and third patches are code cleanup.

Changes since v2:
 * Do not attempt to disable the transmitter after a transmission (original
   behaviour around 3.14). These patches no longer touch with the transmitter
   state at all as requested by Sören Brinkmann.
 * Add Acked-by/Reviewed-by tags from Sören Brinkmann after addressing his
   remarks.

Earlier patches/discussion at:
    https://www.spinics.net/lists/linux-serial/msg23145.html
    https://www.spinics.net/lists/linux-serial/msg23156.html
    https://www.spinics.net/lists/linux-serial/msg23157.html

Helmut Grohne (3):
  serial: xuartps: fix typo in cdns_uart_startup
  serial: xuartps: reduce hardware TX race condition
  serial: xuartps: remove unnecessary register write

 drivers/tty/serial/xilinx_uartps.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

-- 
2.11.0

^ permalink raw reply

* [PATCH v3 1/3] serial: xuartps: fix typo in cdns_uart_startup
From: Helmut Grohne @ 2018-06-04 10:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Simek, linux-serial
  Cc: linux-arm-kernel, Uwe Kleine-König
In-Reply-To: <cover.1528106392.git.h.grohne@intenta.de>

The bit mask changes in commit 6e14f7c1f2c2 ("tty: xuartps: Improve
startup function") doesn't do what the commit message advertises. The
original behaviour was clearing the RX_DIS bit, but due to missing ~,
that bit is now the only bit kept.

Currently, the regression is harmless, because the previous write to the
control register sets it to TXRST | RXRST. Thus the RX_DIS bit is
previously cleared. The *RST bits are cleared by the hardware, so this
commit does not currently change behaviour, but makes future changes
less risky.

Link: https://www.spinics.net/lists/linux-serial/msg23157.html
Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
Fixes: 6e14f7c1f2c2 ("tty: xuartps: Improve startup function")
Reviewed-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index bd72dd843338..e4b2d8102a04 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -829,7 +829,7 @@ static int cdns_uart_startup(struct uart_port *port)
 	 * the receiver.
 	 */
 	status = readl(port->membase + CDNS_UART_CR);
-	status &= CDNS_UART_CR_RX_DIS;
+	status &= ~CDNS_UART_CR_RX_DIS;
 	status |= CDNS_UART_CR_RX_EN;
 	writel(status, port->membase + CDNS_UART_CR);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 2/3] serial: xuartps: reduce hardware TX race condition
From: Helmut Grohne @ 2018-06-04 10:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Simek, linux-serial
  Cc: linux-arm-kernel, Uwe Kleine-König
In-Reply-To: <cover.1528106392.git.h.grohne@intenta.de>

After sending data to the uart, the driver was waiting until the TX
FIFO was empty (for every single char written). After that, TX was
disabled by writing the original TX state to the status register. At
that time however, the state machine could still be shifting
characters. Not waiting can result in strange hardware states,
especially when coupled with calls to cdns_uart_set_termios, whose
symptom generally is garbage characters being received from uart or a
hang.

According to UG585, the TACTIVE bit of the channel status register
indicates the shifter operation and we should be waiting for that bit
to clear.

Sending characters does not require the TX FIFO to be empty, but merely
to not be full. So cdns_uart_console_putchar is updated accordingly.

During tests with an instrumented kernel and an oscilloscope, we could
determine that the chance of a race is reduced by this patch. It is not
removed entirely. On the oscilloscope, one can see that disabling the
transmitter early can result in the transmission hanging in the middle
of a character for a tiny duration. This hiccup is enough to
desynchronize with a remote device for a sequence of characters until a
data bit doesn't match the start or stop bits anymore.

Link: https://www.spinics.net/lists/linux-serial/msg23156.html
Link: https://www.spinics.net/lists/linux-serial/msg26139.html
Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
Acked-by: Sören Brinkmann <soren.brinkmann@xilinx.com>
---
 drivers/tty/serial/xilinx_uartps.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index e4b2d8102a04..a34b2c757593 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -167,6 +167,7 @@ MODULE_PARM_DESC(rx_timeout, "Rx timeout, 1-255");
 #define CDNS_UART_SR_TXEMPTY	0x00000008 /* TX FIFO empty */
 #define CDNS_UART_SR_TXFULL	0x00000010 /* TX FIFO full */
 #define CDNS_UART_SR_RXTRIG	0x00000001 /* Rx Trigger */
+#define CDNS_UART_SR_TACTIVE	0x00000800 /* TX state machine active */
 
 /* baud dividers min/max values */
 #define CDNS_UART_BDIV_MIN	4
@@ -1138,23 +1139,14 @@ static struct uart_port *cdns_uart_get_port(int id)
 
 #ifdef CONFIG_SERIAL_XILINX_PS_UART_CONSOLE
 /**
- * cdns_uart_console_wait_tx - Wait for the TX to be full
- * @port: Handle to the uart port structure
- */
-static void cdns_uart_console_wait_tx(struct uart_port *port)
-{
-	while (!(readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXEMPTY))
-		barrier();
-}
-
-/**
  * cdns_uart_console_putchar - write the character to the FIFO buffer
  * @port: Handle to the uart port structure
  * @ch: Character to be written
  */
 static void cdns_uart_console_putchar(struct uart_port *port, int ch)
 {
-	cdns_uart_console_wait_tx(port);
+	while (readl(port->membase + CDNS_UART_SR) & CDNS_UART_SR_TXFULL)
+		cpu_relax();
 	writel(ch, port->membase + CDNS_UART_FIFO);
 }
 
@@ -1241,7 +1233,10 @@ static void cdns_uart_console_write(struct console *co, const char *s,
 	writel(ctrl, port->membase + CDNS_UART_CR);
 
 	uart_console_write(port, s, count, cdns_uart_console_putchar);
-	cdns_uart_console_wait_tx(port);
+	while ((readl(port->membase + CDNS_UART_SR) &
+			(CDNS_UART_SR_TXEMPTY | CDNS_UART_SR_TACTIVE)) !=
+			CDNS_UART_SR_TXEMPTY)
+		cpu_relax();
 
 	writel(ctrl, port->membase + CDNS_UART_CR);
 
-- 
2.11.0

^ permalink raw reply related

* [PATCH v3 3/3] serial: xuartps: remove unnecessary register write
From: Helmut Grohne @ 2018-06-04 10:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Michal Simek, linux-serial
  Cc: linux-arm-kernel, Uwe Kleine-König
In-Reply-To: <cover.1528106392.git.h.grohne@intenta.de>

This writel writes the exact same value as the previous writel and is
thus unnecessary. It accidentally became unnecessary in e3538c37ee38
("tty: xuartps: Beautify read-modify writes"), but the new behaviour is
now expected.

Link: https://www.spinics.net/lists/linux-serial/msg23168.html
Signed-off-by: Helmut Grohne <h.grohne@intenta.de>
---
 drivers/tty/serial/xilinx_uartps.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index a34b2c757593..7a2b1a7350ac 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -1238,8 +1238,6 @@ static void cdns_uart_console_write(struct console *co, const char *s,
 			CDNS_UART_SR_TXEMPTY)
 		cpu_relax();
 
-	writel(ctrl, port->membase + CDNS_UART_CR);
-
 	/* restore interrupt state */
 	writel(imr, port->membase + CDNS_UART_IER);
 
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH 0/8] serial: 8250: Add 485 emulation to 8250_dw.
From: Matwey V. Kornilov @ 2018-06-04 10:34 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby, Kees Cook,
	Matthias Brugger, Allen Pais, Sean Young, Ed Blake, Stefan Potyra,
	Philipp Zabel, Joshua Scott, Vignesh R, Rolf Evers-Fischer,
	Aaron Sierra, Rafael Gago, Joel Stanley, Sean Wang, linux-serial,
	linux-kernel
In-Reply-To: <65c26f5db1275e958cfe6390e9336568b0158dea.camel@linux.intel.com>

2018-06-04 13:12 GMT+03:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
>> Need to handle rs485 with 8250_dw port.
>>
>> Use existing em485 emulation layer for 8250 taking care to fix some
>> bug
>> and taking care especially of RTS_AFTER_SEND case.
>>
>
> I don't see in Cc list author of rs485 code, who might be interested in
> this.
>
> So, added him here.
>

Hi, Andy

Nice to see you there. I will look through the code later.
I always thought that DW has its own hardware rs485 support.

>> Giulio Benetti (8):
>>   serial: 8250_dw: add em485 support
>>   serial: 8250_dw: allow enable rs485 at boot time
>>   serial: 8250: Copy em485 from port to real port.
>>   serial: 8250: Handle case port doesn't have TEMT interrupt using
>>     em485.
>>   serial: 8250_dw: treat rpm suspend with -EBUSY if RS485 ON and
>>     RTS_AFTER_SEND
>>   serial: 8250: Copy mctrl when register port.
>>   serial: 8250: Make em485_rts_after_send() set mctrl according to rts
>>     state.
>>   serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and
>>     RTS_AFTER_SEND set.
>>
>>  drivers/tty/serial/8250/8250.h      |  2 +-
>>  drivers/tty/serial/8250/8250_core.c |  2 ++
>>  drivers/tty/serial/8250/8250_dw.c   | 41
>> ++++++++++++++++++++++++++++-
>>  drivers/tty/serial/8250/8250_omap.c |  2 +-
>>  drivers/tty/serial/8250/8250_port.c | 33 ++++++++++++++++-------
>>  drivers/tty/serial/serial_core.c    | 12 ++++++++-
>>  include/linux/serial_8250.h         |  1 +
>>  7 files changed, 79 insertions(+), 14 deletions(-)
>>
>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
>



-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
119234, Moscow, Universitetsky pr-k 13, +7 (495) 9392382

^ permalink raw reply

* Re: [PATCH 0/8] serial: 8250: Add 485 emulation to 8250_dw.
From: Giulio Benetti @ 2018-06-04 10:42 UTC (permalink / raw)
  To: Matwey V. Kornilov, Andy Shevchenko
  Cc: Greg Kroah-Hartman, Jiri Slaby, Kees Cook, Matthias Brugger,
	Allen Pais, Sean Young, Ed Blake, Stefan Potyra, Philipp Zabel,
	Joshua Scott, Vignesh R, Rolf Evers-Fischer, Aaron Sierra,
	Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <CAJs94EZEEBNZRU3ixkdG9w44xsQDi7MUYAya2dzkAErT=xwqVA@mail.gmail.com>

Hi everybody,

Il 04/06/2018 12:34, Matwey V. Kornilov ha scritto:
> 2018-06-04 13:12 GMT+03:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>> On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
>>> Need to handle rs485 with 8250_dw port.
>>>
>>> Use existing em485 emulation layer for 8250 taking care to fix some
>>> bug
>>> and taking care especially of RTS_AFTER_SEND case.
>>>
>>
>> I don't see in Cc list author of rs485 code, who might be interested in
>> this.
>>
>> So, added him here.

Thanks Andy for adding in Cc an reviewing.
Dumb question: how did you find him?
I thought he was between people on get_maintainer.pl output.

>>
> 
> Hi, Andy
> 
> Nice to see you there. I will look through the code later.
> I always thought that DW has its own hardware rs485 support.

Hi Matwey,
I've checked on Synopsis Designware 8250 datasheet and it's not supported.
Here is datasheet I went through:
https://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf

On IER register there is not TEMT bit or something like that used in 
8250_omap.

Am I right?

Thanks everybody and
best regards

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

> 
>>> Giulio Benetti (8):
>>>    serial: 8250_dw: add em485 support
>>>    serial: 8250_dw: allow enable rs485 at boot time
>>>    serial: 8250: Copy em485 from port to real port.
>>>    serial: 8250: Handle case port doesn't have TEMT interrupt using
>>>      em485.
>>>    serial: 8250_dw: treat rpm suspend with -EBUSY if RS485 ON and
>>>      RTS_AFTER_SEND
>>>    serial: 8250: Copy mctrl when register port.
>>>    serial: 8250: Make em485_rts_after_send() set mctrl according to rts
>>>      state.
>>>    serial: core: Mask mctrl with TIOCM_RTS too if rs485 on and
>>>      RTS_AFTER_SEND set.
>>>
>>>   drivers/tty/serial/8250/8250.h      |  2 +-
>>>   drivers/tty/serial/8250/8250_core.c |  2 ++
>>>   drivers/tty/serial/8250/8250_dw.c   | 41
>>> ++++++++++++++++++++++++++++-
>>>   drivers/tty/serial/8250/8250_omap.c |  2 +-
>>>   drivers/tty/serial/8250/8250_port.c | 33 ++++++++++++++++-------
>>>   drivers/tty/serial/serial_core.c    | 12 ++++++++-
>>>   include/linux/serial_8250.h         |  1 +
>>>   7 files changed, 79 insertions(+), 14 deletions(-)
>>>
>>
>> --
>> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Intel Finland Oy
>>
> 
> 
> 

^ permalink raw reply

* Re: [PATCH 4/8] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Giulio Benetti @ 2018-06-04 10:50 UTC (permalink / raw)
  To: Andy Shevchenko, Greg Kroah-Hartman
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel
In-Reply-To: <3a66327727d9bf2ce5adf8ef0f1fcc1fffeaa4ec.camel@linux.intel.com>

Hi,

Il 04/06/2018 12:17, Andy Shevchenko ha scritto:
> On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
>> Some 8250 ports only have TEMT interrupt, so current implementation
>> can't work for ports without it. The only chance to make it work is to
>> loop-read on LSR register.
>>
>> With NO TEMT interrupt check if both TEMT and THRE are set looping on
>> LSR register.
> 
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
> 
>> -		int ret = serial8250_em485_init(up);
>> +		int ret = serial8250_em485_init(up, false);
> 
> Is true for all possible DW configured types? Or it's your particular
> case?
> 

I've checked on Synopsis Designware 8250 datasheet and it's not supported.
Here is datasheet I went through:
https://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf

There seems not to be TEMT interrupt, I use it under sunxi SoC and on 
their datasheet(A20 for example), they don't report that interrupt too.
So it seems to be valid for all DW configured types, anyway I don't know 
how many IP reviews there could be of that peripheral.
I've tried to subscribe at Synopsis to obtain latest Datasheet but it 
ask me an active ID I don't have.

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* Re: [PATCH 3/8] serial: 8250: Copy em485 from port to real port.
From: Giulio Benetti @ 2018-06-04 10:52 UTC (permalink / raw)
  To: Andy Shevchenko, Greg Kroah-Hartman
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel
In-Reply-To: <d282884a4df00d3f010c6af77c61ce07da75adc3.camel@linux.intel.com>

Hi,

Il 04/06/2018 12:13, Andy Shevchenko ha scritto:
> On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
>> em485 gets lost during serial8250_register_8250_port().
>>
>> Copy em485 to final uart port.
> 
> Fixes better to go first.
> I think you need to reorder the series.

Ok, thanks.
So after re-ording and clarifying TEMT interrupt, can I send v2 patchset?

Thanks

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* Re: [PATCH 4/8] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Andy Shevchenko @ 2018-06-04 11:38 UTC (permalink / raw)
  To: Giulio Benetti, Greg Kroah-Hartman
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel
In-Reply-To: <4a7148d5-ab2c-425d-afdc-08ddd3c522c2@micronovasrl.com>

On Mon, 2018-06-04 at 12:50 +0200, Giulio Benetti wrote:
> Hi,
> 
> Il 04/06/2018 12:17, Andy Shevchenko ha scritto:
> > On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
> > > Some 8250 ports only have TEMT interrupt, so current
> > > implementation
> > > can't work for ports without it. The only chance to make it work
> > > is to
> > > loop-read on LSR register.
> > > 
> > > With NO TEMT interrupt check if both TEMT and THRE are set looping
> > > on
> > > LSR register.
> > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > -		int ret = serial8250_em485_init(up);
> > > +		int ret = serial8250_em485_init(up, false);
> > 
> > Is true for all possible DW configured types? Or it's your
> > particular
> > case?
> > 
> 
> I've checked on Synopsis Designware 8250 datasheet and it's not
> supported.
> Here is datasheet I went through:
> https://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf
> 
> There seems not to be TEMT interrupt, I use it under sunxi SoC and on 
> their datasheet(A20 for example), they don't report that interrupt
> too.
> So it seems to be valid for all DW configured types, anyway I don't
> know 
> how many IP reviews there could be of that peripheral.

This is an excerpt from the document you referred to:

--- 8< --- 8< ---

6 TEMT R Transmitter Empty bit. If in FIFO mode (FIFO_MODE != NONE) and
FIFOs enabled (FCR[0] set to one), this bit is set whenever the
Transmitter Shift Register and the FIFO are both empty. If in non-FIFO
mode or FIFOs are disabled, this bit is set whenever the Transmitter
Holding Register and the Transmitter Shift Register are both empty.

Reset Value: 0x1

--- 8< --- 8< ---


If I'm reading this correctly the support is there. Or otherwise, care
to point exact paragraph needs to be read and checked?

> I've tried to subscribe at Synopsis to obtain latest Datasheet but it 
> ask me an active ID I don't have.
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 0/8] serial: 8250: Add 485 emulation to 8250_dw.
From: Andy Shevchenko @ 2018-06-04 11:44 UTC (permalink / raw)
  To: Giulio Benetti, Matwey V. Kornilov
  Cc: Greg Kroah-Hartman, Jiri Slaby, Kees Cook, Matthias Brugger,
	Allen Pais, Sean Young, Ed Blake, Stefan Potyra, Philipp Zabel,
	Joshua Scott, Vignesh R, Rolf Evers-Fischer, Aaron Sierra,
	Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <c14c671c-3240-0170-7c0e-2b195c5b1398@micronovasrl.com>

On Mon, 2018-06-04 at 12:42 +0200, Giulio Benetti wrote:

> > > I don't see in Cc list author of rs485 code, who might be
> > > interested in
> > > this.
> > > 
> > > So, added him here.
> 
> Thanks Andy for adding in Cc an reviewing.
> Dumb question: how did you find him?

I'm just following mailing lists somehow.
Though you always can do `git annotate` to see who lately did change
what.

> I thought he was between people on get_maintainer.pl output.

Hmm... probably. Never checked myself for this particular case.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 1/2] serial: 8250: enable SERIAL_MCTRL_GPIO by default.
From: Andy Shevchenko @ 2018-06-04 11:49 UTC (permalink / raw)
  To: Giulio Benetti, Greg Kroah-Hartman
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel
In-Reply-To: <20180601141158.93817-1-giulio.benetti@micronovasrl.com>

On Fri, 2018-06-01 at 16:11 +0200, Giulio Benetti wrote:
> It can be useful to override 8250 mctrl lines with gpios, for rts on
> rs485 for example, when rts is not mapped correctly to HW RTS pin.
> 
> Enable SERIAL_MCTRL_GPIO by default.
> 

Unfortunately NAK, see 

commit 5db4f7f80d165fc9725f356e99feec409e446baa
Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date:   Tue Aug 16 15:06:54 2016 +0300

    Revert "tty/serial/8250: use mctrl_gpio helpers"

for the details.

I would love to see a solution that will satisfy everyone, though I have
only means to test proposals for now.

> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/tty/serial/8250/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/8250/Kconfig
> b/drivers/tty/serial/8250/Kconfig
> index f005eaf8bc57..c7992b94fece 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -7,6 +7,7 @@ config SERIAL_8250
>  	tristate "8250/16550 and compatible serial support"
>  	depends on !S390
>  	select SERIAL_CORE
> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>  	---help---
>  	  This selects whether you want to include the driver for the
> standard
>  	  serial ports.  The standard answer is Y.  People who might
> say N

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [PATCH 4/8] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Giulio Benetti @ 2018-06-04 11:50 UTC (permalink / raw)
  To: Andy Shevchenko, Greg Kroah-Hartman
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel
In-Reply-To: <2a2f547d787db9d593bb7fe3ad9c833836e23749.camel@linux.intel.com>

Hi,

Il 04/06/2018 13:38, Andy Shevchenko ha scritto:
> On Mon, 2018-06-04 at 12:50 +0200, Giulio Benetti wrote:
>> Hi,
>>
>> Il 04/06/2018 12:17, Andy Shevchenko ha scritto:
>>> On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
>>>> Some 8250 ports only have TEMT interrupt, so current
>>>> implementation
>>>> can't work for ports without it. The only chance to make it work
>>>> is to
>>>> loop-read on LSR register.
>>>>
>>>> With NO TEMT interrupt check if both TEMT and THRE are set looping
>>>> on
>>>> LSR register.
>>>> --- a/drivers/tty/serial/8250/8250_dw.c
>>>> +++ b/drivers/tty/serial/8250/8250_dw.c
>>>> -		int ret = serial8250_em485_init(up);
>>>> +		int ret = serial8250_em485_init(up, false);
>>>
>>> Is true for all possible DW configured types? Or it's your
>>> particular
>>> case?
>>>
>>
>> I've checked on Synopsis Designware 8250 datasheet and it's not
>> supported.
>> Here is datasheet I went through:
>> https://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf
>>
>> There seems not to be TEMT interrupt, I use it under sunxi SoC and on
>> their datasheet(A20 for example), they don't report that interrupt
>> too.
>> So it seems to be valid for all DW configured types, anyway I don't
>> know
>> how many IP reviews there could be of that peripheral.
> 
> This is an excerpt from the document you referred to:
> 
> --- 8< --- 8< ---
> 
> 6 TEMT R Transmitter Empty bit. If in FIFO mode (FIFO_MODE != NONE) and
> FIFOs enabled (FCR[0] set to one), this bit is set whenever the
> Transmitter Shift Register and the FIFO are both empty. If in non-FIFO
> mode or FIFOs are disabled, this bit is set whenever the Transmitter
> Holding Register and the Transmitter Shift Register are both empty.
> 
> Reset Value: 0x1
> 
> --- 8< --- 8< ---
> 
> 
> If I'm reading this correctly the support is there. Or otherwise, care
> to point exact paragraph needs to be read and checked?

In the beginning I thought the same as you but
unfortunately LSR is only a status register and IER doesn't have 
corresponding TEMT bit to enable an interrupt on TEMT triggering.
On OMAP instead there is a specific interrupt bound to TEMT LSR flag.
And THRE interrupt is not enough because shift register won't be empty 
when it triggers, so you would loose some bit of last byte to be 
transmitted.

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* Re: [PATCH 4/8] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Andy Shevchenko @ 2018-06-04 12:26 UTC (permalink / raw)
  To: Giulio Benetti, Greg Kroah-Hartman, Matwey V.Kornilov
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel
In-Reply-To: <2c61887c-53e0-1ece-9f4a-89250134f083@micronovasrl.com>

On Mon, 2018-06-04 at 13:50 +0200, Giulio Benetti wrote:
> Hi,
> 
> Il 04/06/2018 13:38, Andy Shevchenko ha scritto:
> > On Mon, 2018-06-04 at 12:50 +0200, Giulio Benetti wrote:
> > > Hi,
> > > 
> > > Il 04/06/2018 12:17, Andy Shevchenko ha scritto:
> > > > On Fri, 2018-06-01 at 14:40 +0200, Giulio Benetti wrote:
> > > > > Some 8250 ports only have TEMT interrupt, so current
> > > > > implementation
> > > > > can't work for ports without it. The only chance to make it
> > > > > work
> > > > > is to
> > > > > loop-read on LSR register.
> > > > > 
> > > > > With NO TEMT interrupt check if both TEMT and THRE are set
> > > > > looping
> > > > > on
> > > > > LSR register.
> > > > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > > > -		int ret = serial8250_em485_init(up);
> > > > > +		int ret = serial8250_em485_init(up, false);
> > > > 
> > > > Is true for all possible DW configured types? Or it's your
> > > > particular
> > > > case?
> > > > 
> > > 
> > > I've checked on Synopsis Designware 8250 datasheet and it's not
> > > supported.
> > > Here is datasheet I went through:
> > > https://linux-sunxi.org/images/d/d2/Dw_apb_uart_db.pdf
> > > 
> > > There seems not to be TEMT interrupt, I use it under sunxi SoC and
> > > on
> > > their datasheet(A20 for example), they don't report that interrupt
> > > too.
> > > So it seems to be valid for all DW configured types, anyway I
> > > don't
> > > know
> > > how many IP reviews there could be of that peripheral.
> > 
> > This is an excerpt from the document you referred to:
> > 
> > --- 8< --- 8< ---
> > 
> > 6 TEMT R Transmitter Empty bit. If in FIFO mode (FIFO_MODE != NONE)
> > and
> > FIFOs enabled (FCR[0] set to one), this bit is set whenever the
> > Transmitter Shift Register and the FIFO are both empty. If in non-
> > FIFO
> > mode or FIFOs are disabled, this bit is set whenever the Transmitter
> > Holding Register and the Transmitter Shift Register are both empty.
> > 
> > Reset Value: 0x1
> > 
> > --- 8< --- 8< ---
> > 
> > 
> > If I'm reading this correctly the support is there. Or otherwise,
> > care
> > to point exact paragraph needs to be read and checked?
> 
> In the beginning I thought the same as you but
> unfortunately LSR is only a status register and IER doesn't have 
> corresponding TEMT bit to enable an interrupt on TEMT triggering.
> On OMAP instead there is a specific interrupt bound to TEMT LSR flag.
> And THRE interrupt is not enough because shift register won't be
> empty 
> when it triggers, so you would loose some bit of last byte to be 
> transmitted.

Hmm... Okay, it's something you and Matwey better to discuss.

P.S. Latest version of document I have does describe RS485 HW supported
mode. I don't know if it was added recently to the IP itself, or just
missed documentation. That's what you need to clarify with Synopsys.

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* [PATCH v4.9-stable] serial: pl011: add console matching function
From: Ard Biesheuvel @ 2018-06-04 13:16 UTC (permalink / raw)
  To: stable
  Cc: leif.lindholm, graeme.gregory, gregkh, linux, jslaby,
	linux-serial, linux-arm-kernel, ard.biesheuvel

From: Aleksey Makarov <aleksey.makarov@linaro.org>

Commit 10879ae5f12e9cab3c4e8e9504c1aaa8a033bde7 upstream.

This patch adds function pl011_console_match() that implements
method match of struct console.  It allows to match consoles against
data specified in a string, for example taken from command line or
compiled by ACPI SPCR table handler.

This patch was merged to tty-next but then reverted because of
conflict with

commit 46e36683f433 ("serial: earlycon: Extend earlycon command line option to support 64-bit addresses")

Now it is fixed.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Tested-by: Christopher Covington <cov@codeaurora.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

Please consider for v4.9-stable. It is the missing puzzle piece for SPCR
support on arm64 ACPI systems, which got merged for v4.9 [0]. Now that more
systems are becoming available to people working in the kernel community, it
turns out that v4.9 distro installers (e.g., Debian Stretch) won't work
unless you pass a 'console=' parameter explicitly, which is annoying.
Given that it was clearly the intent to include this code at the time,
I hope it will be considered for backporting.

[0] To quote the tty maintainer:

      Also in here is the long-suffering ACPI SPCR patchset, which was
      passed around from maintainer to maintainer like a hot-potato. Seems I
      was the sucker^Wlucky one. All of those patches have been acked by the
      various subsystem maintainers as well.

 drivers/tty/serial/amba-pl011.c | 55 ++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index b42d7f1c9089..6b1863293fe1 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2320,12 +2320,67 @@ static int __init pl011_console_setup(struct console *co, char *options)
 	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
 }
 
+/**
+ *	pl011_console_match - non-standard console matching
+ *	@co:	  registering console
+ *	@name:	  name from console command line
+ *	@idx:	  index from console command line
+ *	@options: ptr to option string from console command line
+ *
+ *	Only attempts to match console command lines of the form:
+ *	    console=pl011,mmio|mmio32,<addr>[,<options>]
+ *	    console=pl011,0x<addr>[,<options>]
+ *	This form is used to register an initial earlycon boot console and
+ *	replace it with the amba_console at pl011 driver init.
+ *
+ *	Performs console setup for a match (as required by interface)
+ *	If no <options> are specified, then assume the h/w is already setup.
+ *
+ *	Returns 0 if console matches; otherwise non-zero to use default matching
+ */
+static int __init pl011_console_match(struct console *co, char *name, int idx,
+				      char *options)
+{
+	unsigned char iotype;
+	resource_size_t addr;
+	int i;
+
+	if (strcmp(name, "pl011") != 0)
+		return -ENODEV;
+
+	if (uart_parse_earlycon(options, &iotype, &addr, &options))
+		return -ENODEV;
+
+	if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
+		return -ENODEV;
+
+	/* try to match the port specified on the command line */
+	for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
+		struct uart_port *port;
+
+		if (!amba_ports[i])
+			continue;
+
+		port = &amba_ports[i]->port;
+
+		if (port->mapbase != addr)
+			continue;
+
+		co->index = i;
+		port->cons = co;
+		return pl011_console_setup(co, options);
+	}
+
+	return -ENODEV;
+}
+
 static struct uart_driver amba_reg;
 static struct console amba_console = {
 	.name		= "ttyAMA",
 	.write		= pl011_console_write,
 	.device		= uart_console_device,
 	.setup		= pl011_console_setup,
+	.match		= pl011_console_match,
 	.flags		= CON_PRINTBUFFER,
 	.index		= -1,
 	.data		= &amba_reg,
-- 
2.17.0

^ permalink raw reply related

* Re: [PATCH 0/8] serial: 8250: Add 485 emulation to 8250_dw.
From: Giulio Benetti @ 2018-06-04 14:58 UTC (permalink / raw)
  To: Andy Shevchenko, Matwey V. Kornilov
  Cc: Greg Kroah-Hartman, Jiri Slaby, Kees Cook, Matthias Brugger,
	Allen Pais, Sean Young, Ed Blake, Stefan Potyra, Philipp Zabel,
	Joshua Scott, Vignesh R, Rolf Evers-Fischer, Aaron Sierra,
	Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <7bdb658727b364e275c121a3bdd351f183d56cb9.camel@linux.intel.com>


Il 04/06/2018 13:44, Andy Shevchenko ha scritto:
> On Mon, 2018-06-04 at 12:42 +0200, Giulio Benetti wrote:
> 
>>>> I don't see in Cc list author of rs485 code, who might be
>>>> interested in
>>>> this.
>>>>
>>>> So, added him here.
>>
>> Thanks Andy for adding in Cc an reviewing.
>> Dumb question: how did you find him?
> 
> I'm just following mailing lists somehow.
> Though you always can do `git annotate` to see who lately did change
> what.
> 
>> I thought he was between people on get_maintainer.pl output.
> 
> Hmm... probably. Never checked myself for this particular case.
> 

Ok, thank you.

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* [PATCH v5 0/6] Driver for at91 usart in spi mode
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial, Radu Pirea

Hello,

This is the second version of driver. I added a mfd driver which by
default probes atmel_serial driver and if in dt is specified to probe
the spi driver, then the spi-at91-usart driver will be probed. The
compatible for atmel_serial is now the compatible for at91-usart mfd
driver and compatilbe for atmel_serial driver was changed in order to
keep the bindings for serial as they are.

Changes in v1:
- added spi-at91-usart driver

Changes in v2:
- added at91-usart mfd driver
- modified spi-at91-usart driver to work as mfd driver child
- modified atmel_serial driver to work as mfd driver child

Changes in v3:
- fixed spi slaves probing

Changes in v4:
- modified the spi driver to use cs gpio support form spi subsystem
- fixed dma transfers for serial driver
- squashed binding for spi and serial and moved them to mfd/atmel-usart.txt

Changes in v5:
- fixed usage of stdout-path property with atmel_serial driver

Radu Pirea (6):
  MAINTAINERS: add at91 usart mfd driver
  dt-bindings: add binding for atmel-usart in SPI mode
  mfd: at91-usart: added mfd driver for usart
  MAINTAINERS: add at91 usart spi driver
  spi: at91-usart: add driver for at91-usart as spi
  tty/serial: atmel: changed the driver to work under at91-usart mfd

 .../bindings/{serial => mfd}/atmel-usart.txt  |  25 +-
 MAINTAINERS                                   |  16 +
 drivers/mfd/Kconfig                           |   9 +
 drivers/mfd/Makefile                          |   1 +
 drivers/mfd/at91-usart.c                      |  67 +++
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-at91-usart.c                  | 436 ++++++++++++++++++
 drivers/tty/serial/Kconfig                    |   1 +
 drivers/tty/serial/atmel_serial.c             |  41 +-
 include/dt-bindings/mfd/at91-usart.h          |  17 +
 11 files changed, 604 insertions(+), 19 deletions(-)
 rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
 create mode 100644 drivers/mfd/at91-usart.c
 create mode 100644 drivers/spi/spi-at91-usart.c
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

-- 
2.17.1

^ permalink raw reply

* [PATCH v5 1/6] MAINTAINERS: add at91 usart mfd driver
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial, Radu Pirea
In-Reply-To: <20180604165943.31381-1-radu.pirea@microchip.com>

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8e2a2fddbd19..12203d07c6af 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9160,6 +9160,7 @@ M:	Richard Genoud <richard.genoud@gmail.com>
 S:	Maintained
 F:	drivers/tty/serial/atmel_serial.c
 F:	drivers/tty/serial/atmel_serial.h
+F:	Documentation/devicetree/bindings/mfd/atmel-usart.txt
 
 MICROCHIP / ATMEL DMA DRIVER
 M:	Ludovic Desroches <ludovic.desroches@microchip.com>
@@ -9192,6 +9193,14 @@ S:	Supported
 F:	drivers/mtd/nand/raw/atmel/*
 F:	Documentation/devicetree/bindings/mtd/atmel-nand.txt
 
+MICROCHIP AT91 USART MFD DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-kernel@vger.kernel.org
+S:	Supported
+F:	drivers/mfd/at91-usart.c
+F:	include/dt-bindings/mfd/at91-usart.h
+F:	Documentation/devicetree/bindings/mfd/atmel-usart.txt
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 2/6] dt-bindings: add binding for atmel-usart in SPI mode
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: devicetree, linux-kernel, linux-spi, linux-serial, Radu Pirea,
	linux-arm-kernel
In-Reply-To: <20180604165943.31381-1-radu.pirea@microchip.com>

This patch moves the bindings for serial from serial/atmel-usart.txt to
mfd/atmel-usart.txt and adds bindings for USART in SPI mode.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/{serial => mfd}/atmel-usart.txt  | 25 +++++++++++++++++--
 include/dt-bindings/mfd/at91-usart.h          | 17 +++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)
 rename Documentation/devicetree/bindings/{serial => mfd}/atmel-usart.txt (76%)
 create mode 100644 include/dt-bindings/mfd/at91-usart.h

diff --git a/Documentation/devicetree/bindings/serial/atmel-usart.txt b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
similarity index 76%
rename from Documentation/devicetree/bindings/serial/atmel-usart.txt
rename to Documentation/devicetree/bindings/mfd/atmel-usart.txt
index 7c0d6b2f53e4..3b9e18642c3b 100644
--- a/Documentation/devicetree/bindings/serial/atmel-usart.txt
+++ b/Documentation/devicetree/bindings/mfd/atmel-usart.txt
@@ -1,6 +1,6 @@
 * Atmel Universal Synchronous Asynchronous Receiver/Transmitter (USART)
 
-Required properties:
+Required properties for USART:
 - compatible: Should be "atmel,<chip>-usart" or "atmel,<chip>-dbgu"
   The compatible <chip> indicated will be the first SoC to support an
   additional mode or an USART new feature.
@@ -11,7 +11,13 @@ Required properties:
 	Required elements: "usart"
 - clocks: phandles to input clocks.
 
-Optional properties:
+Required properties for USART in SPI mode:
+- #size-cells      : Must be <0>
+- #address-cells   : Must be <1>
+- cs-gpios: chipselects (internal cs not supported)
+- atmel,usart-mode : Must be <USART_MODE_SPI> (found in dt-bindings/mfd/at91-usart.h)
+
+Optional properties in serial mode:
 - atmel,use-dma-rx: use of PDC or DMA for receiving data
 - atmel,use-dma-tx: use of PDC or DMA for transmitting data
 - {rts,cts,dtr,dsr,rng,dcd}-gpios: specify a GPIO for RTS/CTS/DTR/DSR/RI/DCD line respectively.
@@ -62,3 +68,18 @@ Example:
 		dma-names = "tx", "rx";
 		atmel,fifo-size = <32>;
 	};
+
+- SPI mode:
+	#include <dt-bindings/mfd/at91-usart.h>
+
+	spi0: spi@f001c000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "atmel,at91rm9200-usart", "atmel,at91sam9260-usart";
+		atmel,usart-mode = <USART_MODE_SPI>;
+		reg = <0xf001c000 0x100>;
+		interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&usart0_clk>;
+		clock-names = "usart";
+		cs-gpios = <&pioB 3 0>;
+	};
diff --git a/include/dt-bindings/mfd/at91-usart.h b/include/dt-bindings/mfd/at91-usart.h
new file mode 100644
index 000000000000..ac811628a42d
--- /dev/null
+++ b/include/dt-bindings/mfd/at91-usart.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * This header provides macros for AT91 USART DT bindings.
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#ifndef __DT_BINDINGS_AT91_USART_H__
+#define __DT_BINDINGS_AT91_USART_H__
+
+#define AT91_USART_MODE_SERIAL	1
+#define AT91_USART_MODE_SPI	2
+
+#endif /* __DT_BINDINGS_AT91_USART_H__ */
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 3/6] mfd: at91-usart: added mfd driver for usart
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial, Radu Pirea
In-Reply-To: <20180604165943.31381-1-radu.pirea@microchip.com>

This mfd driver is just a wrapper over atmel_serial driver and
spi-at91-usart driver. Selection of one of the drivers is based on a
property from device tree. If the property is not specified, the default
driver is atmel_serial.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
Acked-by: Rob Herring <robh@kernel.org>
---
 drivers/mfd/Kconfig      |  9 ++++++
 drivers/mfd/Makefile     |  1 +
 drivers/mfd/at91-usart.c | 67 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 drivers/mfd/at91-usart.c

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index b860eb5aa194..a886672b960d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -99,6 +99,15 @@ config MFD_AAT2870_CORE
 	  additional drivers must be enabled in order to use the
 	  functionality of the device.
 
+config MFD_AT91_USART
+	tristate "AT91 USART Driver"
+	select MFD_CORE
+	help
+	  Select this to get support for AT91 USART IP. This is a wrapper
+	  over at91-usart-serial driver and usart-spi-driver. Only one function
+	  can be used at a time. The choice is done at boot time by the probe
+	  function of this MFD driver according to a device tree property.
+
 config MFD_ATMEL_FLEXCOM
 	tristate "Atmel Flexcom (Flexible Serial Communication Unit)"
 	select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index d9d2cf0d32ef..db1332aa96db 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -185,6 +185,7 @@ obj-$(CONFIG_MFD_SPMI_PMIC)	+= qcom-spmi-pmic.o
 obj-$(CONFIG_TPS65911_COMPARATOR)	+= tps65911-comparator.o
 obj-$(CONFIG_MFD_TPS65090)	+= tps65090.o
 obj-$(CONFIG_MFD_AAT2870_CORE)	+= aat2870-core.o
+obj-$(CONFIG_MFD_AT91_USART)	+= at91-usart.o
 obj-$(CONFIG_MFD_ATMEL_FLEXCOM)	+= atmel-flexcom.o
 obj-$(CONFIG_MFD_ATMEL_HLCDC)	+= atmel-hlcdc.o
 obj-$(CONFIG_MFD_ATMEL_SMC)	+= atmel-smc.o
diff --git a/drivers/mfd/at91-usart.c b/drivers/mfd/at91-usart.c
new file mode 100644
index 000000000000..262e7affba22
--- /dev/null
+++ b/drivers/mfd/at91-usart.c
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Driver for AT91 USART
+ *
+ * Copyright (C) 2018 Microchip Technology
+ *
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ *
+ */
+
+#include <dt-bindings/mfd/at91-usart.h>
+
+#include <linux/module.h>
+#include <linux/mfd/core.h>
+#include <linux/property.h>
+
+static struct mfd_cell at91_usart_spi_subdev = {
+		.name = "at91_usart_spi",
+		.of_compatible = "microchip,at91sam9g45-usart-spi",
+	};
+
+static struct mfd_cell at91_usart_serial_subdev = {
+		.name = "atmel_usart_serial",
+		.of_compatible = "atmel,at91rm9200-usart-serial",
+	};
+
+static int at91_usart_mode_probe(struct platform_device *pdev)
+{
+	struct mfd_cell cell;
+	u32 opmode;
+	int err;
+
+	err = device_property_read_u32(&pdev->dev, "atmel,usart-mode", &opmode);
+
+	switch (opmode) {
+	case AT91_USART_MODE_SPI:
+		cell = at91_usart_spi_subdev;
+		break;
+	case AT91_USART_MODE_SERIAL:
+	default:
+		cell = at91_usart_serial_subdev;
+	}
+
+	return devm_mfd_add_devices(&pdev->dev, PLATFORM_DEVID_AUTO, &cell, 1,
+			      NULL, 0, NULL);
+}
+
+static const struct of_device_id at91_usart_mode_of_match[] = {
+	{ .compatible = "atmel,at91rm9200-usart" },
+	{ .compatible = "atmel,at91sam9260-usart" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, at91_flexcom_of_match);
+
+static struct platform_driver at91_usart_mfd = {
+	.probe	= at91_usart_mode_probe,
+	.driver	= {
+		.name		= "at91_usart_mode",
+		.of_match_table	= at91_usart_mode_of_match,
+	},
+};
+
+module_platform_driver(at91_usart_mfd);
+
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_DESCRIPTION("AT91 USART MFD driver");
+MODULE_LICENSE("GPL v2");
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 4/6] MAINTAINERS: add at91 usart spi driver
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial, Radu Pirea
In-Reply-To: <20180604165943.31381-1-radu.pirea@microchip.com>

Added entry for at91 usart mfd driver.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 12203d07c6af..dae31df711fb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9201,6 +9201,13 @@ F:	drivers/mfd/at91-usart.c
 F:	include/dt-bindings/mfd/at91-usart.h
 F:	Documentation/devicetree/bindings/mfd/atmel-usart.txt
 
+MICROCHIP AT91 USART SPI DRIVER
+M:	Radu Pirea <radu.pirea@microchip.com>
+L:	linux-spi@vger.kernel.org
+S:	Supported
+F:	drivers/spi/spi-at91-usart.c
+F:	Documentation/devicetree/bindings/mfd/atmel-usart.txt
+
 MICROCHIP KSZ SERIES ETHERNET SWITCH DRIVER
 M:	Woojung Huh <Woojung.Huh@microchip.com>
 M:	Microchip Linux Driver Support <UNGLinuxDriver@microchip.com>
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial, Radu Pirea
In-Reply-To: <20180604165943.31381-1-radu.pirea@microchip.com>

This is the driver for at91-usart in spi mode. The USART IP can be configured
to work in many modes and one of them is SPI.

The driver was tested on sama5d3-xplained and sama5d4-xplained boards with
enc28j60 ethernet controller as slave.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/spi/Kconfig          |   9 +
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-at91-usart.c | 436 +++++++++++++++++++++++++++++++++++
 3 files changed, 446 insertions(+)
 create mode 100644 drivers/spi/spi-at91-usart.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 6fb0347a24f2..1a002a32d7aa 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -77,6 +77,15 @@ config SPI_ATMEL
 	  This selects a driver for the Atmel SPI Controller, present on
 	  many AT91 (ARM) chips.
 
+config SPI_AT91_USART
+	tristate "Atmel USART Controller SPI driver"
+	depends on HAS_DMA
+	depends on (ARCH_AT91 || COMPILE_TEST)
+	select MFD_AT91_USART
+	help
+	  This selects a driver for the AT91 USART Controller as SPI Master,
+	  present on AT91 and SAMA5 SoC series.
+
 config SPI_AU1550
 	tristate "Au1550/Au1200/Au1300 SPI Controller"
 	depends on MIPS_ALCHEMY
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 34c5f2832ddf..fb6cb42f4eaa 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SPI_LOOPBACK_TEST)		+= spi-loopback-test.o
 obj-$(CONFIG_SPI_ALTERA)		+= spi-altera.o
 obj-$(CONFIG_SPI_ARMADA_3700)		+= spi-armada-3700.o
 obj-$(CONFIG_SPI_ATMEL)			+= spi-atmel.o
+obj-$(CONFIG_SPI_AT91_USART)		+= spi-at91-usart.o
 obj-$(CONFIG_SPI_ATH79)			+= spi-ath79.o
 obj-$(CONFIG_SPI_AU1550)		+= spi-au1550.o
 obj-$(CONFIG_SPI_AXI_SPI_ENGINE)	+= spi-axi-spi-engine.o
diff --git a/drivers/spi/spi-at91-usart.c b/drivers/spi/spi-at91-usart.c
new file mode 100644
index 000000000000..be7495527c72
--- /dev/null
+++ b/drivers/spi/spi-at91-usart.c
@@ -0,0 +1,436 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for AT91 USART Controllers as SPI
+ *
+ * Copyright (C) 2018 Microchip Technology Inc.
+ * Author: Radu Pirea <radu.pirea@microchip.com>
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/platform_device.h>
+
+#include <linux/spi/spi.h>
+
+#define US_CR			0x00
+#define US_MR			0x04
+#define US_IER			0x08
+#define US_IDR			0x0C
+#define US_CSR			0x14
+#define US_RHR			0x18
+#define US_THR			0x1C
+#define US_BRGR			0x20
+#define US_VERSION		0xFC
+
+#define US_CR_RSTRX		BIT(2)
+#define US_CR_RSTTX		BIT(3)
+#define US_CR_RXEN		BIT(4)
+#define US_CR_RXDIS		BIT(5)
+#define US_CR_TXEN		BIT(6)
+#define US_CR_TXDIS		BIT(7)
+
+#define US_MR_SPI_MASTER	0x0E
+#define US_MR_CHRL		GENMASK(7, 6)
+#define US_MR_CPHA		BIT(8)
+#define US_MR_CPOL		BIT(16)
+#define US_MR_CLKO		BIT(18)
+#define US_MR_WRDBT		BIT(20)
+#define US_MR_LOOP		BIT(15)
+
+#define US_IR_RXRDY		BIT(0)
+#define US_IR_TXRDY		BIT(1)
+#define US_IR_OVRE		BIT(5)
+
+#define US_BRGR_SIZE		BIT(16)
+
+#define US_MIN_CLK_DIV		0x06
+#define US_MAX_CLK_DIV		BIT(16)
+
+#define US_RESET		(US_CR_RSTRX | US_CR_RSTTX)
+#define US_DISABLE		(US_CR_RXDIS | US_CR_TXDIS)
+#define US_ENABLE		(US_CR_RXEN | US_CR_TXEN)
+#define US_OVRE_RXRDY_IRQS	(US_IR_OVRE | US_IR_RXRDY)
+
+#define US_INIT \
+	(US_MR_SPI_MASTER | US_MR_CHRL | US_MR_CLKO | US_MR_WRDBT)
+
+/* Register access macros */
+#define at91_usart_spi_readl(port, reg) \
+	readl_relaxed((port)->regs + US_##reg)
+#define at91_usart_spi_writel(port, reg, value) \
+	writel_relaxed((value), (port)->regs + US_##reg)
+
+#define at91_usart_spi_readb(port, reg) \
+	readb_relaxed((port)->regs + US_##reg)
+#define at91_usart_spi_writeb(port, reg, value) \
+	writeb_relaxed((value), (port)->regs + US_##reg)
+
+struct at91_usart_spi {
+	struct spi_transfer	*current_transfer;
+	void __iomem		*regs;
+	struct device		*dev;
+	struct clk		*clk;
+
+	/*used in interrupt to protect data reading*/
+	spinlock_t		lock;
+
+	int			irq;
+	unsigned int		current_tx_remaining_bytes;
+	unsigned int		current_rx_remaining_bytes;
+	int			done_status;
+
+	u32			spi_clk;
+	u32			status;
+
+	bool			xfer_failed;
+};
+
+static inline u32 at91_usart_spi_tx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_TXRDY;
+}
+
+static inline u32 at91_usart_spi_rx_ready(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_RXRDY;
+}
+
+static inline u32 at91_usart_spi_check_overrun(struct at91_usart_spi *aus)
+{
+	return aus->status & US_IR_OVRE;
+}
+
+static inline u32 at91_usart_spi_read_status(struct at91_usart_spi *aus)
+{
+	aus->status = at91_usart_spi_readl(aus, CSR);
+	return aus->status;
+}
+
+static inline void at91_usart_spi_tx(struct at91_usart_spi *aus)
+{
+	unsigned int len = aus->current_transfer->len;
+	unsigned int remaining = aus->current_tx_remaining_bytes;
+	const u8  *tx_buf = aus->current_transfer->tx_buf;
+
+	if (!remaining)
+		return;
+
+	if (at91_usart_spi_tx_ready(aus)) {
+		at91_usart_spi_writeb(aus, THR, tx_buf[len - remaining]);
+		aus->current_tx_remaining_bytes--;
+	}
+}
+
+static inline void at91_usart_spi_rx(struct at91_usart_spi *aus)
+{
+	int len = aus->current_transfer->len;
+	int remaining = aus->current_rx_remaining_bytes;
+	u8  *rx_buf = aus->current_transfer->rx_buf;
+
+	if (!remaining)
+		return;
+
+	rx_buf[len - remaining] = at91_usart_spi_readb(aus, RHR);
+	aus->current_rx_remaining_bytes--;
+}
+
+static inline void
+at91_usart_spi_set_xfer_speed(struct at91_usart_spi *aus,
+			      struct spi_transfer *xfer)
+{
+	at91_usart_spi_writel(aus, BRGR,
+			      DIV_ROUND_UP(aus->spi_clk, xfer->speed_hz));
+}
+
+static irqreturn_t at91_usart_spi_interrupt(int irq, void *dev_id)
+{
+	struct spi_controller *controller = dev_id;
+	struct at91_usart_spi *aus = spi_master_get_devdata(controller);
+
+	spin_lock(&aus->lock);
+	at91_usart_spi_read_status(aus);
+
+	if (at91_usart_spi_check_overrun(aus)) {
+		aus->xfer_failed = true;
+		aus->done_status = -EIO;
+		at91_usart_spi_writel(aus, IDR, US_IR_OVRE | US_IR_RXRDY);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+
+	if (at91_usart_spi_rx_ready(aus)) {
+		at91_usart_spi_rx(aus);
+		spin_unlock(&aus->lock);
+		return IRQ_HANDLED;
+	}
+
+	spin_unlock(&aus->lock);
+
+	return IRQ_NONE;
+}
+
+static int at91_usart_spi_setup(struct spi_device *spi)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(spi->controller);
+	u32 *ausd = spi->controller_state;
+	unsigned int mr = at91_usart_spi_readl(aus, MR);
+	u8 bits = spi->bits_per_word;
+
+	if (bits != 8) {
+		dev_dbg(&spi->dev, "Only 8 bits per word are supported\n");
+		return -EINVAL;
+	}
+
+	if (spi->mode & SPI_CPOL)
+		mr |= US_MR_CPOL;
+	else
+		mr &= ~US_MR_CPOL;
+
+	if (spi->mode & SPI_CPHA)
+		mr |= US_MR_CPHA;
+	else
+		mr &= ~US_MR_CPHA;
+
+	if (spi->mode & SPI_LOOP)
+		mr |= US_MR_LOOP;
+	else
+		mr &= ~US_MR_LOOP;
+
+	if (!ausd) {
+		ausd = kzalloc(sizeof(*ausd), GFP_KERNEL);
+		if (!ausd)
+			return -ENOMEM;
+
+		spi->controller_state = ausd;
+	}
+
+	*ausd = mr;
+
+	dev_dbg(&spi->dev,
+		"setup: bpw %u mode 0x%x -> mr %d %08x\n",
+		bits, spi->mode, spi->chip_select, mr);
+
+	return 0;
+}
+
+int at91_usart_spi_transfer_one(struct spi_controller *ctlr,
+				struct spi_device *spi,
+				struct spi_transfer *xfer)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(ctlr);
+
+	at91_usart_spi_set_xfer_speed(aus, xfer);
+	aus->done_status = 0;
+	aus->xfer_failed = false;
+	aus->current_transfer = xfer;
+	aus->current_tx_remaining_bytes = xfer->len;
+	aus->current_rx_remaining_bytes = xfer->len;
+
+	while ((aus->current_tx_remaining_bytes ||
+		aus->current_rx_remaining_bytes) && !aus->xfer_failed) {
+		at91_usart_spi_read_status(aus);
+		at91_usart_spi_tx(aus);
+		cpu_relax();
+	}
+	if (aus->xfer_failed) {
+		dev_err(aus->dev, "Overrun!\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+int at91_usart_spi_prepare_message(struct spi_controller *ctlr,
+				   struct spi_message *message)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(ctlr);
+	struct spi_device *spi = message->spi;
+	u32 *ausd = spi->controller_state;
+
+	at91_usart_spi_writel(aus, CR, US_ENABLE);
+	at91_usart_spi_writel(aus, IER, US_OVRE_RXRDY_IRQS);
+	at91_usart_spi_writel(aus, MR, *ausd);
+
+	return 0;
+}
+
+int at91_usart_spi_unprepare_message(struct spi_controller *ctlr,
+				     struct spi_message *message)
+{
+	struct at91_usart_spi *aus = spi_master_get_devdata(ctlr);
+
+	at91_usart_spi_writel(aus, CR, US_RESET | US_DISABLE);
+	at91_usart_spi_writel(aus, IDR, US_OVRE_RXRDY_IRQS);
+
+	return 0;
+}
+
+static void at91_usart_spi_cleanup(struct spi_device *spi)
+{
+	struct at91_usart_spi_device *ausd = spi->controller_state;
+
+	spi->controller_state = NULL;
+	kfree(ausd);
+}
+
+static void at91_usart_spi_init(struct at91_usart_spi *aus)
+{
+	at91_usart_spi_writel(aus, MR, US_INIT);
+	at91_usart_spi_writel(aus, CR, US_RESET | US_DISABLE);
+}
+
+static int at91_usart_gpio_setup(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.parent->of_node;
+	int i;
+	int ret;
+	int nb;
+
+	if (!np)
+		return -EINVAL;
+
+	nb = of_gpio_named_count(np, "cs-gpios");
+	for (i = 0; i < nb; i++) {
+		int cs_gpio = of_get_named_gpio(np, "cs-gpios", i);
+
+		if (cs_gpio < 0)
+			return cs_gpio;
+
+		if (gpio_is_valid(cs_gpio)) {
+			ret = devm_gpio_request_one(&pdev->dev, cs_gpio,
+						    GPIOF_DIR_OUT,
+						    dev_name(&pdev->dev));
+			if (ret)
+				return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int at91_usart_spi_probe(struct platform_device *pdev)
+{
+	struct resource *regs;
+	struct spi_controller *controller;
+	struct at91_usart_spi *aus;
+	struct clk *clk;
+	int irq;
+	int ret;
+
+	regs = platform_get_resource(to_platform_device(pdev->dev.parent),
+				     IORESOURCE_MEM, 0);
+	if (!regs)
+		return -EINVAL;
+
+	irq = platform_get_irq(to_platform_device(pdev->dev.parent), 0);
+	if (irq < 0)
+		return irq;
+
+	clk = devm_clk_get(pdev->dev.parent, "usart");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	ret = -ENOMEM;
+	controller = spi_alloc_master(&pdev->dev, sizeof(*aus));
+	if (!controller)
+		goto at91_usart_spi_probe_fail;
+
+	ret = at91_usart_gpio_setup(pdev);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	controller->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP | SPI_CS_HIGH;
+	controller->dev.of_node = pdev->dev.parent->of_node;
+	controller->bits_per_word_mask = SPI_BPW_MASK(8);
+	controller->setup = at91_usart_spi_setup;
+	controller->flags = SPI_MASTER_MUST_RX | SPI_MASTER_MUST_TX;
+	controller->transfer_one = at91_usart_spi_transfer_one;
+	controller->prepare_message = at91_usart_spi_prepare_message;
+	controller->unprepare_message = at91_usart_spi_unprepare_message;
+	controller->cleanup = at91_usart_spi_cleanup;
+	controller->max_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MIN_CLK_DIV);
+	controller->min_speed_hz = DIV_ROUND_UP(clk_get_rate(clk),
+						US_MAX_CLK_DIV);
+	platform_set_drvdata(pdev, controller);
+
+	aus = spi_master_get_devdata(controller);
+
+	aus->dev = &pdev->dev;
+	aus->regs = devm_ioremap_resource(&pdev->dev, regs);
+	if (IS_ERR(aus->regs)) {
+		ret = PTR_ERR(aus->regs);
+		goto at91_usart_spi_probe_fail;
+	}
+
+	aus->irq = irq;
+	aus->clk = clk;
+
+	ret = devm_request_irq(&pdev->dev, irq, at91_usart_spi_interrupt, 0,
+			       dev_name(&pdev->dev), controller);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		goto at91_usart_spi_probe_fail;
+
+	aus->spi_clk = clk_get_rate(clk);
+	at91_usart_spi_init(aus);
+
+	spin_lock_init(&aus->lock);
+	ret = devm_spi_register_master(&pdev->dev, controller);
+	if (ret)
+		goto fail_register_master;
+
+	dev_info(&pdev->dev,
+		 "Atmel USART SPI Controller version 0x%x at 0x%08x (irq %d)\n",
+		 at91_usart_spi_readl(aus, VERSION),
+		 regs->start, irq);
+
+	return 0;
+
+fail_register_master:
+	clk_disable_unprepare(clk);
+at91_usart_spi_probe_fail:
+	spi_master_put(controller);
+	return ret;
+}
+
+static int at91_usart_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct at91_usart_spi *aus = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(aus->clk);
+
+	return 0;
+}
+
+static const struct of_device_id at91_usart_spi_dt_ids[] = {
+	{ .compatible = "microchip,sama5d3-usart-spi"},
+	{ .compatible = "microchip,sama5d4-usart-spi"},
+	{ .compatible = "microchip,at91sam9g45-usart-spi"},
+	{ /* sentinel */}
+};
+
+MODULE_DEVICE_TABLE(of, at91_usart_spi_dt_ids);
+
+static struct platform_driver at91_usart_spi_driver = {
+	.driver = {
+		.name = "at91_usart_spi",
+	},
+	.probe = at91_usart_spi_probe,
+	.remove = at91_usart_spi_remove,
+};
+
+module_platform_driver(at91_usart_spi_driver);
+
+MODULE_DESCRIPTION("Microchip AT91 USART SPI Controller driver");
+MODULE_AUTHOR("Radu Pirea <radu.pirea@microchip.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:at91_usart_spi");
-- 
2.17.1

^ permalink raw reply related

* [PATCH v5 6/6] tty/serial: atmel: changed the driver to work under at91-usart mfd
From: Radu Pirea @ 2018-06-04 16:59 UTC (permalink / raw)
  To: broonie, nicolas.ferre, alexandre.belloni, lee.jones,
	richard.genoud, robh+dt, mark.rutland, gregkh
  Cc: linux-spi, linux-arm-kernel, linux-kernel, devicetree,
	linux-serial, Radu Pirea
In-Reply-To: <20180604165943.31381-1-radu.pirea@microchip.com>

This patch modifies the place where resources and device tree properties
are searched.

Signed-off-by: Radu Pirea <radu.pirea@microchip.com>
---
 drivers/tty/serial/Kconfig        |  1 +
 drivers/tty/serial/atmel_serial.c | 41 ++++++++++++++++++-------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 3682fd3e960c..25e55332f8b1 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -119,6 +119,7 @@ config SERIAL_ATMEL
 	depends on ARCH_AT91 || COMPILE_TEST
 	select SERIAL_CORE
 	select SERIAL_MCTRL_GPIO if GPIOLIB
+	select MFD_AT91_USART
 	help
 	  This enables the driver for the on-chip UARTs of the Atmel
 	  AT91 processors.
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index df46a9e88c34..5c74e03396ef 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -193,8 +193,8 @@ static struct console atmel_console;
 
 #if defined(CONFIG_OF)
 static const struct of_device_id atmel_serial_dt_ids[] = {
-	{ .compatible = "atmel,at91rm9200-usart" },
-	{ .compatible = "atmel,at91sam9260-usart" },
+	{ .compatible = "atmel,at91rm9200-usart-serial" },
+	{ .compatible = "atmel,at91sam9260-usart-serial" },
 	{ /* sentinel */ }
 };
 #endif
@@ -915,6 +915,7 @@ static void atmel_tx_dma(struct uart_port *port)
 static int atmel_prepare_tx_dma(struct uart_port *port)
 {
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	struct device *mfd_dev = port->dev->parent;
 	dma_cap_mask_t		mask;
 	struct dma_slave_config config;
 	int ret, nent;
@@ -922,7 +923,7 @@ static int atmel_prepare_tx_dma(struct uart_port *port)
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_SLAVE, mask);
 
-	atmel_port->chan_tx = dma_request_slave_channel(port->dev, "tx");
+	atmel_port->chan_tx = dma_request_slave_channel(mfd_dev, "tx");
 	if (atmel_port->chan_tx == NULL)
 		goto chan_err;
 	dev_info(port->dev, "using %s for tx DMA transfers\n",
@@ -1093,6 +1094,7 @@ static void atmel_rx_from_dma(struct uart_port *port)
 static int atmel_prepare_rx_dma(struct uart_port *port)
 {
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	struct device *mfd_dev = port->dev->parent;
 	struct dma_async_tx_descriptor *desc;
 	dma_cap_mask_t		mask;
 	struct dma_slave_config config;
@@ -1104,7 +1106,7 @@ static int atmel_prepare_rx_dma(struct uart_port *port)
 	dma_cap_zero(mask);
 	dma_cap_set(DMA_CYCLIC, mask);
 
-	atmel_port->chan_rx = dma_request_slave_channel(port->dev, "rx");
+	atmel_port->chan_rx = dma_request_slave_channel(mfd_dev, "rx");
 	if (atmel_port->chan_rx == NULL)
 		goto chan_err;
 	dev_info(port->dev, "using %s for rx DMA transfers\n",
@@ -1631,7 +1633,7 @@ static void atmel_tasklet_tx_func(unsigned long data)
 static void atmel_init_property(struct atmel_uart_port *atmel_port,
 				struct platform_device *pdev)
 {
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 
 	/* DMA/PDC usage specification */
 	if (of_property_read_bool(np, "atmel,use-dma-rx")) {
@@ -2222,8 +2224,8 @@ static const char *atmel_type(struct uart_port *port)
  */
 static void atmel_release_port(struct uart_port *port)
 {
-	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	struct platform_device *mpdev = to_platform_device(port->dev->parent);
+	int size = resource_size(mpdev->resource);
 
 	release_mem_region(port->mapbase, size);
 
@@ -2238,8 +2240,8 @@ static void atmel_release_port(struct uart_port *port)
  */
 static int atmel_request_port(struct uart_port *port)
 {
-	struct platform_device *pdev = to_platform_device(port->dev);
-	int size = pdev->resource[0].end - pdev->resource[0].start + 1;
+	struct platform_device *mpdev = to_platform_device(port->dev->parent);
+	int size = resource_size(mpdev->resource);
 
 	if (!request_mem_region(port->mapbase, size, "atmel_serial"))
 		return -EBUSY;
@@ -2341,27 +2343,28 @@ static int atmel_init_port(struct atmel_uart_port *atmel_port,
 {
 	int ret;
 	struct uart_port *port = &atmel_port->uart;
+	struct platform_device *mpdev = to_platform_device(pdev->dev.parent);
 
 	atmel_init_property(atmel_port, pdev);
 	atmel_set_ops(port);
 
-	uart_get_rs485_mode(&pdev->dev, &port->rs485);
+	uart_get_rs485_mode(&mpdev->dev, &port->rs485);
 
 	port->iotype		= UPIO_MEM;
 	port->flags		= UPF_BOOT_AUTOCONF | UPF_IOREMAP;
 	port->ops		= &atmel_pops;
 	port->fifosize		= 1;
 	port->dev		= &pdev->dev;
-	port->mapbase	= pdev->resource[0].start;
-	port->irq	= pdev->resource[1].start;
+	port->mapbase		= mpdev->resource[0].start;
+	port->irq		= mpdev->resource[1].start;
 	port->rs485_config	= atmel_config_rs485;
-	port->membase	= NULL;
+	port->membase		= NULL;
 
 	memset(&atmel_port->rx_ring, 0, sizeof(atmel_port->rx_ring));
 
 	/* for console, the clock could already be configured */
 	if (!atmel_port->clk) {
-		atmel_port->clk = clk_get(&pdev->dev, "usart");
+		atmel_port->clk = clk_get(&mpdev->dev, "usart");
 		if (IS_ERR(atmel_port->clk)) {
 			ret = PTR_ERR(atmel_port->clk);
 			atmel_port->clk = NULL;
@@ -2652,11 +2655,13 @@ static int atmel_serial_resume(struct platform_device *pdev)
 static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 				     struct platform_device *pdev)
 {
+	struct device *dev = pdev->dev.parent;
+
 	atmel_port->fifo_size = 0;
 	atmel_port->rts_low = 0;
 	atmel_port->rts_high = 0;
 
-	if (of_property_read_u32(pdev->dev.of_node,
+	if (of_property_read_u32(dev->of_node,
 				 "atmel,fifo-size",
 				 &atmel_port->fifo_size))
 		return;
@@ -2694,13 +2699,15 @@ static void atmel_serial_probe_fifos(struct atmel_uart_port *atmel_port,
 static int atmel_serial_probe(struct platform_device *pdev)
 {
 	struct atmel_uart_port *atmel_port;
-	struct device_node *np = pdev->dev.of_node;
+	struct device_node *np = pdev->dev.parent->of_node;
 	void *data;
 	int ret = -ENODEV;
 	bool rs485_enabled;
 
 	BUILD_BUG_ON(ATMEL_SERIAL_RINGSIZE & (ATMEL_SERIAL_RINGSIZE - 1));
 
+	pdev->dev.of_node = np;
+
 	ret = of_alias_get_id(np, "serial");
 	if (ret < 0)
 		/* port id not found in platform data nor device-tree aliases:
@@ -2845,7 +2852,7 @@ static struct platform_driver atmel_serial_driver = {
 	.suspend	= atmel_serial_suspend,
 	.resume		= atmel_serial_resume,
 	.driver		= {
-		.name			= "atmel_usart",
+		.name			= "atmel_usart_serial",
 		.of_match_table		= of_match_ptr(atmel_serial_dt_ids),
 	},
 };
-- 
2.17.1

^ permalink raw reply related

* Re: [PATCH 4/8] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Matwey V. Kornilov @ 2018-06-04 17:40 UTC (permalink / raw)
  To: Giulio Benetti, Greg Kroah-Hartman
  Cc: Jiri Slaby, Andy Shevchenko, Kees Cook, Matthias Brugger,
	Allen Pais, Sean Young, Ed Blake, Stefan Potyra, Philipp Zabel,
	Joshua Scott, Vignesh R, Rolf Evers-Fischer, Aaron Sierra,
	Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <20180601124021.102970-5-giulio.benetti@micronovasrl.com>

01.06.2018 15:40, Giulio Benetti пишет:
> Some 8250 ports only have TEMT interrupt, so current implementation
> can't work for ports without it. The only chance to make it work is to
> loop-read on LSR register.
> 
> With NO TEMT interrupt check if both TEMT and THRE are set looping on
> LSR register.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  drivers/tty/serial/8250/8250.h      |  2 +-
>  drivers/tty/serial/8250/8250_dw.c   |  2 +-
>  drivers/tty/serial/8250/8250_omap.c |  2 +-
>  drivers/tty/serial/8250/8250_port.c | 26 ++++++++++++++++++--------
>  include/linux/serial_8250.h         |  1 +
>  5 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index ebfb0bd5bef5..5c6ae5f69432 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -136,7 +136,7 @@ void serial8250_rpm_put(struct uart_8250_port *p);
>  void serial8250_rpm_get_tx(struct uart_8250_port *p);
>  void serial8250_rpm_put_tx(struct uart_8250_port *p);
>  
> -int serial8250_em485_init(struct uart_8250_port *p);
> +int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr);
>  void serial8250_em485_destroy(struct uart_8250_port *p);
>  
>  static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 0f8b4da03d4e..888280ff5451 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -330,7 +330,7 @@ static int dw8250_rs485_config(struct uart_port *p,
>  	 * are idempotent
>  	 */
>  	if (rs485->flags & SER_RS485_ENABLED) {
> -		int ret = serial8250_em485_init(up);
> +		int ret = serial8250_em485_init(up, false);
>  
>  		if (ret) {
>  			rs485->flags &= ~SER_RS485_ENABLED;
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 624b501fd253..ab483c8b30c8 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -725,7 +725,7 @@ static int omap_8250_rs485_config(struct uart_port *port,
>  	 * are idempotent
>  	 */
>  	if (rs485->flags & SER_RS485_ENABLED) {
> -		int ret = serial8250_em485_init(up);
> +		int ret = serial8250_em485_init(up, true);
>  
>  		if (ret) {
>  			rs485->flags &= ~SER_RS485_ENABLED;
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 95833cbc4338..e14badbbf181 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -599,15 +599,16 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
>  /**
>   *	serial8250_em485_init() - put uart_8250_port into rs485 emulating
>   *	@p:	uart_8250_port port instance
> + *	@p:	bool specify if 8250 port has TEMT interrupt or not
>   *
>   *	The function is used to start rs485 software emulating on the
>   *	&struct uart_8250_port* @p. Namely, RTS is switched before/after
>   *	transmission. The function is idempotent, so it is safe to call it
>   *	multiple times.
>   *
> - *	The caller MUST enable interrupt on empty shift register before
> - *	calling serial8250_em485_init(). This interrupt is not a part of
> - *	8250 standard, but implementation defined.
> + *	If has_temt_isr is passed as true, the caller MUST enable interrupt
> + *	on empty shift register before calling serial8250_em485_init().
> + *	This interrupt is not a part of	8250 standard, but implementation defined.
>   *
>   *	The function is supposed to be called from .rs485_config callback
>   *	or from any other callback protected with p->port.lock spinlock.
> @@ -616,7 +617,7 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
>   *
>   *	Return 0 - success, -errno - otherwise
>   */
> -int serial8250_em485_init(struct uart_8250_port *p)
> +int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr)
>  {
>  	if (p->em485)
>  		return 0;
> @@ -633,6 +634,7 @@ int serial8250_em485_init(struct uart_8250_port *p)
>  	p->em485->start_tx_timer.function = &serial8250_em485_handle_start_tx;
>  	p->em485->port = p;
>  	p->em485->active_timer = NULL;
> +	p->em485->has_temt_isr = has_temt_isr;
>  	serial8250_em485_rts_after_send(p);
>  
>  	return 0;
> @@ -1517,11 +1519,19 @@ static inline void __stop_tx(struct uart_8250_port *p)
>  		/*
>  		 * To provide required timeing and allow FIFO transfer,
>  		 * __stop_tx_rs485() must be called only when both FIFO and
> -		 * shift register are empty. It is for device driver to enable
> -		 * interrupt on TEMT.
> +		 * shift register are empty. If 8250 port supports it,
> +		 * it is for device driver to enable interrupt on TEMT.
> +		 * Otherwise must loop-read until TEMT and THRE flags are set.
>  		 */
> -		if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
> -			return;
> +		if (em485->has_temt_isr) {
> +			if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
> +				return;
> +		} else {
> +			while ((lsr & BOTH_EMPTY) != BOTH_EMPTY) {
> +				lsr = serial_in(p, UART_LSR);
> +				cpu_relax();
> +			}

What happens if TEMP never be empty after interruption occurring?

> +		}
>  
>  		em485->active_timer = NULL;
>  
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index a27ef5f56431..9b13cf59726b 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -83,6 +83,7 @@ struct uart_8250_em485 {
>  	struct hrtimer		start_tx_timer; /* "rs485 start tx" timer */
>  	struct hrtimer		stop_tx_timer;  /* "rs485 stop tx" timer */
>  	struct hrtimer		*active_timer;  /* pointer to active timer */
> +	bool			has_temt_isr;	/* say if 8250 has TEMT interrupt or no */
>  	struct uart_8250_port	*port;          /* for hrtimer callbacks */
>  };
>  
> 

^ permalink raw reply

* Re: [PATCH 4/8] serial: 8250: Handle case port doesn't have TEMT interrupt using em485.
From: Giulio Benetti @ 2018-06-04 18:50 UTC (permalink / raw)
  To: Matwey V. Kornilov, Greg Kroah-Hartman
  Cc: Jiri Slaby, Andy Shevchenko, Kees Cook, Matthias Brugger,
	Allen Pais, Sean Young, Ed Blake, Stefan Potyra, Philipp Zabel,
	Joshua Scott, Vignesh R, Rolf Evers-Fischer, Aaron Sierra,
	Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <b14d4970-f14d-d59f-2072-b7d5704dd842@gmail.com>

Il 04/06/2018 19:40, Matwey V. Kornilov ha scritto:
> 01.06.2018 15:40, Giulio Benetti пишет:
>> Some 8250 ports only have TEMT interrupt, so current implementation
>> can't work for ports without it. The only chance to make it work is to
>> loop-read on LSR register.
>>
>> With NO TEMT interrupt check if both TEMT and THRE are set looping on
>> LSR register.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   drivers/tty/serial/8250/8250.h      |  2 +-
>>   drivers/tty/serial/8250/8250_dw.c   |  2 +-
>>   drivers/tty/serial/8250/8250_omap.c |  2 +-
>>   drivers/tty/serial/8250/8250_port.c | 26 ++++++++++++++++++--------
>>   include/linux/serial_8250.h         |  1 +
>>   5 files changed, 22 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
>> index ebfb0bd5bef5..5c6ae5f69432 100644
>> --- a/drivers/tty/serial/8250/8250.h
>> +++ b/drivers/tty/serial/8250/8250.h
>> @@ -136,7 +136,7 @@ void serial8250_rpm_put(struct uart_8250_port *p);
>>   void serial8250_rpm_get_tx(struct uart_8250_port *p);
>>   void serial8250_rpm_put_tx(struct uart_8250_port *p);
>>   
>> -int serial8250_em485_init(struct uart_8250_port *p);
>> +int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr);
>>   void serial8250_em485_destroy(struct uart_8250_port *p);
>>   
>>   static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
>> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
>> index 0f8b4da03d4e..888280ff5451 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -330,7 +330,7 @@ static int dw8250_rs485_config(struct uart_port *p,
>>   	 * are idempotent
>>   	 */
>>   	if (rs485->flags & SER_RS485_ENABLED) {
>> -		int ret = serial8250_em485_init(up);
>> +		int ret = serial8250_em485_init(up, false);
>>   
>>   		if (ret) {
>>   			rs485->flags &= ~SER_RS485_ENABLED;
>> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
>> index 624b501fd253..ab483c8b30c8 100644
>> --- a/drivers/tty/serial/8250/8250_omap.c
>> +++ b/drivers/tty/serial/8250/8250_omap.c
>> @@ -725,7 +725,7 @@ static int omap_8250_rs485_config(struct uart_port *port,
>>   	 * are idempotent
>>   	 */
>>   	if (rs485->flags & SER_RS485_ENABLED) {
>> -		int ret = serial8250_em485_init(up);
>> +		int ret = serial8250_em485_init(up, true);
>>   
>>   		if (ret) {
>>   			rs485->flags &= ~SER_RS485_ENABLED;
>> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
>> index 95833cbc4338..e14badbbf181 100644
>> --- a/drivers/tty/serial/8250/8250_port.c
>> +++ b/drivers/tty/serial/8250/8250_port.c
>> @@ -599,15 +599,16 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
>>   /**
>>    *	serial8250_em485_init() - put uart_8250_port into rs485 emulating
>>    *	@p:	uart_8250_port port instance
>> + *	@p:	bool specify if 8250 port has TEMT interrupt or not
>>    *
>>    *	The function is used to start rs485 software emulating on the
>>    *	&struct uart_8250_port* @p. Namely, RTS is switched before/after
>>    *	transmission. The function is idempotent, so it is safe to call it
>>    *	multiple times.
>>    *
>> - *	The caller MUST enable interrupt on empty shift register before
>> - *	calling serial8250_em485_init(). This interrupt is not a part of
>> - *	8250 standard, but implementation defined.
>> + *	If has_temt_isr is passed as true, the caller MUST enable interrupt
>> + *	on empty shift register before calling serial8250_em485_init().
>> + *	This interrupt is not a part of	8250 standard, but implementation defined.
>>    *
>>    *	The function is supposed to be called from .rs485_config callback
>>    *	or from any other callback protected with p->port.lock spinlock.
>> @@ -616,7 +617,7 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put);
>>    *
>>    *	Return 0 - success, -errno - otherwise
>>    */
>> -int serial8250_em485_init(struct uart_8250_port *p)
>> +int serial8250_em485_init(struct uart_8250_port *p, bool has_temt_isr)
>>   {
>>   	if (p->em485)
>>   		return 0;
>> @@ -633,6 +634,7 @@ int serial8250_em485_init(struct uart_8250_port *p)
>>   	p->em485->start_tx_timer.function = &serial8250_em485_handle_start_tx;
>>   	p->em485->port = p;
>>   	p->em485->active_timer = NULL;
>> +	p->em485->has_temt_isr = has_temt_isr;
>>   	serial8250_em485_rts_after_send(p);
>>   
>>   	return 0;
>> @@ -1517,11 +1519,19 @@ static inline void __stop_tx(struct uart_8250_port *p)
>>   		/*
>>   		 * To provide required timeing and allow FIFO transfer,
>>   		 * __stop_tx_rs485() must be called only when both FIFO and
>> -		 * shift register are empty. It is for device driver to enable
>> -		 * interrupt on TEMT.
>> +		 * shift register are empty. If 8250 port supports it,
>> +		 * it is for device driver to enable interrupt on TEMT.
>> +		 * Otherwise must loop-read until TEMT and THRE flags are set.
>>   		 */
>> -		if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
>> -			return;
>> +		if (em485->has_temt_isr) {
>> +			if ((lsr & BOTH_EMPTY) != BOTH_EMPTY)
>> +				return;
>> +		} else {
>> +			while ((lsr & BOTH_EMPTY) != BOTH_EMPTY) {
>> +				lsr = serial_in(p, UART_LSR);
>> +				cpu_relax();
>> +			}
> 
> What happens if TEMP never be empty after interruption occurring?
> 

I've added the field has_temt_isr to identify if peripheral provides or 
not TEMT interrupt. In DW case, differentely from OMAP case, there is 
not TEMT interrupt, at least on Datasheet I've found.
At this time I don't have access to latest Datasheet.

Specifying has_temt_isr = false during serial8250_em485_init(),
I assume TEMT interrupt is not available and also is not enabled.

IMHO the only possible case to loop inside there is if shift register is 
costantly filled, but soon or late it will get empty(hope I didn't miss 
something).

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

^ permalink raw reply

* Re: [PATCH 1/2] serial: 8250: enable SERIAL_MCTRL_GPIO by default.
From: Giulio Benetti @ 2018-06-04 18:57 UTC (permalink / raw)
  To: Andy Shevchenko, Greg Kroah-Hartman
  Cc: Jiri Slaby, Kees Cook, Matthias Brugger, Allen Pais, Sean Young,
	Ed Blake, Stefan Potyra, Philipp Zabel, Joshua Scott, Vignesh R,
	Rolf Evers-Fischer, Aaron Sierra, Rafael Gago, Joel Stanley,
	Sean Wang, linux-serial, linux-kernel, Yegor Yefremov
In-Reply-To: <b850a3cf565a24414d3290f6699f5a040bf61223.camel@linux.intel.com>

Il 04/06/2018 13:49, Andy Shevchenko ha scritto:
> On Fri, 2018-06-01 at 16:11 +0200, Giulio Benetti wrote:
>> It can be useful to override 8250 mctrl lines with gpios, for rts on
>> rs485 for example, when rts is not mapped correctly to HW RTS pin.
>>
>> Enable SERIAL_MCTRL_GPIO by default.
>>
> 
> Unfortunately NAK, see
> 
> commit 5db4f7f80d165fc9725f356e99feec409e446baa
> Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Date:   Tue Aug 16 15:06:54 2016 +0300
> 
>      Revert "tty/serial/8250: use mctrl_gpio helpers"
> 
> for the details.
> 
> I would love to see a solution that will satisfy everyone, though I have
> only means to test proposals for now.

Thanks for pointing me that.
I would try to solve serial breakage on intel with already extisting 
patches dropping this one.
I'm going to try.

I can't understand if it's enough using qemu x86 to reproduce the bug.
If so I'm going to debug and check what makes driver to fail.

Do you think it makes sense? Would it be accepted after bug fixing?

Thanks
Best regards

-- 
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642

> 
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   drivers/tty/serial/8250/Kconfig | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/tty/serial/8250/Kconfig
>> b/drivers/tty/serial/8250/Kconfig
>> index f005eaf8bc57..c7992b94fece 100644
>> --- a/drivers/tty/serial/8250/Kconfig
>> +++ b/drivers/tty/serial/8250/Kconfig
>> @@ -7,6 +7,7 @@ config SERIAL_8250
>>   	tristate "8250/16550 and compatible serial support"
>>   	depends on !S390
>>   	select SERIAL_CORE
>> +	select SERIAL_MCTRL_GPIO if GPIOLIB
>>   	---help---
>>   	  This selects whether you want to include the driver for the
>> standard
>>   	  serial ports.  The standard answer is Y.  People who might
>> say N
> 

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox