* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Andy Shevchenko @ 2018-06-06 13:11 UTC (permalink / raw)
To: Giulio Benetti
Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
open list
In-Reply-To: <0bc400b1-6178-2021-c9a3-3190d1a1de32@micronovasrl.com>
On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
> > On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
> > > em485 gets lost during
> > >
> > > Copy em485 to final uart port.
> > >
> >
> > Is it needed at all?
> >
> > The individual driver decides either to use software emulation (and
> > calls explicitly serial8250_em485_init() for that) or do HW assisted
> > stuff.
>
> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config()
> against local struct uart_8250_port uart = {};
> Inside serial8250_register_8250_port() not all uart fields are
> copied(em485 too).
> So after probe, em485 is NULL.
>
> Another way could be to call dw8250_rs485_config() against real uart
> port, after calling serial8250_register_8250_port(),
> would it make sense?
Look at OMAP case closely. They have a callback to configure RS485 which
is called in uart_set_rs485_config() which is called whenever user
space does TIOCGRS485 IOCTL.
So, it's completely driven by user space which makes sense by my
opinion.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH 6/8] serial: 8250: Copy mctrl when register port.
From: Aaron Sierra @ 2018-06-06 14:31 UTC (permalink / raw)
To: Giulio Benetti
Cc: Greg Kroah-Hartman, 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,
Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <20180601124021.102970-7-giulio.benetti@micronovasrl.com>
----- Original Message -----
> From: "Giulio Benetti" <giulio.benetti@micronovasrl.com>
> Sent: Friday, June 1, 2018 7:40:19 AM
> RS485 can modify mctrl on startup, especially when RTS_AFTER_SEND is on
> TIOCM_RTS is set, then need to keep it set when registering port.
>
> Copy mctrl to new port too.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> drivers/tty/serial/8250/8250_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/8250/8250_core.c
> b/drivers/tty/serial/8250/8250_core.c
> index c8c2b260c681..c8e62fbd6570 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -993,6 +993,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
> uart->port.unthrottle = up->port.unthrottle;
> uart->port.rs485_config = up->port.rs485_config;
> uart->port.rs485 = up->port.rs485;
> + uart->port.mctrl = up->port.mctrl;
Hi Guilio,
I ran into this same thing about six months ago, but I was able to
accomplish what I needed by assigning a set_mctrl() function in my
port definition. Perhaps that would be enough for your case, too?
You should see a little lower in this file that set_mctrl is copied
to the new port.
-Aaron
> uart->dma = up->dma;
> uart->em485 = up->em485;
>
> --
> 2.17.0
^ permalink raw reply
* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Giulio Benetti @ 2018-06-06 14:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: matwey.kornilov, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
open list
In-Reply-To: <2abe2137e699e5ae3100b97316da469f6d1c9bb9.camel@linux.intel.com>
Hi Andy,
Il 06/06/2018 15:11, Andy Shevchenko ha scritto:
> On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
>> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
>>> On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>>>> em485 gets lost during
>>>>
>>>> Copy em485 to final uart port.
>>>>
>>>
>>> Is it needed at all?
>>>
>>> The individual driver decides either to use software emulation (and
>>> calls explicitly serial8250_em485_init() for that) or do HW assisted
>>> stuff.
>>
>> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config()
>> against local struct uart_8250_port uart = {};
>> Inside serial8250_register_8250_port() not all uart fields are
>> copied(em485 too).
>> So after probe, em485 is NULL.
>>
>> Another way could be to call dw8250_rs485_config() against real uart
>> port, after calling serial8250_register_8250_port(),
>> would it make sense?
>
> Look at OMAP case closely. They have a callback to configure RS485 which
> is called in uart_set_rs485_config() which is called whenever user
> space does TIOCGRS485 IOCTL.
>
> So, it's completely driven by user space which makes sense by my
> opinion.
This is my problem, being driven only by userspace means that until you
don't open ttyS* from there, dw8250_rs485_config() won't be called and
rs485 won't be configured.
In the case you have RTS_AFTER_SEND and
"linux,rs485-enabled-at-boot-time" in dts, you need to handle RTS correctly.
Without calling:
- uart_get_rs485_mode() to collect dts properties
and
- dw8250_rs485_config() to configure according properties
the result is RTS NOT asserted, then pin is HIGH, meaning that rs485
transceiver will be in tx mode until port is open.
Other drivers I've watched to for insipiration are:
- atmel_serial.c
- fsl_lpuart.c
- imx.c
etc.
everything containing uart_get_rs485_mode().
The main difficulty to understand this without a scope is that on
rs485.txt documentation the property "rs485-rts-active-low" is described as:
"drive RTS low when sending (default is high)"
Instead it should report:
"de-assert RTS when sending(pin high), default is asserted(pin low)"
Maybe I should send a patch for that also.
I ended to this conclusion because on every check of RTS_AFTER_SEND and
SER_RS485_RTS_ON_SEND RTS is treated this way.
Try to take a look at uart_port_dtr_rts() in serial_core.c for example.
Or serial8250_em485_rts_after_send() in 8250_port.c
What do you think?
--
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 6/8] serial: 8250: Copy mctrl when register port.
From: Giulio Benetti @ 2018-06-06 14:44 UTC (permalink / raw)
To: Aaron Sierra
Cc: Greg Kroah-Hartman, 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,
Rafael Gago, Joel Stanley, Sean Wang, linux-serial, linux-kernel
In-Reply-To: <1533624932.66943.1528295462220.JavaMail.zimbra@xes-inc.com>
Hi Aaron,
Il 06/06/2018 16:31, Aaron Sierra ha scritto:
> ----- Original Message -----
>> From: "Giulio Benetti" <giulio.benetti@micronovasrl.com>
>> Sent: Friday, June 1, 2018 7:40:19 AM
>
>> RS485 can modify mctrl on startup, especially when RTS_AFTER_SEND is on
>> TIOCM_RTS is set, then need to keep it set when registering port.
>>
>> Copy mctrl to new port too.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>> drivers/tty/serial/8250/8250_core.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_core.c
>> b/drivers/tty/serial/8250/8250_core.c
>> index c8c2b260c681..c8e62fbd6570 100644
>> --- a/drivers/tty/serial/8250/8250_core.c
>> +++ b/drivers/tty/serial/8250/8250_core.c
>> @@ -993,6 +993,7 @@ int serial8250_register_8250_port(struct uart_8250_port *up)
>> uart->port.unthrottle = up->port.unthrottle;
>> uart->port.rs485_config = up->port.rs485_config;
>> uart->port.rs485 = up->port.rs485;
>> + uart->port.mctrl = up->port.mctrl;
>
> Hi Guilio,
>
> I ran into this same thing about six months ago, but I was able to
> accomplish what I needed by assigning a set_mctrl() function in my
> port definition. Perhaps that would be enough for your case, too?
Thanks for pointing me.
But wouldn't it be better copying mctrl?
In any case, serial8250_register_8250_port() will call:
- uart_add_one_port()
- uart_configure_port()
- port->ops->set_mctrl(port, mctrl);
So it would be a double call IMHO.
Are there any drawbacks on doing what I'm saying?
Maybe I'm missing something.
> You should see a little lower in this file that set_mctrl is copied
> to the new port. > -Aaron
>
>> uart->dma = up->dma;
>> uart->em485 = up->em485;
>>
>> --
>> 2.17.0
--
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/4] serial: 8250_dw: add em485 support
From: Andy Shevchenko @ 2018-06-06 16:51 UTC (permalink / raw)
To: Giulio Benetti
Cc: Matwey V. Kornilov, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Ed Blake, Joshua Scott, open list:SERIAL DRIVERS,
open list
In-Reply-To: <20180606095156.72628-4-giulio.benetti@micronovasrl.com>
On Wed, Jun 6, 2018 at 12:51 PM, Giulio Benetti
<giulio.benetti@micronovasrl.com> wrote:
> Need to use rs485 transceiver so let's use existing em485 485 emulation
> layer on top of 8250.
>
> Add rs485_config callback to port.
Besides the fact the series lacks of cover letter, I think it should
be postponed until we get a clear understanding how RS485 is supposed
to be initialized and what exact problems you are trying to address.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
> drivers/tty/serial/8250/8250_dw.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index 6fcdb90f616a..45366e6e5411 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -314,6 +314,36 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
> serial8250_do_set_ldisc(p, termios);
> }
>
> +static int dw8250_rs485_config(struct uart_port *p,
> + struct serial_rs485 *rs485)
> +{
> + struct uart_8250_port *up = up_to_u8250p(p);
> +
> + /* 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);
> +
> + p->rs485 = *rs485;
> +
> + /*
> + * Both serial8250_em485_init and serial8250_em485_destroy
> + * are idempotent
> + */
> + if (rs485->flags & SER_RS485_ENABLED) {
> + int ret = serial8250_em485_init(up);
> +
> + if (ret) {
> + rs485->flags &= ~SER_RS485_ENABLED;
> + p->rs485.flags &= ~SER_RS485_ENABLED;
> + }
> + return ret;
> + }
> +
> + serial8250_em485_destroy(up);
> +
> + return 0;
> +}
> +
> /*
> * dw8250_fallback_dma_filter will prevent the UART from getting just any free
> * channel on platforms that have DMA engines, but don't have any channels
> @@ -449,6 +479,7 @@ static int dw8250_probe(struct platform_device *pdev)
> p->serial_out = dw8250_serial_out;
> p->set_ldisc = dw8250_set_ldisc;
> p->set_termios = dw8250_set_termios;
> + p->rs485_config = dw8250_rs485_config;
>
> p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
> if (!p->membase)
> --
> 2.17.1
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH] serial: 8250_dw: 8250_dw need to depend on COMMON_CLK
From: Corentin Labbe @ 2018-06-06 18:47 UTC (permalink / raw)
To: gregkh, jslaby; +Cc: linux-kernel, linux-serial, Corentin Labbe
This patch fix the following build error on M68K:
drivers/tty/serial/8250/8250_dw.o: In function `dw8250_set_termios':
8250_dw.c:(.text+0x50c): undefined reference to `clk_round_rate'
8250_dw.c:(.text+0x594): undefined reference to `clk_set_rate'
So 8250_dw need to depend on COMMON_CLK.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
drivers/tty/serial/8250/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index f005eaf8bc57..37d4ec5f3cc9 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -351,7 +351,7 @@ config SERIAL_8250_FSL
config SERIAL_8250_DW
tristate "Support for Synopsys DesignWare 8250 quirks"
- depends on SERIAL_8250
+ depends on SERIAL_8250 && COMMON_CLK
help
Selecting this option will enable handling of the extra features
present in the Synopsys DesignWare APB UART.
--
2.16.4
^ permalink raw reply related
* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Matwey V. Kornilov @ 2018-06-06 18:55 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Giulio Benetti, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
open list
In-Reply-To: <2abe2137e699e5ae3100b97316da469f6d1c9bb9.camel@linux.intel.com>
2018-06-06 16:11 GMT+03:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
>> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
>> > On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>> > > em485 gets lost during
>> > >
>> > > Copy em485 to final uart port.
>> > >
>> >
>> > Is it needed at all?
>> >
>> > The individual driver decides either to use software emulation (and
>> > calls explicitly serial8250_em485_init() for that) or do HW assisted
>> > stuff.
>>
>> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config()
>> against local struct uart_8250_port uart = {};
>> Inside serial8250_register_8250_port() not all uart fields are
>> copied(em485 too).
>> So after probe, em485 is NULL.
>>
>> Another way could be to call dw8250_rs485_config() against real uart
>> port, after calling serial8250_register_8250_port(),
>> would it make sense?
>
> Look at OMAP case closely. They have a callback to configure RS485 which
> is called in uart_set_rs485_config() which is called whenever user
> space does TIOCGRS485 IOCTL.
>
> So, it's completely driven by user space which makes sense by my
> opinion.
AFAIU, Giulio wants to add support for rs485-enabled-at-boot-time
device tree option (see bindings/serial/rs485.txt for reference).
I suppose it is only important for use-case when rs485 used as slave
(peripheral role).
>
> --
> Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Intel Finland Oy
--
With best regards,
Matwey V. Kornilov
^ permalink raw reply
* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Giulio Benetti @ 2018-06-06 19:15 UTC (permalink / raw)
To: Matwey V. Kornilov, Andy Shevchenko
Cc: Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger, Kees Cook,
Allen Pais, Sean Young, open list:SERIAL DRIVERS, open list
In-Reply-To: <CAJs94EZx2ZdEL14SCEBomEfXLRPN4FfOp4yE44kfpnE+5+Ze-g@mail.gmail.com>
Il 06/06/2018 20:55, Matwey V. Kornilov ha scritto:
> 2018-06-06 16:11 GMT+03:00 Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>> On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
>>> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
>>>> On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>>>>> em485 gets lost during
>>>>>
>>>>> Copy em485 to final uart port.
>>>>>
>>>>
>>>> Is it needed at all?
>>>>
>>>> The individual driver decides either to use software emulation (and
>>>> calls explicitly serial8250_em485_init() for that) or do HW assisted
>>>> stuff.
>>>
>>> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config()
>>> against local struct uart_8250_port uart = {};
>>> Inside serial8250_register_8250_port() not all uart fields are
>>> copied(em485 too).
>>> So after probe, em485 is NULL.
>>>
>>> Another way could be to call dw8250_rs485_config() against real uart
>>> port, after calling serial8250_register_8250_port(),
>>> would it make sense?
>>
>> Look at OMAP case closely. They have a callback to configure RS485 which
>> is called in uart_set_rs485_config() which is called whenever user
>> space does TIOCGRS485 IOCTL.
>>
>> So, it's completely driven by user space which makes sense by my
>> opinion.
>
> AFAIU, Giulio wants to add support for rs485-enabled-at-boot-time
> device tree option (see bindings/serial/rs485.txt for reference).
Yes, I want to add support for "rs485-enabled-at-boot-time" property,
maybe I had to write better commit log and a cover letter. Sorry.
> I suppose it is only important for use-case when rs485 used as slave
> (peripheral role).
It's important also for master, because RTS, if RTS_AFTER_SEND is set,
remains not asserted(trasnmission) until userspace opens that serial port.
Sorry again for not explaining myself well.
--
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/4] serial: 8250_dw: add em485 support
From: Giulio Benetti @ 2018-06-06 19:16 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Matwey V. Kornilov, Andy Shevchenko, Greg Kroah-Hartman,
Jiri Slaby, Ed Blake, Joshua Scott, open list:SERIAL DRIVERS,
open list
In-Reply-To: <CAHp75VfgYmX5WiG5HpN7hLiW1CfhDYV2gya58OGYyBQwVGTY2A@mail.gmail.com>
Il 06/06/2018 18:51, Andy Shevchenko ha scritto:
> On Wed, Jun 6, 2018 at 12:51 PM, Giulio Benetti
> <giulio.benetti@micronovasrl.com> wrote:
>> Need to use rs485 transceiver so let's use existing em485 485 emulation
>> layer on top of 8250.
>>
>> Add rs485_config callback to port.
>
> Besides the fact the series lacks of cover letter, I think it should
Sorry for that, next patchsets will have cover-letters.
> be postponed until we get a clear understanding how RS485 is supposed
> to be initialized and what exact problems you are trying to address.
>
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>> drivers/tty/serial/8250/8250_dw.c | 31 +++++++++++++++++++++++++++++++
>> 1 file changed, 31 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
>> index 6fcdb90f616a..45366e6e5411 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -314,6 +314,36 @@ static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
>> serial8250_do_set_ldisc(p, termios);
>> }
>>
>> +static int dw8250_rs485_config(struct uart_port *p,
>> + struct serial_rs485 *rs485)
>> +{
>> + struct uart_8250_port *up = up_to_u8250p(p);
>> +
>> + /* 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);
>> +
>> + p->rs485 = *rs485;
>> +
>> + /*
>> + * Both serial8250_em485_init and serial8250_em485_destroy
>> + * are idempotent
>> + */
>> + if (rs485->flags & SER_RS485_ENABLED) {
>> + int ret = serial8250_em485_init(up);
>> +
>> + if (ret) {
>> + rs485->flags &= ~SER_RS485_ENABLED;
>> + p->rs485.flags &= ~SER_RS485_ENABLED;
>> + }
>> + return ret;
>> + }
>> +
>> + serial8250_em485_destroy(up);
>> +
>> + return 0;
>> +}
>> +
>> /*
>> * dw8250_fallback_dma_filter will prevent the UART from getting just any free
>> * channel on platforms that have DMA engines, but don't have any channels
>> @@ -449,6 +479,7 @@ static int dw8250_probe(struct platform_device *pdev)
>> p->serial_out = dw8250_serial_out;
>> p->set_ldisc = dw8250_set_ldisc;
>> p->set_termios = dw8250_set_termios;
>> + p->rs485_config = dw8250_rs485_config;
>>
>> p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
>> if (!p->membase)
>> --
>> 2.17.1
>>
>
>
>
--
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] uart: fix race between uart_put_char() and uart_shutdown()
From: Tycho Andersen @ 2018-06-06 21:42 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Greg Kroah-Hartman, Jiri Slaby, linux-serial, linux-kernel
In-Reply-To: <20180605035936.GA19642@mail.hallyn.com>
On Mon, Jun 04, 2018 at 10:59:36PM -0500, Serge E. Hallyn wrote:
> Quoting Tycho Andersen (tycho@tycho.ws):
> > Unfortunately, I don't have any insightful thoughts about how to test this.
> > Ideas are appreciated :)
>
> I wonder whether there is something we can do with qemu -serial pipe: ?
Good idea. I couldn't get tty_put_char() to trigger nicely, but I just
hard coded one to occur, so at least now I know that this doesn't
deadlock when called normally.
Another suggestion Serge gave off list was to write a kernel module
that implemented a driver. I'll see about doing that to see if I can
force the original crash.
> > Signed-off-by: Tycho Andersen <tycho@tycho.ws>
>
> Acked-by: Serge Hallyn <serge@hallyn.com>
Thanks!
Tycho
^ permalink raw reply
* Re: possible deadlock in console_unlock
From: Sergey Senozhatsky @ 2018-06-07 5:10 UTC (permalink / raw)
To: syzbot
Cc: linux-kernel, pmladek, rostedt, sergey.senozhatsky,
syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby, linux-serial,
Andrew Morton
In-Reply-To: <00000000000087008b056df8fbb3@google.com>
Cc-ing more people
Link: lkml.kernel.org/r/00000000000087008b056df8fbb3@google.com
On (06/06/18 06:17), syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: af6c5d5e01ad Merge branch 'for-4.18' of git://git.kernel.o..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=173d93ef800000
> kernel config: https://syzkaller.appspot.com/x/.config?x=12ff770540994680
> dashboard link: https://syzkaller.appspot.com/bug?extid=43e93968b964e369db0b
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> userspace arch: i386
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=16e00bb7800000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+43e93968b964e369db0b@syzkaller.appspotmail.com
Thanks a ton!
So tty ioctl is known to be printk-deadlock prone and we don't know
how to handle this in printk, as of now.
ioctl
tty_ioctl
tty_port->lock
printk
call_console_driver
console_driver
uart_port->lock
The problem is that call_console_driver->console_driver also can do
this thing
uart_port->lock
tty_wakeup
tty_port->lock
So we can have the following:
ioctl
tty_ioctl
tty_port->lock
printk
call_console_driver
console_driver
uart_port->lock
tty_wakeup
tty_port->lock << deadlock
But lockdep is complaining about another scenario:
> the existing dependency chain (in reverse order) is:
>
> -> #2 (&(&port->lock)->rlock){-.-.}:
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
> tty_port_tty_get+0x20/0x80 drivers/tty/tty_port.c:288
> tty_port_default_wakeup+0x15/0x40 drivers/tty/tty_port.c:47
> tty_port_tty_wakeup+0x5d/0x70 drivers/tty/tty_port.c:390
> uart_write_wakeup+0x44/0x60 drivers/tty/serial/serial_core.c:103
> serial8250_tx_chars+0x4be/0xb60
> drivers/tty/serial/8250/8250_port.c:1808
> serial8250_handle_irq.part.25+0x1ee/0x280
> drivers/tty/serial/8250/8250_port.c:1881
> serial8250_handle_irq drivers/tty/serial/8250/8250_port.c:1867
> [inline]
> serial8250_default_handle_irq+0xc8/0x150
> drivers/tty/serial/8250/8250_port.c:1897
> serial8250_interrupt+0xfa/0x1d0
> drivers/tty/serial/8250/8250_core.c:125
> __handle_irq_event_percpu+0x1c0/0xad0 kernel/irq/handle.c:149
> handle_irq_event_percpu+0x98/0x1c0 kernel/irq/handle.c:189
> handle_irq_event+0xa7/0x135 kernel/irq/handle.c:206
> handle_edge_irq+0x20f/0x870 kernel/irq/chip.c:791
> generic_handle_irq_desc include/linux/irqdesc.h:159 [inline]
> handle_irq+0x18c/0x2e7 arch/x86/kernel/irq_64.c:77
> do_IRQ+0x78/0x190 arch/x86/kernel/irq.c:245
> ret_from_intr+0x0/0x1e
> native_safe_halt+0x6/0x10 arch/x86/include/asm/irqflags.h:54
> arch_safe_halt arch/x86/include/asm/paravirt.h:94 [inline]
> default_idle+0xc2/0x440 arch/x86/kernel/process.c:500
> arch_cpu_idle+0x10/0x20 arch/x86/kernel/process.c:491
> default_idle_call+0x6d/0x90 kernel/sched/idle.c:93
> cpuidle_idle_call kernel/sched/idle.c:153 [inline]
> do_idle+0x395/0x560 kernel/sched/idle.c:262
> cpu_startup_entry+0x104/0x120 kernel/sched/idle.c:368
> start_secondary+0x42b/0x5c0 arch/x86/kernel/smpboot.c:265
> secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242
So this one is IRQ ==> uart_port->lock ==> tty_port->lock
> -> #1 (&port_lock_key){-.-.}:
> __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> _raw_spin_lock_irqsave+0x96/0xc0 kernel/locking/spinlock.c:152
> serial8250_console_write+0x8d5/0xb00
> drivers/tty/serial/8250/8250_port.c:3230
> univ8250_console_write+0x5f/0x70
> drivers/tty/serial/8250/8250_core.c:590
> call_console_drivers kernel/printk/printk.c:1718 [inline]
> console_unlock+0xac2/0x1100 kernel/printk/printk.c:2395
> vprintk_emit+0x6ad/0xdd0 kernel/printk/printk.c:1907
> vprintk_default+0x28/0x30 kernel/printk/printk.c:1947
> vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379
> printk+0x9e/0xba kernel/printk/printk.c:1980
> register_console+0x7e7/0xc00 kernel/printk/printk.c:2714
> univ8250_console_init+0x3f/0x4b
> drivers/tty/serial/8250/8250_core.c:685
> console_init+0x6d9/0xa38 kernel/printk/printk.c:2798
> start_kernel+0x608/0x92d init/main.c:661
> x86_64_start_reservations+0x29/0x2b arch/x86/kernel/head64.c:452
> x86_64_start_kernel+0x76/0x79 arch/x86/kernel/head64.c:433
> secondary_startup_64+0xa5/0xb0 arch/x86/kernel/head_64.S:242
This one is console_owner/console_sem ==> uart_port->lock
> -> #0 (console_owner){-...}:
> lock_acquire+0x1dc/0x520 kernel/locking/lockdep.c:3924
> console_lock_spinning_enable kernel/printk/printk.c:1581 [inline]
> console_unlock+0x5ef/0x1100 kernel/printk/printk.c:2392
> vprintk_emit+0x6ad/0xdd0 kernel/printk/printk.c:1907
> vprintk_default+0x28/0x30 kernel/printk/printk.c:1947
> vprintk_func+0x7a/0xe7 kernel/printk/printk_safe.c:379
> printk+0x9e/0xba kernel/printk/printk.c:1980
> fail_dump lib/fault-inject.c:44 [inline]
> should_fail+0x97a/0xbcd lib/fault-inject.c:149
> __should_failslab+0x124/0x180 mm/failslab.c:32
> should_failslab+0x9/0x14 mm/slab_common.c:1522
> slab_pre_alloc_hook mm/slab.h:423 [inline]
> slab_alloc mm/slab.c:3378 [inline]
> __do_kmalloc mm/slab.c:3716 [inline]
> __kmalloc+0x63/0x760 mm/slab.c:3727
> kmalloc include/linux/slab.h:517 [inline]
> tty_buffer_alloc drivers/tty/tty_buffer.c:170 [inline]
> __tty_buffer_request_room+0x2d2/0x7f0 drivers/tty/tty_buffer.c:268
> tty_insert_flip_string_fixed_flag+0x8d/0x1f0
> drivers/tty/tty_buffer.c:313
> tty_insert_flip_string include/linux/tty_flip.h:37 [inline]
> pty_write+0x12c/0x1f0 drivers/tty/pty.c:121
> tty_put_char+0x129/0x150 drivers/tty/tty_io.c:2865
> __process_echoes+0x4d9/0x8d0 drivers/tty/n_tty.c:703
> process_echoes+0xfc/0x170 drivers/tty/n_tty.c:781
> n_tty_set_termios+0xb56/0xe80 drivers/tty/n_tty.c:1837
> tty_set_termios+0x7a0/0xac0 drivers/tty/tty_ioctl.c:341
> set_termios+0x41e/0x7d0 drivers/tty/tty_ioctl.c:414
> tty_mode_ioctl+0x871/0xb50 drivers/tty/tty_ioctl.c:781
> n_tty_ioctl_helper+0x54/0x3b0 drivers/tty/tty_ioctl.c:940
> n_tty_ioctl+0x54/0x320 drivers/tty/n_tty.c:2441
> tty_ioctl+0x5e1/0x1870 drivers/tty/tty_io.c:2655
> vfs_ioctl fs/ioctl.c:46 [inline]
> file_ioctl fs/ioctl.c:500 [inline]
> do_vfs_ioctl+0x1cf/0x16f0 fs/ioctl.c:684
> __do_compat_sys_ioctl fs/compat_ioctl.c:1483 [inline]
> __se_compat_sys_ioctl fs/compat_ioctl.c:1407 [inline]
> __ia32_compat_sys_ioctl+0x43e/0x640 fs/compat_ioctl.c:1407
> do_syscall_32_irqs_on arch/x86/entry/common.c:323 [inline]
> do_fast_syscall_32+0x345/0xf9b arch/x86/entry/common.c:394
> entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
This one is tty IOCTL ==> tty_port->lock ==> console_owner/console_sem ==> uart_port->lock
> Possible unsafe locking scenario:
>
> CPU0 CPU1
> ---- ----
> lock(&(&port->lock)->rlock);
> lock(&port_lock_key);
> lock(&(&port->lock)->rlock);
> lock(console_owner);
IOW
tty ioctl
tty_port->lock IRQ
printk uart_port->lock
console_owner
uart_port->lock tty_port->rlock
The simplest thing to do [not necessarily the right one, tho]
would be to break the IOCTL ==> tty_port->lock ==> printk ==> uart_port->lock
chain.
E.g. by adding __GFP_NOWARN
---
drivers/tty/tty_buffer.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
index c996b6859c5e..71958ef6a831 100644
--- a/drivers/tty/tty_buffer.c
+++ b/drivers/tty/tty_buffer.c
@@ -167,7 +167,8 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
have queued and recycle that ? */
if (atomic_read(&port->buf.mem_used) > port->buf.mem_limit)
return NULL;
- p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
+ p = kmalloc(sizeof(struct tty_buffer) + 2 * size,
+ GFP_ATOMIC | __GFP_NOWARN);
if (p == NULL)
return NULL;
---
Another way could be - switch to printk_safe mode around that
kmalloc():
__printk_safe_enter();
kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
__printk_safe_exit();
This will redirect all printk()-s from kmalloc() to a special per-CPU
buffer, which will be flushed later from a safe context (irq work).
Or, may be, we even can switch to printk_safe mode every time we grab
tty_port lock.
#define tty_port_lock_irqsave(l,f) \
do { \
spin_lock_irqsave((l), f); \
__printk_safe_enter(); \
} while (0)
#define tty_port_unlock_irqrestore(l,f) \
do { \
__printk_safe_exit(); \
spin_lock_irqrestore((l),f); \
} while (0)
This will require some "automatic" replacement of all port->lock
operation in drivers/tty/*.
Perhaps something like this should be done for uart_port->lock
as well. Because, technically, we can have the following
IRQ
uart_port->lock
tty_wakeup
printk
call_console_drivers
console_driver
uart_port->lock << deadlock
Which is totally possible.
E.g. tty_port_default_wakeup()->tty_port_tty_get()->refcount_inc()->WARN_ONCE()
Any opinions?
-ss
^ permalink raw reply related
* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Matwey V. Kornilov @ 2018-06-07 7:03 UTC (permalink / raw)
To: Giulio Benetti
Cc: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
open list
In-Reply-To: <d3c4e356-2c04-731f-a665-a445a6bf68af@micronovasrl.com>
2018-06-06 22:15 GMT+03:00 Giulio Benetti <giulio.benetti@micronovasrl.com>:
> Il 06/06/2018 20:55, Matwey V. Kornilov ha scritto:
>>
>> 2018-06-06 16:11 GMT+03:00 Andy Shevchenko
>> <andriy.shevchenko@linux.intel.com>:
>>>
>>> On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
>>>>
>>>> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
>>>>>
>>>>> On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>>>>>>
>>>>>> em485 gets lost during
>>>>>>
>>>>>> Copy em485 to final uart port.
>>>>>>
>>>>>
>>>>> Is it needed at all?
>>>>>
>>>>> The individual driver decides either to use software emulation (and
>>>>> calls explicitly serial8250_em485_init() for that) or do HW assisted
>>>>> stuff.
>>>>
>>>>
>>>> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config()
>>>> against local struct uart_8250_port uart = {};
>>>> Inside serial8250_register_8250_port() not all uart fields are
>>>> copied(em485 too).
>>>> So after probe, em485 is NULL.
>>>>
>>>> Another way could be to call dw8250_rs485_config() against real uart
>>>> port, after calling serial8250_register_8250_port(),
>>>> would it make sense?
>>>
>>>
>>> Look at OMAP case closely. They have a callback to configure RS485 which
>>> is called in uart_set_rs485_config() which is called whenever user
>>> space does TIOCGRS485 IOCTL.
>>>
>>> So, it's completely driven by user space which makes sense by my
>>> opinion.
>>
>>
>> AFAIU, Giulio wants to add support for rs485-enabled-at-boot-time
>> device tree option (see bindings/serial/rs485.txt for reference).
>
>
> Yes, I want to add support for "rs485-enabled-at-boot-time" property,
> maybe I had to write better commit log and a cover letter. Sorry.
>
>> I suppose it is only important for use-case when rs485 used as slave
>> (peripheral role).
>
>
> It's important also for master, because RTS, if RTS_AFTER_SEND is set,
> remains not asserted(trasnmission) until userspace opens that serial port.
>
And it makes no sense, because how are you going to transmit and
receive not opening the port and not running an application?
It is master who transmits the data first. So, before all systems
going to work you have to open the port from userspace.
In case of slave this of course makes sense.
> Sorry again for not explaining myself well.
>
>
> --
> 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
--
With best regards,
Matwey V. Kornilov
^ permalink raw reply
* Re: [PATCH v2] serial: imx: fix cached UCR2 read on software reset
From: Uwe Kleine-König @ 2018-06-07 7:56 UTC (permalink / raw)
To: Stefan Agner; +Cc: gregkh, festevam, jslaby, linux-serial, linux-kernel
In-Reply-To: <20180420124407.12892-1-stefan@agner.ch>
On Fri, Apr 20, 2018 at 02:44:07PM +0200, Stefan Agner wrote:
> To reset the UART the SRST needs be cleared (low active). According
> to the documentation the bit will remain active for 4 module clocks
> until it is cleared (set to 1).
>
> Hence the real register need to be read in case the cached register
> indicates that the SRST bit is zero.
>
> This bug lead to wrong baudrate because the baud rate register got
> restored before reset completed in imx_flush_buffer.
>
> Fixes: 3a0ab62f43de ("serial: imx: implement shadow registers for UCRx and UFCR")
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
For the record, there is a customer of mine who reports that this commit
breaks rs485 communication on i.MX25 because RTS stops to toggle as
intended.
(Some details: uart3, fsl,uart-has-rtscts, fsl,dte-mode,
linux,rs485-enabled-at-boot-time, native RTS.)
I didn't debug this yet.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table
From: Pavel Machek @ 2018-06-07 10:27 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Ricardo Ribalda Delgado, LKML, linux-serial, Johan Hedberg,
Rob Herring, Johan Hovold, linux-bluetooth
In-Reply-To: <E848BED5-1D83-47FA-A673-29B366A60227@holtmann.org>
Hi!
> > Describe which hardware is supported by the current driver.
> >
> > Cc: Marcel Holtmann <marcel@holtmann.org>
> > Cc: Johan Hedberg <johan.hedberg@gmail.com>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Johan Hovold <johan@kernel.org>
> > Cc: linux-bluetooth@vger.kernel.org
> > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> > ---
> > drivers/bluetooth/hci_nokia.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
> > index 3539fd03f47e..e32dfcd56b8d 100644
> > --- a/drivers/bluetooth/hci_nokia.c
> > +++ b/drivers/bluetooth/hci_nokia.c
> > @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
> > MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
> > #endif
> >
> > +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
> > + { "hp4-bluetooth", },
> > + {},
> > +};
> > +
> > static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> > .probe = nokia_bluetooth_serdev_probe,
> > .remove = nokia_bluetooth_serdev_remove,
> > @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
> > .pm = &nokia_bluetooth_pm_ops,
> > .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
> > },
> > + .id_table = nokia_bluetooth_serdev_id,
> > };
>
> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
>
Would it make sense to cc: sre here? We want good support even for old
hardware, and this is n9/n950, it is still on "top ten supported
phones" list... Probably even top 5.
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply
* [PATCH v6 0/6] Driver for at91 usart in spi mode
From: Radu Pirea @ 2018-06-07 11:00 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
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
Changes in v6:
- removed unused compatible strings from serial and spi drivers
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: change 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 | 68 +++
drivers/spi/Kconfig | 9 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-at91-usart.c | 434 ++++++++++++++++++
drivers/tty/serial/Kconfig | 1 +
drivers/tty/serial/atmel_serial.c | 42 +-
include/dt-bindings/mfd/at91-usart.h | 17 +
11 files changed, 606 insertions(+), 17 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 v6 1/6] MAINTAINERS: add at91 usart mfd driver
From: Radu Pirea @ 2018-06-07 11:00 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: <20180607110020.20565-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 v6 2/6] dt-bindings: add binding for atmel-usart in SPI mode
From: Radu Pirea @ 2018-06-07 11:00 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: <20180607110020.20565-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 v6 3/6] mfd: at91-usart: added mfd driver for usart
From: Radu Pirea @ 2018-06-07 11:00 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: <20180607110020.20565-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 | 68 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 78 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..1a2bdeff7f6d
--- /dev/null
+++ b/drivers/mfd/at91-usart.c
@@ -0,0 +1,68 @@
+// 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 v6 4/6] MAINTAINERS: add at91 usart spi driver
From: Radu Pirea @ 2018-06-07 11:00 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: <20180607110020.20565-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 v6 5/6] spi: at91-usart: add driver for at91-usart as spi
From: Radu Pirea @ 2018-06-07 11:00 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: <20180607110020.20565-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 | 434 +++++++++++++++++++++++++++++++++++
3 files changed, 444 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..ae889aa6bda3
--- /dev/null
+++ b/drivers/spi/spi-at91-usart.c
@@ -0,0 +1,434 @@
+// 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,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 v6 6/6] tty/serial: atmel: change the driver to work under at91-usart mfd
From: Radu Pirea @ 2018-06-07 11:00 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: <20180607110020.20565-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 | 42 ++++++++++++++++++++-----------
2 files changed, 28 insertions(+), 15 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..5ef8a6a6fe17 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -193,8 +193,7 @@ 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" },
{ /* sentinel */ }
};
#endif
@@ -915,6 +914,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 +922,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 +1093,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 +1105,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",
@@ -2222,8 +2223,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 +2239,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 +2342,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;
@@ -2694,13 +2696,22 @@ 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));
+ /*
+ * In device tree is no node with "atmel,at91rm9200-usart-serial"
+ * as compatible string. This driver is probed by at91-usart mfd driver
+ * which is just a wrapper over the atmel_serial driver and
+ * spi-at91-usart driver. All attributes needed by this driver are
+ * found in of_node of parent.
+ */
+ 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:
@@ -2835,6 +2846,7 @@ static int atmel_serial_remove(struct platform_device *pdev)
clk_put(atmel_port->clk);
atmel_port->clk = NULL;
+ pdev->dev.of_node = NULL;
return ret;
}
@@ -2845,7 +2857,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: possible deadlock in console_unlock
From: Petr Mladek @ 2018-06-07 11:00 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: syzbot, linux-kernel, rostedt, sergey.senozhatsky, syzkaller-bugs,
Greg Kroah-Hartman, Jiri Slaby, linux-serial, Andrew Morton
In-Reply-To: <20180607051019.GA10406@jagdpanzerIV>
On Thu 2018-06-07 14:10:19, Sergey Senozhatsky wrote:
> Cc-ing more people
> Link: lkml.kernel.org/r/00000000000087008b056df8fbb3@google.com
>
> On (06/06/18 06:17), syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: af6c5d5e01ad Merge branch 'for-4.18' of git://git.kernel.o..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=173d93ef800000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=12ff770540994680
> > dashboard link: https://syzkaller.appspot.com/bug?extid=43e93968b964e369db0b
> > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > userspace arch: i386
> > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=16e00bb7800000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+43e93968b964e369db0b@syzkaller.appspotmail.com
>
> IOW
>
> tty ioctl
> tty_port->lock IRQ
> printk uart_port->lock
> console_owner
> uart_port->lock tty_port->rlock
Great analyze!
> The simplest thing to do [not necessarily the right one, tho]
> would be to break the IOCTL ==> tty_port->lock ==> printk ==> uart_port->lock
> chain.
>
> E.g. by adding __GFP_NOWARN
>
> ---
>
> drivers/tty/tty_buffer.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
> index c996b6859c5e..71958ef6a831 100644
> --- a/drivers/tty/tty_buffer.c
> +++ b/drivers/tty/tty_buffer.c
> @@ -167,7 +167,8 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
> have queued and recycle that ? */
> if (atomic_read(&port->buf.mem_used) > port->buf.mem_limit)
> return NULL;
> - p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
> + p = kmalloc(sizeof(struct tty_buffer) + 2 * size,
> + GFP_ATOMIC | __GFP_NOWARN);
> if (p == NULL)
> return NULL;
>
> ---
This looks like the most simple solution for this particular problem.
I am just afraid that there are many other locations like this.
> Another way could be - switch to printk_safe mode around that
> kmalloc():
>
> __printk_safe_enter();
> kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
> __printk_safe_exit();
>
> Or, may be, we even can switch to printk_safe mode every time we grab
> tty_port lock.
> Perhaps something like this should be done for uart_port->lock
> as well. Because, technically, we can have the following
Yeah, we would need this basically around any lock that can be taken
from console write() callbacks. Well, this would be needed even
around locks that might be in a chain with a lock used in these
callbacks (as shown by this report).
BTW: printk_safe context might be too strict. In fact,
printk_deferred() would be enough. We might think about
introducing also printk_deferred context.
Best Regards,
Petr
^ permalink raw reply
* Re: possible deadlock in console_unlock
From: Tetsuo Handa @ 2018-06-07 11:40 UTC (permalink / raw)
To: Petr Mladek, Sergey Senozhatsky
Cc: syzbot, linux-kernel, rostedt, sergey.senozhatsky, syzkaller-bugs,
Greg Kroah-Hartman, Jiri Slaby, linux-serial, Andrew Morton
In-Reply-To: <20180607110034.qrkencwsr4stv6xp@pathway.suse.cz>
On 2018/06/07 20:00, Petr Mladek wrote:
>> ---
>>
>> drivers/tty/tty_buffer.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/tty_buffer.c b/drivers/tty/tty_buffer.c
>> index c996b6859c5e..71958ef6a831 100644
>> --- a/drivers/tty/tty_buffer.c
>> +++ b/drivers/tty/tty_buffer.c
>> @@ -167,7 +167,8 @@ static struct tty_buffer *tty_buffer_alloc(struct tty_port *port, size_t size)
>> have queued and recycle that ? */
>> if (atomic_read(&port->buf.mem_used) > port->buf.mem_limit)
>> return NULL;
>> - p = kmalloc(sizeof(struct tty_buffer) + 2 * size, GFP_ATOMIC);
>> + p = kmalloc(sizeof(struct tty_buffer) + 2 * size,
>> + GFP_ATOMIC | __GFP_NOWARN);
>> if (p == NULL)
>> return NULL;
>>
>> ---
>
> This looks like the most simple solution for this particular problem.
> I am just afraid that there are many other locations like this.
>
I haven't tried the reproducer with that change. But isn't __GFP_NOWARN
ignored by fail_dump() (and thus printk() from fault injection still occurs)?
By the way, this reproducer is tricky. It needs to run like ./a.out
followed by "while :; do fg; done" because it always stops by a signal.
^ permalink raw reply
* Re: [PATCH 04/19] Bluetooth: hci_nokia: Add serdev_id_table
From: Marcel Holtmann @ 2018-06-07 12:32 UTC (permalink / raw)
To: Pavel Machek
Cc: Ricardo Ribalda Delgado, LKML, linux-serial, Johan Hedberg,
Rob Herring, Johan Hovold, linux-bluetooth
In-Reply-To: <20180607102740.GD4372@localhost>
Hi Pavel,
>>> Describe which hardware is supported by the current driver.
>>>
>>> Cc: Marcel Holtmann <marcel@holtmann.org>
>>> Cc: Johan Hedberg <johan.hedberg@gmail.com>
>>> Cc: Rob Herring <robh@kernel.org>
>>> Cc: Johan Hovold <johan@kernel.org>
>>> Cc: linux-bluetooth@vger.kernel.org
>>> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
>>> ---
>>> drivers/bluetooth/hci_nokia.c | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/bluetooth/hci_nokia.c b/drivers/bluetooth/hci_nokia.c
>>> index 3539fd03f47e..e32dfcd56b8d 100644
>>> --- a/drivers/bluetooth/hci_nokia.c
>>> +++ b/drivers/bluetooth/hci_nokia.c
>>> @@ -801,6 +801,11 @@ static const struct of_device_id nokia_bluetooth_of_match[] = {
>>> MODULE_DEVICE_TABLE(of, nokia_bluetooth_of_match);
>>> #endif
>>>
>>> +static struct serdev_device_id nokia_bluetooth_serdev_id[] = {
>>> + { "hp4-bluetooth", },
>>> + {},
>>> +};
>>> +
>>> static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
>>> .probe = nokia_bluetooth_serdev_probe,
>>> .remove = nokia_bluetooth_serdev_remove,
>>> @@ -809,6 +814,7 @@ static struct serdev_device_driver nokia_bluetooth_serdev_driver = {
>>> .pm = &nokia_bluetooth_pm_ops,
>>> .of_match_table = of_match_ptr(nokia_bluetooth_of_match),
>>> },
>>> + .id_table = nokia_bluetooth_serdev_id,
>>> };
>>
>> I would actually skip this hardware. First of all it is such a dedicated custom Nokia transport and hardware, and secondly it is no longer produced anyway.
>>
>
> Would it make sense to cc: sre here? We want good support even for old
> hardware, and this is n9/n950, it is still on "top ten supported
> phones" list... Probably even top 5.
but that is not what this patch series is about. We do not need new_id kinda support for the existing hardware. My point is there will be no newly designed hardware using this driver.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH 1/4] serial: 8250: Copy em485 from port to real port.
From: Giulio Benetti @ 2018-06-07 12:43 UTC (permalink / raw)
To: Matwey V. Kornilov
Cc: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby, Matthias Brugger,
Kees Cook, Allen Pais, Sean Young, open list:SERIAL DRIVERS,
open list
In-Reply-To: <CAJs94EbY1apGLUK+6ZXy=j5Uitox+WbG903_5SX2kW_tCLQzPw@mail.gmail.com>
Il 07/06/2018 09:03, Matwey V. Kornilov ha scritto:
> 2018-06-06 22:15 GMT+03:00 Giulio Benetti <giulio.benetti@micronovasrl.com>:
>> Il 06/06/2018 20:55, Matwey V. Kornilov ha scritto:
>>>
>>> 2018-06-06 16:11 GMT+03:00 Andy Shevchenko
>>> <andriy.shevchenko@linux.intel.com>:
>>>>
>>>> On Wed, 2018-06-06 at 14:15 +0200, Giulio Benetti wrote:
>>>>>
>>>>> Il 06/06/2018 13:56, Andy Shevchenko ha scritto:
>>>>>>
>>>>>> On Wed, 2018-06-06 at 11:49 +0200, Giulio Benetti wrote:
>>>>>>>
>>>>>>> em485 gets lost during
>>>>>>>
>>>>>>> Copy em485 to final uart port.
>>>>>>>
>>>>>>
>>>>>> Is it needed at all?
>>>>>>
>>>>>> The individual driver decides either to use software emulation (and
>>>>>> calls explicitly serial8250_em485_init() for that) or do HW assisted
>>>>>> stuff.
>>>>>
>>>>>
>>>>> In 8250_dw.c, during probe(), I need to call dw8250_rs485_config()
>>>>> against local struct uart_8250_port uart = {};
>>>>> Inside serial8250_register_8250_port() not all uart fields are
>>>>> copied(em485 too).
>>>>> So after probe, em485 is NULL.
>>>>>
>>>>> Another way could be to call dw8250_rs485_config() against real uart
>>>>> port, after calling serial8250_register_8250_port(),
>>>>> would it make sense?
>>>>
>>>>
>>>> Look at OMAP case closely. They have a callback to configure RS485 which
>>>> is called in uart_set_rs485_config() which is called whenever user
>>>> space does TIOCGRS485 IOCTL.
>>>>
>>>> So, it's completely driven by user space which makes sense by my
>>>> opinion.
>>>
>>>
>>> AFAIU, Giulio wants to add support for rs485-enabled-at-boot-time
>>> device tree option (see bindings/serial/rs485.txt for reference).
>>
>>
>> Yes, I want to add support for "rs485-enabled-at-boot-time" property,
>> maybe I had to write better commit log and a cover letter. Sorry.
>>
>>> I suppose it is only important for use-case when rs485 used as slave
>>> (peripheral role).
>>
>>
>> It's important also for master, because RTS, if RTS_AFTER_SEND is set,
>> remains not asserted(trasnmission) until userspace opens that serial port.
>>
>
> And it makes no sense, because how are you going to transmit and
> receive not opening the port and not running an application?
> It is master who transmits the data first. So, before all systems
> going to work you have to open the port from userspace.
>
> In case of slave this of course makes sense.
>
Yes it's true, my mistake, it works as master, even if bus will be held
busy by master until port is open.
As slave it keeps bus busy.
IMHO it would be the best to have rs485 free until port is open,
this also reflects the reality:
when rs485 ON and RTS_AFTER_SEND is set, if port is closed or open
without transmitting RTS must be asserted(pin low, rx mode).
Agree?
--
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
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