* [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. @ 2021-12-31 17:15 jmades 2022-01-02 10:07 ` Lukas Wunner 0 siblings, 1 reply; 10+ messages in thread From: jmades @ 2021-12-31 17:15 UTC (permalink / raw) To: gregkh; +Cc: jochen, Russell King, Jiri Slaby, linux-serial, linux-kernel Based on the "0001-serial-amba-pl011-add-RS485-support.patch" this change is necesarry otherwise the RTS-line will be pulled up in SER_RS485_RTS_BEFORE_SEND mode before sending data. This hinders the driver to receive data, f.ex. when the device is an RS485 slave device. Signed-off-by: jmades <jochen@mades.net> --- drivers/tty/serial/amba-pl011.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 537f37ac4..1749c1498 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) container_of(port, struct uart_amba_port, port); unsigned int cr; - if (port->rs485.flags & SER_RS485_ENABLED) - mctrl &= ~TIOCM_RTS; + if (port->rs485.flags & SER_RS485_ENABLED) { + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) + mctrl &= ~TIOCM_RTS; + else + mctrl |= TIOCM_RTS; + } cr = pl011_read(uap, REG_CR); -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2021-12-31 17:15 [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function jmades @ 2022-01-02 10:07 ` Lukas Wunner 2022-01-02 15:06 ` Lino Sanfilippo 2022-01-13 10:12 ` Jochen Mades 0 siblings, 2 replies; 10+ messages in thread From: Lukas Wunner @ 2022-01-02 10:07 UTC (permalink / raw) To: jmades Cc: gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Lino Sanfilippo, Philipp Rosenberger On Fri, Dec 31, 2021 at 05:15:14PM +0000, jmades wrote: > Based on the "0001-serial-amba-pl011-add-RS485-support.patch" this change is necesarry otherwise the RTS-line will be pulled up in SER_RS485_RTS_BEFORE_SEND mode before sending data. This hinders the driver to receive data, f.ex. when the device is an RS485 slave device. > > Signed-off-by: jmades <jochen@mades.net> Patch is correct, but commit message could be improved: * Subject should be in imperative mood (by convention), it should be prepended by "serial: pl011: " (in line with previous commits touching this driver, use "git log --oneline amba-pl011.c") and the trailing dot is unnecessary, e.g.: "serial: pl011: Fix incorrect rs485 RTS polarity on set_mctrl" * Commit message should be wrapped at 72 characters (so that it appears centered when displayed with "git log" on an 80 chars terminal). The reference to "0001-serial-amba-pl011-add-RS485-support.patch" should be replaced with a reference to the offending commit, e.g.: "Commit 8d479237727c ("serial: amba-pl011: add RS485 support") sought to keep RTS deasserted on set_mctrl if rs485 is enabled. However it did so only if deasserted RTS polarity is high. Fix it in case it's low." Feel free to copy this to a v2 of your patch and amend as you see fit. * Add tags for the offending commit: Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support") Cc: stable@vger.kernel.org # v5.15+ * Be sure to cc the author of the offending commit. Thanks, Lukas > --- > drivers/tty/serial/amba-pl011.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c > index 537f37ac4..1749c1498 100644 > --- a/drivers/tty/serial/amba-pl011.c > +++ b/drivers/tty/serial/amba-pl011.c > @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) > container_of(port, struct uart_amba_port, port); > unsigned int cr; > > - if (port->rs485.flags & SER_RS485_ENABLED) > - mctrl &= ~TIOCM_RTS; > + if (port->rs485.flags & SER_RS485_ENABLED) { > + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > + mctrl &= ~TIOCM_RTS; > + else > + mctrl |= TIOCM_RTS; > + } > > cr = pl011_read(uap, REG_CR); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-02 10:07 ` Lukas Wunner @ 2022-01-02 15:06 ` Lino Sanfilippo 2022-01-02 18:28 ` Lukas Wunner 2022-01-13 10:12 ` Jochen Mades 1 sibling, 1 reply; 10+ messages in thread From: Lino Sanfilippo @ 2022-01-02 15:06 UTC (permalink / raw) To: Lukas Wunner, jmades Cc: gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Philipp Rosenberger Hi, On 02.01.22 at 11:07, Lukas Wunner wrote: > On Fri, Dec 31, 2021 at 05:15:14PM +0000, jmades wrote: >> Based on the "0001-serial-amba-pl011-add-RS485-support.patch" this change is necesarry otherwise the RTS-line will be pulled up in SER_RS485_RTS_BEFORE_SEND mode before sending data. This hinders the driver to receive data, f.ex. when the device is an RS485 slave device. >> >> Signed-off-by: jmades <jochen@mades.net> > > Patch is correct, but commit message could be improved: > > * Subject should be in imperative mood (by convention), it should be > prepended by "serial: pl011: " (in line with previous commits touching > this driver, use "git log --oneline amba-pl011.c") and the trailing dot > is unnecessary, e.g.: > > "serial: pl011: Fix incorrect rs485 RTS polarity on set_mctrl" > > * Commit message should be wrapped at 72 characters (so that it appears > centered when displayed with "git log" on an 80 chars terminal). > The reference to "0001-serial-amba-pl011-add-RS485-support.patch" > should be replaced with a reference to the offending commit, e.g.: > > "Commit 8d479237727c ("serial: amba-pl011: add RS485 support") sought > to keep RTS deasserted on set_mctrl if rs485 is enabled. However it > did so only if deasserted RTS polarity is high. Fix it in case it's > low." > > Feel free to copy this to a v2 of your patch and amend as you see fit. > > * Add tags for the offending commit: > > Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support") > Cc: stable@vger.kernel.org # v5.15+ > > * Be sure to cc the author of the offending commit. > > Thanks, > > Lukas > >> --- >> drivers/tty/serial/amba-pl011.c | 8 ++++++-- >> 1 file changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c >> index 537f37ac4..1749c1498 100644 >> --- a/drivers/tty/serial/amba-pl011.c >> +++ b/drivers/tty/serial/amba-pl011.c >> @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) >> container_of(port, struct uart_amba_port, port); >> unsigned int cr; >> >> - if (port->rs485.flags & SER_RS485_ENABLED) >> - mctrl &= ~TIOCM_RTS; >> + if (port->rs485.flags & SER_RS485_ENABLED) { >> + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) >> + mctrl &= ~TIOCM_RTS; >> + else >> + mctrl |= TIOCM_RTS; >> + } >> >> cr = pl011_read(uap, REG_CR); Does this logic really have to be implemented in the driver? It looks as if the serial core already takes RS485 into account before calling set_mctrls(). At least I get the impression when looking at uart_tiocmset() and uart_port_dtr_rts(). Also other drivers like imx simply seem to ignore RTS in case of RS485. Regards, Lino ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-02 15:06 ` Lino Sanfilippo @ 2022-01-02 18:28 ` Lukas Wunner 2022-01-03 13:44 ` Lino Sanfilippo 2022-01-11 10:00 ` Jochen Mades 0 siblings, 2 replies; 10+ messages in thread From: Lukas Wunner @ 2022-01-02 18:28 UTC (permalink / raw) To: Lino Sanfilippo Cc: jmades, gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Philipp Rosenberger On Sun, Jan 02, 2022 at 04:06:53PM +0100, Lino Sanfilippo wrote: > On 02.01.22 at 11:07, Lukas Wunner wrote: > > On Fri, Dec 31, 2021 at 05:15:14PM +0000, jmades wrote: > > > --- a/drivers/tty/serial/amba-pl011.c > > > +++ b/drivers/tty/serial/amba-pl011.c > > > @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) > > > container_of(port, struct uart_amba_port, port); > > > unsigned int cr; > > > > > > - if (port->rs485.flags & SER_RS485_ENABLED) > > > - mctrl &= ~TIOCM_RTS; > > > + if (port->rs485.flags & SER_RS485_ENABLED) { > > > + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > > > + mctrl &= ~TIOCM_RTS; > > > + else > > > + mctrl |= TIOCM_RTS; > > > + } > > > > > > cr = pl011_read(uap, REG_CR); > > Does this logic really have to be implemented in the driver? No, it doesn't have to be and indeed I'm working towards consolidating it in the serial core with this collection of patches: https://git.kernel.org/gregkh/tty/c/d3b3404df318 https://lore.kernel.org/all/f49f945375f5ccb979893c49f1129f51651ac738.1641129062.git.lukas@wunner.de https://lore.kernel.org/all/e22089ab49e6e78822c50c8c4db46bf3ee885623.1641129328.git.lukas@wunner.de https://lore.kernel.org/all/bceeaba030b028ed810272d55d5fc6f3656ddddb.1641129752.git.lukas@wunner.de https://github.com/l1k/linux/commit/532ef2ad757f The last of these removes the rs485 logic from pl011_set_mctrl(). I'll post it once the others (and Jochen Mades' patch) have landed. Even though the logic is eventually removed from pl011_set_mctrl(), Jochen's patch makes sense as a backportable fix for v5.15. > It looks as if the serial core already takes RS485 into account before > calling set_mctrls(). At least I get the impression when looking > at uart_tiocmset() and uart_port_dtr_rts(). Also other drivers like imx > simply seem to ignore RTS in case of RS485. The logic in uart_port_dtr_rts() is broken. That's fixed by d3b3404df318, which is queued up in tty-next for v5.17. The pl011 driver papered over it with its own rs485-specific logic in pl011_set_mctrl(). But as Jochen Mades correctly pointed out, that only worked correctly if RTS is driven high on idle. The logic in uart_tiocmset() is correct, but not sufficient because uart_throttle(), uart_unthrottle and uart_set_termios() need to become rs485-aware as well. That's also addressed by the above-linked GitHub commit. Thanks, Lukas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-02 18:28 ` Lukas Wunner @ 2022-01-03 13:44 ` Lino Sanfilippo 2022-01-11 10:00 ` Jochen Mades 1 sibling, 0 replies; 10+ messages in thread From: Lino Sanfilippo @ 2022-01-03 13:44 UTC (permalink / raw) To: Lukas Wunner Cc: jmades, gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Philipp Rosenberger On 02.01.22 at 19:28, Lukas Wunner wrote: > On Sun, Jan 02, 2022 at 04:06:53PM +0100, Lino Sanfilippo wrote: >> On 02.01.22 at 11:07, Lukas Wunner wrote: >>> On Fri, Dec 31, 2021 at 05:15:14PM +0000, jmades wrote: >>>> --- a/drivers/tty/serial/amba-pl011.c >>>> +++ b/drivers/tty/serial/amba-pl011.c >>>> @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) >>>> container_of(port, struct uart_amba_port, port); >>>> unsigned int cr; >>>> >>>> - if (port->rs485.flags & SER_RS485_ENABLED) >>>> - mctrl &= ~TIOCM_RTS; >>>> + if (port->rs485.flags & SER_RS485_ENABLED) { >>>> + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) >>>> + mctrl &= ~TIOCM_RTS; >>>> + else >>>> + mctrl |= TIOCM_RTS; >>>> + } >>>> >>>> cr = pl011_read(uap, REG_CR); >> >> Does this logic really have to be implemented in the driver? > > No, it doesn't have to be and indeed I'm working towards consolidating > it in the serial core with this collection of patches: > > https://git.kernel.org/gregkh/tty/c/d3b3404df318 > https://lore.kernel.org/all/f49f945375f5ccb979893c49f1129f51651ac738.1641129062.git.lukas@wunner.de > https://lore.kernel.org/all/e22089ab49e6e78822c50c8c4db46bf3ee885623.1641129328.git.lukas@wunner.de > https://lore.kernel.org/all/bceeaba030b028ed810272d55d5fc6f3656ddddb.1641129752.git.lukas@wunner.de > https://github.com/l1k/linux/commit/532ef2ad757f > > The last of these removes the rs485 logic from pl011_set_mctrl(). > I'll post it once the others (and Jochen Mades' patch) have landed. > > Even though the logic is eventually removed from pl011_set_mctrl(), > Jochen's patch makes sense as a backportable fix for v5.15. > > >> It looks as if the serial core already takes RS485 into account before >> calling set_mctrls(). At least I get the impression when looking >> at uart_tiocmset() and uart_port_dtr_rts(). Also other drivers like imx >> simply seem to ignore RTS in case of RS485. > > The logic in uart_port_dtr_rts() is broken. That's fixed by d3b3404df318, > which is queued up in tty-next for v5.17. > > The pl011 driver papered over it with its own rs485-specific logic in > pl011_set_mctrl(). But as Jochen Mades correctly pointed out, that > only worked correctly if RTS is driven high on idle. > > > The logic in uart_tiocmset() is correct, but not sufficient because > uart_throttle(), uart_unthrottle and uart_set_termios() need to become > rs485-aware as well. That's also addressed by the above-linked > GitHub commit. > Thanks for this information. I have one or two patches prepared a while ago that aim into a similar direction (move RS485 related code out of the drivers into the serial core). I will send them in the next days. Best regards, Lino ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-02 18:28 ` Lukas Wunner 2022-01-03 13:44 ` Lino Sanfilippo @ 2022-01-11 10:00 ` Jochen Mades 2022-01-12 1:41 ` Lino Sanfilippo 1 sibling, 1 reply; 10+ messages in thread From: Jochen Mades @ 2022-01-11 10:00 UTC (permalink / raw) To: Lukas Wunner, Lino Sanfilippo Cc: gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Philipp Rosenberger Hi Lukas, Lino, please let me know when I could test again with an "official" linux kernel, instead of using my local patch. Bests Jochen > On 02/01/2022 19:28 Lukas Wunner <lukas@wunner.de> wrote: > > > On Sun, Jan 02, 2022 at 04:06:53PM +0100, Lino Sanfilippo wrote: > > On 02.01.22 at 11:07, Lukas Wunner wrote: > > > On Fri, Dec 31, 2021 at 05:15:14PM +0000, jmades wrote: > > > > --- a/drivers/tty/serial/amba-pl011.c > > > > +++ b/drivers/tty/serial/amba-pl011.c > > > > @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) > > > > container_of(port, struct uart_amba_port, port); > > > > unsigned int cr; > > > > > > > > - if (port->rs485.flags & SER_RS485_ENABLED) > > > > - mctrl &= ~TIOCM_RTS; > > > > + if (port->rs485.flags & SER_RS485_ENABLED) { > > > > + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > > > > + mctrl &= ~TIOCM_RTS; > > > > + else > > > > + mctrl |= TIOCM_RTS; > > > > + } > > > > > > > > cr = pl011_read(uap, REG_CR); > > > > Does this logic really have to be implemented in the driver? > > No, it doesn't have to be and indeed I'm working towards consolidating > it in the serial core with this collection of patches: > > https://git.kernel.org/gregkh/tty/c/d3b3404df318 > https://lore.kernel.org/all/f49f945375f5ccb979893c49f1129f51651ac738.1641129062.git.lukas@wunner.de > https://lore.kernel.org/all/e22089ab49e6e78822c50c8c4db46bf3ee885623.1641129328.git.lukas@wunner.de > https://lore.kernel.org/all/bceeaba030b028ed810272d55d5fc6f3656ddddb.1641129752.git.lukas@wunner.de > https://github.com/l1k/linux/commit/532ef2ad757f > > The last of these removes the rs485 logic from pl011_set_mctrl(). > I'll post it once the others (and Jochen Mades' patch) have landed. > > Even though the logic is eventually removed from pl011_set_mctrl(), > Jochen's patch makes sense as a backportable fix for v5.15. > > > > It looks as if the serial core already takes RS485 into account before > > calling set_mctrls(). At least I get the impression when looking > > at uart_tiocmset() and uart_port_dtr_rts(). Also other drivers like imx > > simply seem to ignore RTS in case of RS485. > > The logic in uart_port_dtr_rts() is broken. That's fixed by d3b3404df318, > which is queued up in tty-next for v5.17. > > The pl011 driver papered over it with its own rs485-specific logic in > pl011_set_mctrl(). But as Jochen Mades correctly pointed out, that > only worked correctly if RTS is driven high on idle. > > > The logic in uart_tiocmset() is correct, but not sufficient because > uart_throttle(), uart_unthrottle and uart_set_termios() need to become > rs485-aware as well. That's also addressed by the above-linked > GitHub commit. > > Thanks, > > Lukas ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-11 10:00 ` Jochen Mades @ 2022-01-12 1:41 ` Lino Sanfilippo 0 siblings, 0 replies; 10+ messages in thread From: Lino Sanfilippo @ 2022-01-12 1:41 UTC (permalink / raw) To: Jochen Mades, Lukas Wunner Cc: gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Philipp Rosenberger Hi Jochen, On 11.01.22 at 11:00, Jochen Mades wrote: > Hi Lukas, Lino, > > please let me know when I could test again with an "official" linux kernel, instead of using my local patch. > > Bests > Jochen While the fix itself looks good so far there are still some style issue as Lukas pointed out. These issues have to be fixed before the patch can be accepted for mainline integration. Please have a look at Documentation/process/submitting-patches.rst for the correct patch format. After these issues are fixed please send the patch as a v2, so we can review the new version. Best regards, Lino ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-02 10:07 ` Lukas Wunner 2022-01-02 15:06 ` Lino Sanfilippo @ 2022-01-13 10:12 ` Jochen Mades 2022-01-13 10:20 ` Greg KH 2022-01-23 4:32 ` Lukas Wunner 1 sibling, 2 replies; 10+ messages in thread From: Jochen Mades @ 2022-01-13 10:12 UTC (permalink / raw) To: Lukas Wunner Cc: gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Lino Sanfilippo, Philipp Rosenberger [-- Attachment #1: Type: text/plain, Size: 2190 bytes --] Hi Lukas, > Patch is correct, but commit message could be improved: > > * Subject should be in imperative mood (by convention), it should be > prepended by "serial: pl011: " (in line with previous commits touching > this driver, use "git log --oneline amba-pl011.c") and the trailing dot > is unnecessary, e.g.: > > "serial: pl011: Fix incorrect rs485 RTS polarity on set_mctrl" > > * Commit message should be wrapped at 72 characters (so that it appears > centered when displayed with "git log" on an 80 chars terminal). > The reference to "0001-serial-amba-pl011-add-RS485-support.patch" > should be replaced with a reference to the offending commit, e.g.: > > "Commit 8d479237727c ("serial: amba-pl011: add RS485 support") sought > to keep RTS deasserted on set_mctrl if rs485 is enabled. However it > did so only if deasserted RTS polarity is high. Fix it in case it's > low." > > Feel free to copy this to a v2 of your patch and amend as you see fit. > Find attached the patch with the new subject and corretced commit message. > * Add tags for the offending commit: > > Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support") > Cc: stable@vger.kernel.org # v5.15+ > > * Be sure to cc the author of the offending commit. Sorry I don't know how to do that correctly. Can you please give support/hints? > Thanks, > > Lukas > > > --- > > drivers/tty/serial/amba-pl011.c | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c > > index 537f37ac4..1749c1498 100644 > > --- a/drivers/tty/serial/amba-pl011.c > > +++ b/drivers/tty/serial/amba-pl011.c > > @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) > > container_of(port, struct uart_amba_port, port); > > unsigned int cr; > > > > - if (port->rs485.flags & SER_RS485_ENABLED) > > - mctrl &= ~TIOCM_RTS; > > + if (port->rs485.flags & SER_RS485_ENABLED) { > > + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > > + mctrl &= ~TIOCM_RTS; > > + else > > + mctrl |= TIOCM_RTS; > > + } > > > > cr = pl011_read(uap, REG_CR); [-- Attachment #2: 0001-serial-pl011-Fix-incorrect-rs485-RTS-polarity-on-set.patch --] [-- Type: application/octet-stream, Size: 1210 bytes --] From 90690b3e8afbdfbbb5ceb0f6cef40ed282b1c004 Mon Sep 17 00:00:00 2001 From: jmades <jochen@mades.net> Date: Mon, 27 Dec 2021 08:37:26 +0000 Subject: [PATCH] serial: pl011: Fix incorrect rs485 RTS polarity on set_mctrl Commit 8d479237727c ("serial: amba-pl011: add RS485 support") sought to keep RTS deasserted on set_mctrl if rs485 is enabled. However it did so only if deasserted RTS polarity is high. Fix it in case it's low. Signed-off-by: jmades <jochen@mades.net> --- drivers/tty/serial/amba-pl011.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 537f37ac4..1749c1498 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) container_of(port, struct uart_amba_port, port); unsigned int cr; - if (port->rs485.flags & SER_RS485_ENABLED) - mctrl &= ~TIOCM_RTS; + if (port->rs485.flags & SER_RS485_ENABLED) { + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) + mctrl &= ~TIOCM_RTS; + else + mctrl |= TIOCM_RTS; + } cr = pl011_read(uap, REG_CR); -- 2.20.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-13 10:12 ` Jochen Mades @ 2022-01-13 10:20 ` Greg KH 2022-01-23 4:32 ` Lukas Wunner 1 sibling, 0 replies; 10+ messages in thread From: Greg KH @ 2022-01-13 10:20 UTC (permalink / raw) To: Jochen Mades Cc: Lukas Wunner, Russell King, Jiri Slaby, linux-serial, linux-kernel, Lino Sanfilippo, Philipp Rosenberger On Thu, Jan 13, 2022 at 11:12:12AM +0100, Jochen Mades wrote: > Hi Lukas, > > > Patch is correct, but commit message could be improved: > > > > * Subject should be in imperative mood (by convention), it should be > > prepended by "serial: pl011: " (in line with previous commits touching > > this driver, use "git log --oneline amba-pl011.c") and the trailing dot > > is unnecessary, e.g.: > > > > "serial: pl011: Fix incorrect rs485 RTS polarity on set_mctrl" > > > > * Commit message should be wrapped at 72 characters (so that it appears > > centered when displayed with "git log" on an 80 chars terminal). > > The reference to "0001-serial-amba-pl011-add-RS485-support.patch" > > should be replaced with a reference to the offending commit, e.g.: > > > > "Commit 8d479237727c ("serial: amba-pl011: add RS485 support") sought > > to keep RTS deasserted on set_mctrl if rs485 is enabled. However it > > did so only if deasserted RTS polarity is high. Fix it in case it's > > low." > > > > Feel free to copy this to a v2 of your patch and amend as you see fit. > > > > Find attached the patch with the new subject and corretced commit message. > > > * Add tags for the offending commit: > > > > Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support") > > Cc: stable@vger.kernel.org # v5.15+ > > > > * Be sure to cc the author of the offending commit. > > Sorry I don't know how to do that correctly. Can you please give support/hints? > > > > Thanks, > > > > Lukas > > > > > --- > > > drivers/tty/serial/amba-pl011.c | 8 ++++++-- > > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c > > > index 537f37ac4..1749c1498 100644 > > > --- a/drivers/tty/serial/amba-pl011.c > > > +++ b/drivers/tty/serial/amba-pl011.c > > > @@ -1646,8 +1646,12 @@ static void pl011_set_mctrl(struct uart_port *port, unsigned int mctrl) > > > container_of(port, struct uart_amba_port, port); > > > unsigned int cr; > > > > > > - if (port->rs485.flags & SER_RS485_ENABLED) > > > - mctrl &= ~TIOCM_RTS; > > > + if (port->rs485.flags & SER_RS485_ENABLED) { > > > + if (port->rs485.flags & SER_RS485_RTS_AFTER_SEND) > > > + mctrl &= ~TIOCM_RTS; > > > + else > > > + mctrl |= TIOCM_RTS; > > > + } > > > > > > cr = pl011_read(uap, REG_CR); Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch was attached, please place it inline so that it can be applied directly from the email message itself. - It looks like you did not use your "real" name for the patch on either the Signed-off-by: line, or the From: line (both of which have to match). Please read the kernel file, Documentation/SubmittingPatches for how to do this correctly. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function. 2022-01-13 10:12 ` Jochen Mades 2022-01-13 10:20 ` Greg KH @ 2022-01-23 4:32 ` Lukas Wunner 1 sibling, 0 replies; 10+ messages in thread From: Lukas Wunner @ 2022-01-23 4:32 UTC (permalink / raw) To: Jochen Mades Cc: gregkh, Russell King, Jiri Slaby, linux-serial, linux-kernel, Lino Sanfilippo, Philipp Rosenberger On Thu, Jan 13, 2022 at 11:12:12AM +0100, Jochen Mades wrote: > Find attached the patch with the new subject and corretced commit message. Greg doesn't accept patches as attachments and during the merge window (which ends today). I've just resubmitted your patch as part of a 2-patch series, together with a patch of mine which goes on top of yours: https://lore.kernel.org/linux-serial/85fa3323ba8c307943969b7343e23f34c3e652ba.1642909284.git.lukas@wunner.de/ (I sent your patch twice because I botched the Date header the first time around, sorry about that.) Note that you still get patch authorship (and thus credit for the patch) even though I submitted it, due to the From: line at the top of the message body. > > * Add tags for the offending commit: > > > > Fixes: 8d479237727c ("serial: amba-pl011: add RS485 support") > > Cc: stable@vger.kernel.org # v5.15+ > > > > * Be sure to cc the author of the offending commit. > > Sorry I don't know how to do that correctly. Can you please give support/hints? It just means that you need to add the two above-quoted tags to the commit message. The maintainers of stable kernels use them to determine which patches need to be backported to which kernel releases. I've fixed up the commit message of your patch with the missing tags, no problem. And "cc the author of the offending commit" just means that when posting the patch, it is customary to send a copy of the e-mail to the author of the commit you're fixing, which is Lino in this case, as he authored 8d479237727c. Thanks, Lukas ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-01-23 4:33 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-31 17:15 [PATCH] Bugfix RTS line config in RS485 mode is overwritten in pl011_set_mctrl() function jmades 2022-01-02 10:07 ` Lukas Wunner 2022-01-02 15:06 ` Lino Sanfilippo 2022-01-02 18:28 ` Lukas Wunner 2022-01-03 13:44 ` Lino Sanfilippo 2022-01-11 10:00 ` Jochen Mades 2022-01-12 1:41 ` Lino Sanfilippo 2022-01-13 10:12 ` Jochen Mades 2022-01-13 10:20 ` Greg KH 2022-01-23 4:32 ` Lukas Wunner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.