Messages in this thread | | | Subject | Re: [PATCH v2 4/7] serial: 8250: Handle implementations not having TEMT interrupt using em485 | From | Giulio Benetti <> | Date | Thu, 26 Mar 2020 03:02:39 +0100 |
| |
Hi Heiko,
Il 26/03/2020 01:05, Heiko Stübner ha scritto: > Hi Giulio, > > Am Donnerstag, 26. März 2020, 00:47:38 CET schrieb Giulio Benetti: >> very cleaner way to handle TEMT as a capability! >> And I've found one thing... >> >> Il 26/03/2020 00:14, Heiko Stuebner ha scritto: >>> From: Giulio Benetti <giulio.benetti@micronovasrl.com> >>> >>> Some 8250 ports have a TEMT interrupt but it's not a part of the 8250 >>> standard, instead only available on some implementations. >>> >>> The current em485 implementation does not work on ports without it. >>> The only chance to make it work is to loop-read on LSR register. >>> >>> So add UART_CAP_TEMT to mark 8250 uarts having this interrupt, >>> update all current em485 users with that capability and make >>> the stop_tx function loop-read on uarts not having it. >>> >>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com> >>> [moved to use added UART_CAP_TEMT, use readx_poll_timeout] >>> Signed-off-by: Heiko Stuebner <heiko.stuebner@theobroma-systems.com> >>> --- >>> drivers/tty/serial/8250/8250.h | 1 + >>> drivers/tty/serial/8250/8250_bcm2835aux.c | 2 +- >>> drivers/tty/serial/8250/8250_of.c | 2 ++ >>> drivers/tty/serial/8250/8250_omap.c | 2 +- >>> drivers/tty/serial/8250/8250_port.c | 25 +++++++++++++++++++---- >>> 5 files changed, 26 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h >>> index 52bb21205bb6..770eb00db497 100644 >>> --- a/drivers/tty/serial/8250/8250.h >>> +++ b/drivers/tty/serial/8250/8250.h >>> @@ -82,6 +82,7 @@ struct serial8250_config { >>> #define UART_CAP_MINI (1 << 17) /* Mini UART on BCM283X family lacks: >>> * STOP PARITY EPAR SPAR WLEN5 WLEN6 >>> */ >>> +#define UART_CAP_TEMT (1 << 18) /* UART has TEMT interrupt */ >>> >>> #define UART_BUG_QUOT (1 << 0) /* UART has buggy quot LSB */ >>> #define UART_BUG_TXEN (1 << 1) /* UART has buggy TX IIR status */ >>> diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c >>> index 12d03e678295..3881242424ca 100644 >>> --- a/drivers/tty/serial/8250/8250_bcm2835aux.c >>> +++ b/drivers/tty/serial/8250/8250_bcm2835aux.c >>> @@ -91,7 +91,7 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev) >>> return -ENOMEM; >>> >>> /* initialize data */ >>> - up.capabilities = UART_CAP_FIFO | UART_CAP_MINI; >>> + up.capabilities = UART_CAP_FIFO | UART_CAP_MINI | UART_CAP_TEMT; >>> up.port.dev = &pdev->dev; >>> up.port.regshift = 2; >>> up.port.type = PORT_16550; >>> diff --git a/drivers/tty/serial/8250/8250_of.c b/drivers/tty/serial/8250/8250_of.c >>> index 65e9045dafe6..841f6fcb2878 100644 >>> --- a/drivers/tty/serial/8250/8250_of.c >>> +++ b/drivers/tty/serial/8250/8250_of.c >>> @@ -225,6 +225,8 @@ static int of_platform_serial_probe(struct platform_device *ofdev) >>> &port8250.overrun_backoff_time_ms) != 0) >>> port8250.overrun_backoff_time_ms = 0; >>> >>> + port8250.capabilities |= UART_CAP_TEMT; >>> + >> >> Shouldn't this be NOT UART_CAP_TEMT set by default? On all other >> vendor specific files you enable it, I think here you shouldn't enable >> it too by default. Right? > > 8250_of does use the em485 emulation - see of_platform_serial_setup() > So I did go by the lazy assumption that any 8250 driver using rs485 > before my series always used the interrupt driver code path, so > implicitly required to have the TEMT interrupt. > > Of course, you're right that with the 8250_of maybe not all variants > actually do have this interrupt, so falling back to the polling here might > be safer.
Probably here it's worth introducing a dt boolean property like "temt-capability", then you set or not UART_CAP_TEMT according to its presence in dts. This way all cases are covered and we can act completely through dts files.
What about that?
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
| |