* sc16is7xx: fix RTS/CTS implementation and add reading CTS @ 2016-07-27 20:58 Pascal JEAN 2016-07-27 20:58 ` [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation Pascal JEAN 2016-07-27 20:58 ` Pascal JEAN 0 siblings, 2 replies; 8+ messages in thread From: Pascal JEAN @ 2016-07-27 20:58 UTC (permalink / raw) To: gregkh; +Cc: jslaby, linux-serial, linux-kernel Hi Greg, Here are explanations of the two patches: 0001-sc16is7xx-fix-RTS-CTS-implementation.patch: RTS/CTS handshaking for sc16is7xx driver does not work, no character is sent regardless of the state of CTS. This test was done on a raspberry pi 2: 1- sc16is7xx_set_baud() overwrites the flow control configuration bits in EFR. Writing in EFR has been replaced by an update. 2- As noted in the file Documentation/serial/driver, get_mctrl() should return an active state for unsupported entries. 0002-sc16is7xx-add-reading-CTS.patch: This patch adds the possibility to read the actual status of the CTS input when RTS/CTS handshaking is not activated. After those changes everything works. Thank you for your hard work. Have a nice day. Pascal ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation 2016-07-27 20:58 sc16is7xx: fix RTS/CTS implementation and add reading CTS Pascal JEAN @ 2016-07-27 20:58 ` Pascal JEAN 2016-07-27 21:28 ` Greg KH 2016-07-27 20:58 ` Pascal JEAN 1 sibling, 1 reply; 8+ messages in thread From: Pascal JEAN @ 2016-07-27 20:58 UTC (permalink / raw) To: gregkh; +Cc: jslaby, linux-serial, linux-kernel, Pascal JEAN Signed-off-by: Pascal JEAN <epsilonrt@gmail.com> --- drivers/tty/serial/sc16is7xx.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index f36e6df..3e65079 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -511,16 +511,18 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) /* Enable enhanced features */ regcache_cache_bypass(s->regmap, true); - sc16is7xx_port_write(port, SC16IS7XX_EFR_REG, - SC16IS7XX_EFR_ENABLE_BIT); + sc16is7xx_port_update(port, SC16IS7XX_EFR_REG, + SC16IS7XX_EFR_ENABLE_BIT, + SC16IS7XX_EFR_ENABLE_BIT); + sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, + SC16IS7XX_MCR_CLKSEL_BIT, + prescaler); + sc16is7xx_port_update(port, SC16IS7XX_EFR_REG, + SC16IS7XX_EFR_ENABLE_BIT, 0); regcache_cache_bypass(s->regmap, false); /* Put LCR back to the normal mode */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr); - sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, - SC16IS7XX_MCR_CLKSEL_BIT, - prescaler); /* Open the LCR divisors for configuration */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, @@ -817,9 +819,9 @@ static unsigned int sc16is7xx_tx_empty(struct uart_port *port) static unsigned int sc16is7xx_get_mctrl(struct uart_port *port) { /* DCD and DSR are not wired and CTS/RTS is handled automatically - * so just indicate DSR and CAR asserted + * so just indicate all inputs asserted */ - return TIOCM_DSR | TIOCM_CAR; + return TIOCM_DSR | TIOCM_CAR | TIOCM_RI | TIOCM_CTS; } static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl) -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation 2016-07-27 20:58 ` [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation Pascal JEAN @ 2016-07-27 21:28 ` Greg KH 2016-07-28 6:56 ` Pascal JEAN 2016-07-28 7:07 ` [PATCH 2/2] sc16is7xx: add reading CTS Pascal JEAN 0 siblings, 2 replies; 8+ messages in thread From: Greg KH @ 2016-07-27 21:28 UTC (permalink / raw) To: Pascal JEAN; +Cc: jslaby, linux-serial, linux-kernel On Wed, Jul 27, 2016 at 10:58:42PM +0200, Pascal JEAN wrote: > Signed-off-by: Pascal JEAN <epsilonrt@gmail.com> > --- > drivers/tty/serial/sc16is7xx.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) I can't take patches with out any changelog text, sorry, please add the information you had in patch 0/2 to the text in these two patches, plus any other information that might be needed. Also, your Signed-off-by: and From: email addresses don't match, please fix that up somehow. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation 2016-07-27 21:28 ` Greg KH @ 2016-07-28 6:56 ` Pascal JEAN 2016-07-28 7:07 ` [PATCH 2/2] sc16is7xx: add reading CTS Pascal JEAN 1 sibling, 0 replies; 8+ messages in thread From: Pascal JEAN @ 2016-07-28 6:56 UTC (permalink / raw) To: Greg KH; +Cc: jslaby, linux-serial, linux-kernel Hi Greg, Here are changelog for this patch. Best Regards Le 27/07/2016 à 23:28, Greg KH a écrit : > On Wed, Jul 27, 2016 at 10:58:42PM +0200, Pascal JEAN wrote: >> Signed-off-by: Pascal JEAN <epsilonrt@gmail.com> >> --- >> drivers/tty/serial/sc16is7xx.c | 16 +++++++++------- >> 1 file changed, 9 insertions(+), 7 deletions(-) > > I can't take patches with out any changelog text, sorry, please add the > information you had in patch 0/2 to the text in these two patches, plus > any other information that might be needed. > > Also, your Signed-off-by: and From: email addresses don't match, please > fix that up somehow. > > thanks, > > greg k-h > Changelog: sc16is7xx: fix RTS/CTS implementation RTS/CTS handshaking for sc16is7xx driver does not work, no character is sent regardless of the state of CTS. Fixes: 1- sc16is7xx_set_baud() overwrites the flow control configuration bits in EFR. Writing in EFR has been replaced by an update. 2- As noted in the file Documentation/serial/driver, get_mctrl() should return an active state for unsupported entries. After those changes everything works. Signed-off-by: Pascal JEAN <epsilonrt@gmail.com> --- drivers/tty/serial/sc16is7xx.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index f36e6df..3e65079 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -511,16 +511,18 @@ static int sc16is7xx_set_baud(struct uart_port *port, int baud) /* Enable enhanced features */ regcache_cache_bypass(s->regmap, true); - sc16is7xx_port_write(port, SC16IS7XX_EFR_REG, - SC16IS7XX_EFR_ENABLE_BIT); + sc16is7xx_port_update(port, SC16IS7XX_EFR_REG, + SC16IS7XX_EFR_ENABLE_BIT, + SC16IS7XX_EFR_ENABLE_BIT); + sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, + SC16IS7XX_MCR_CLKSEL_BIT, + prescaler); + sc16is7xx_port_update(port, SC16IS7XX_EFR_REG, + SC16IS7XX_EFR_ENABLE_BIT, 0); regcache_cache_bypass(s->regmap, false); /* Put LCR back to the normal mode */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, lcr); - sc16is7xx_port_update(port, SC16IS7XX_MCR_REG, - SC16IS7XX_MCR_CLKSEL_BIT, - prescaler); /* Open the LCR divisors for configuration */ sc16is7xx_port_write(port, SC16IS7XX_LCR_REG, @@ -817,9 +819,9 @@ static unsigned int sc16is7xx_tx_empty(struct uart_port *port) static unsigned int sc16is7xx_get_mctrl(struct uart_port *port) { /* DCD and DSR are not wired and CTS/RTS is handled automatically - * so just indicate DSR and CAR asserted + * so just indicate all inputs asserted */ - return TIOCM_DSR | TIOCM_CAR; + return TIOCM_DSR | TIOCM_CAR | TIOCM_RI | TIOCM_CTS; } static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl) -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] sc16is7xx: add reading CTS 2016-07-27 21:28 ` Greg KH 2016-07-28 6:56 ` Pascal JEAN @ 2016-07-28 7:07 ` Pascal JEAN 2016-07-28 14:10 ` Greg KH 1 sibling, 1 reply; 8+ messages in thread From: Pascal JEAN @ 2016-07-28 7:07 UTC (permalink / raw) To: Greg KH; +Cc: jslaby, linux-serial, linux-kernel Hi Greg, Here are changelog for this patch. Best Regards Changelog: sc16is7xx: add reading CTS This patch adds the possibility to read the actual status of the CTS input when RTS/CTS handshaking is not activated (for general purposes). Signed-off-by: Pascal JEAN <epsilonrt@gmail.com> --- drivers/tty/serial/sc16is7xx.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 3e65079..8833a18 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -818,10 +818,19 @@ static unsigned int sc16is7xx_tx_empty(struct uart_port *port) static unsigned int sc16is7xx_get_mctrl(struct uart_port *port) { - /* DCD and DSR are not wired and CTS/RTS is handled automatically - * so just indicate all inputs asserted - */ - return TIOCM_DSR | TIOCM_CAR | TIOCM_RI | TIOCM_CTS; + /* DCD, DSR and RI are not wired so just indicate asserted */ + unsigned int mctrl = TIOCM_CAR | TIOCM_DSR | TIOCM_RI; + + if (port->status & UPSTAT_CTS_ENABLE) + /* CTS handled automatically, indicates that it is always + * asserted, this is required for proper management of + * the upper layer + */ + mctrl |= TIOCM_CTS; + else { + /* CTS is not managed automatically, returns its actual state + * the upper layer + */ + u8 msr = sc16is7xx_port_read(port, SC16IS7XX_MSR_REG); + + mctrl |= (msr & SC16IS7XX_MSR_CTS_BIT) ? TIOCM_CTS : 0; + } + return mctrl; } static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl) -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] sc16is7xx: add reading CTS 2016-07-28 7:07 ` [PATCH 2/2] sc16is7xx: add reading CTS Pascal JEAN @ 2016-07-28 14:10 ` Greg KH 0 siblings, 0 replies; 8+ messages in thread From: Greg KH @ 2016-07-28 14:10 UTC (permalink / raw) To: Pascal JEAN; +Cc: jslaby, linux-serial, linux-kernel On Thu, Jul 28, 2016 at 09:07:36AM +0200, Pascal JEAN wrote: > Hi Greg, > > Here are changelog for this patch. > Best Regards > > Changelog: <snip> Um, I can't take this, sorry. Please re-read Documentation/SubmittingPatches for how to properly format and send things, I should not have to hand-edit any files in order to accept a patch. Please also version number your patches, with the needed information. This would have been "v2", I'll expect to see "v3" after this. Same goes for the other patch in this series. thanks, greg k-h ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] sc16is7xx: add reading CTS 2016-07-27 20:58 sc16is7xx: fix RTS/CTS implementation and add reading CTS Pascal JEAN 2016-07-27 20:58 ` [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation Pascal JEAN @ 2016-07-27 20:58 ` Pascal JEAN 1 sibling, 0 replies; 8+ messages in thread From: Pascal JEAN @ 2016-07-27 20:58 UTC (permalink / raw) To: gregkh; +Cc: jslaby, linux-serial, linux-kernel, Pascal JEAN Signed-off-by: Pascal JEAN <epsilonrt@gmail.com> --- drivers/tty/serial/sc16is7xx.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c index 3e65079..8833a18 100644 --- a/drivers/tty/serial/sc16is7xx.c +++ b/drivers/tty/serial/sc16is7xx.c @@ -818,10 +818,19 @@ static unsigned int sc16is7xx_tx_empty(struct uart_port *port) static unsigned int sc16is7xx_get_mctrl(struct uart_port *port) { - /* DCD and DSR are not wired and CTS/RTS is handled automatically - * so just indicate all inputs asserted - */ - return TIOCM_DSR | TIOCM_CAR | TIOCM_RI | TIOCM_CTS; + /* DCD, DSR and RI are not wired so just indicate asserted */ + unsigned int mctrl = TIOCM_CAR | TIOCM_DSR | TIOCM_RI; + + if (port->status & UPSTAT_CTS_ENABLE) + /* CTS handled automatically, indicates that it is always + * asserted, this is required for proper management of + * the upper layer + */ + mctrl |= TIOCM_CTS; + else { + /* CTS is not managed automatically, returns its actual state + * the upper layer + */ + u8 msr = sc16is7xx_port_read(port, SC16IS7XX_MSR_REG); + + mctrl |= (msr & SC16IS7XX_MSR_CTS_BIT) ? TIOCM_CTS : 0; + } + return mctrl; } static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl) -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] sc16is7xx: add reading CTS
@ 2016-07-28 11:16 Maarten Brock
0 siblings, 0 replies; 8+ messages in thread
From: Maarten Brock @ 2016-07-28 11:16 UTC (permalink / raw)
To: Pascal JEAN, Greg KH; +Cc: jslaby, linux-serial, linux-kernel
Hello Pascal,
If you look at the history of this driver, you'll see that something similar was
present in the past, but was removed because it introduced problems. The problem
is that you can't just call sc16is7xx_port_read() unsynchronized to the worker
thread. And this is related to the device using indirect access through an I2C
or SPI driver with separate interrupts from the UART. There is also more info on
the linux-serial mailing list.
Kind regards,
Maarten Brock
----- Original Message -----
From: Pascal JEAN [mailto:epsilonrt@gmail.com]
To: Greg KH [mailto:gregkh@linuxfoundation.org]
Cc: jslaby@suse.com, linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org
Sent: Thu, 28 Jul 2016 09:07:36 +0200
Subject: [PATCH 2/2] sc16is7xx: add reading CTS
> Hi Greg,
>
> Here are changelog for this patch.
> Best Regards
>
> Changelog:
>
> sc16is7xx: add reading CTS
>
> This patch adds the possibility to read the actual
> status of the CTS input when RTS/CTS handshaking is
> not activated (for general purposes).
>
>
> Signed-off-by: Pascal JEAN <epsilonrt@gmail.com>
> ---
> drivers/tty/serial/sc16is7xx.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> index 3e65079..8833a18 100644
> --- a/drivers/tty/serial/sc16is7xx.c
> +++ b/drivers/tty/serial/sc16is7xx.c
> @@ -818,10 +818,19 @@ static unsigned int sc16is7xx_tx_empty(struct
> uart_port *port)
>
> static unsigned int sc16is7xx_get_mctrl(struct uart_port *port)
> {
> - /* DCD and DSR are not wired and CTS/RTS is handled automatically
> - * so just indicate all inputs asserted
> - */
> - return TIOCM_DSR | TIOCM_CAR | TIOCM_RI | TIOCM_CTS;
> + /* DCD, DSR and RI are not wired so just indicate asserted */
> + unsigned int mctrl = TIOCM_CAR | TIOCM_DSR | TIOCM_RI;
> +
> + if (port->status & UPSTAT_CTS_ENABLE)
> + /* CTS handled automatically, indicates that it is always
> + * asserted, this is required for proper management of
> + * the upper layer
> + */
> + mctrl |= TIOCM_CTS;
> + else {
> + /* CTS is not managed automatically, returns its actual state
> + * the upper layer
> + */
> + u8 msr = sc16is7xx_port_read(port, SC16IS7XX_MSR_REG);
> +
> + mctrl |= (msr & SC16IS7XX_MSR_CTS_BIT) ? TIOCM_CTS : 0;
> + }
> + return mctrl;
> }
>
> static void sc16is7xx_set_mctrl(struct uart_port *port, unsigned int mctrl)
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-serial" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 8+ messages in threadend of thread, other threads:[~2016-07-28 14:10 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-07-27 20:58 sc16is7xx: fix RTS/CTS implementation and add reading CTS Pascal JEAN 2016-07-27 20:58 ` [PATCH 1/2] sc16is7xx: fix RTS/CTS implementation Pascal JEAN 2016-07-27 21:28 ` Greg KH 2016-07-28 6:56 ` Pascal JEAN 2016-07-28 7:07 ` [PATCH 2/2] sc16is7xx: add reading CTS Pascal JEAN 2016-07-28 14:10 ` Greg KH 2016-07-27 20:58 ` Pascal JEAN -- strict thread matches above, loose matches on Subject: below -- 2016-07-28 11:16 Maarten Brock
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).