From: Claudio Scordino <claudio@evidence.eu.com>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Greg KH <greg@kroah.com>,
linux-arm-kernel@lists.infradead.org, nicolas.ferre@atmel.com,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
Bernhard Roth <br@pwrnet.de>
Subject: Re: [PATCH] atmel_serial: RS485: receiving enabled when sending data
Date: Tue, 23 Aug 2011 12:06:43 +0200 [thread overview]
Message-ID: <4E537BB3.60001@evidence.eu.com> (raw)
In-Reply-To: <20110823093035.GB19622@n2100.arm.linux.org.uk>
Il 23/08/2011 11:30, Russell King - ARM Linux ha scritto:
> On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>> /* Disable interrupts */
>> UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> - if (atmel_port->rs485.flags& SER_RS485_ENABLED)
>> - atmel_start_rx(port);
>> + if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
>> + !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX))
>> + atmel_start_rx(port);
>
> However, this is incorrect formatting. The code following the 'if' ends
> up being indented by two tabs.
>
> This illustrates why the whole idea that tabs should only be used for
> indenting is wrong. You end up with either what you have above, or:
>
> if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
> !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX))
> atmel_start_rx(port);
>
> both of which are dire for readability. The only sane way to do this is
> to ignore that stupid "indentation by tabs only" thing and do it as the
> kernel code has _always_ been done over the last 19 years:
>
> if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
> !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX))
> atmel_start_rx(port);
>
> IOW, use spaces to align the wrapped 'if' statement.
>
>> + if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
>> + !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX)) {
>> + /* DMA done, stop TX, start RX for RS485 */
>> + atmel_start_rx(port);
>
> And here it makes it look like there's a missing brace.
>
I understand. So the right patch should be the following one.
Best regards,
Claudio
atmel_serial: RS485: receiving enabled when sending data
By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.
Signed-off-by: Bernhard Roth <br@pwrnet.de>
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
drivers/tty/serial/atmel_serial.c | 9 ++++++---
include/linux/serial.h | 1 +
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..c7232a9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,7 +339,8 @@ static void atmel_stop_tx(struct uart_port *port)
/* Disable interrupts */
UART_PUT_IDR(port, atmel_port->tx_done_mask);
- if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+ if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+ !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
atmel_start_rx(port);
}
@@ -356,7 +357,8 @@ static void atmel_start_tx(struct uart_port *port)
really need this.*/
return;
- if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+ if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+ !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
atmel_stop_rx(port);
/* re-enable PDC transmit */
@@ -680,7 +682,8 @@ static void atmel_tx_dma(struct uart_port *port)
/* Enable interrupts */
UART_PUT_IER(port, atmel_port->tx_done_mask);
} else {
- if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
+ if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+ !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
#define SER_RS485_RTS_ON_SEND (1 << 1)
#define SER_RS485_RTS_AFTER_SEND (1 << 2)
#define SER_RS485_RTS_BEFORE_SEND (1 << 3)
+#define SER_RS485_RX_DURING_TX (1 << 4)
__u32 delay_rts_before_send; /* Milliseconds */
__u32 delay_rts_after_send; /* Milliseconds */
__u32 padding[5]; /* Memory is cheap, new structs
--
1.7.1
WARNING: multiple messages have this Message-ID (diff)
From: claudio@evidence.eu.com (Claudio Scordino)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] atmel_serial: RS485: receiving enabled when sending data
Date: Tue, 23 Aug 2011 12:06:43 +0200 [thread overview]
Message-ID: <4E537BB3.60001@evidence.eu.com> (raw)
In-Reply-To: <20110823093035.GB19622@n2100.arm.linux.org.uk>
Il 23/08/2011 11:30, Russell King - ARM Linux ha scritto:
> On Tue, Aug 23, 2011 at 10:30:46AM +0200, Claudio Scordino wrote:
>> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
>> index af9b781..5f6c745 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -339,8 +339,9 @@ static void atmel_stop_tx(struct uart_port *port)
>> /* Disable interrupts */
>> UART_PUT_IDR(port, atmel_port->tx_done_mask);
>>
>> - if (atmel_port->rs485.flags& SER_RS485_ENABLED)
>> - atmel_start_rx(port);
>> + if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
>> + !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX))
>> + atmel_start_rx(port);
>
> However, this is incorrect formatting. The code following the 'if' ends
> up being indented by two tabs.
>
> This illustrates why the whole idea that tabs should only be used for
> indenting is wrong. You end up with either what you have above, or:
>
> if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
> !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX))
> atmel_start_rx(port);
>
> both of which are dire for readability. The only sane way to do this is
> to ignore that stupid "indentation by tabs only" thing and do it as the
> kernel code has _always_ been done over the last 19 years:
>
> if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
> !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX))
> atmel_start_rx(port);
>
> IOW, use spaces to align the wrapped 'if' statement.
>
>> + if ((atmel_port->rs485.flags& SER_RS485_ENABLED)&&
>> + !(atmel_port->rs485.flags& SER_RS485_RX_DURING_TX)) {
>> + /* DMA done, stop TX, start RX for RS485 */
>> + atmel_start_rx(port);
>
> And here it makes it look like there's a missing brace.
>
I understand. So the right patch should be the following one.
Best regards,
Claudio
atmel_serial: RS485: receiving enabled when sending data
By default the atmel_serial driver in RS485 mode disables receiving data until
all data in the send buffer has been sent. This flag allows to receive data
even whilst sending data.
Signed-off-by: Bernhard Roth <br@pwrnet.de>
Signed-off-by: Claudio Scordino <claudio@evidence.eu.com>
---
drivers/tty/serial/atmel_serial.c | 9 ++++++---
include/linux/serial.h | 1 +
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index af9b781..c7232a9 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -339,7 +339,8 @@ static void atmel_stop_tx(struct uart_port *port)
/* Disable interrupts */
UART_PUT_IDR(port, atmel_port->tx_done_mask);
- if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+ if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+ !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
atmel_start_rx(port);
}
@@ -356,7 +357,8 @@ static void atmel_start_tx(struct uart_port *port)
really need this.*/
return;
- if (atmel_port->rs485.flags & SER_RS485_ENABLED)
+ if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+ !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX))
atmel_stop_rx(port);
/* re-enable PDC transmit */
@@ -680,7 +682,8 @@ static void atmel_tx_dma(struct uart_port *port)
/* Enable interrupts */
UART_PUT_IER(port, atmel_port->tx_done_mask);
} else {
- if (atmel_port->rs485.flags & SER_RS485_ENABLED) {
+ if ((atmel_port->rs485.flags & SER_RS485_ENABLED) &&
+ !(atmel_port->rs485.flags & SER_RS485_RX_DURING_TX)) {
/* DMA done, stop TX, start RX for RS485 */
atmel_start_rx(port);
}
diff --git a/include/linux/serial.h b/include/linux/serial.h
index ef91406..97ff8e2 100644
--- a/include/linux/serial.h
+++ b/include/linux/serial.h
@@ -211,6 +211,7 @@ struct serial_rs485 {
#define SER_RS485_RTS_ON_SEND (1 << 1)
#define SER_RS485_RTS_AFTER_SEND (1 << 2)
#define SER_RS485_RTS_BEFORE_SEND (1 << 3)
+#define SER_RS485_RX_DURING_TX (1 << 4)
__u32 delay_rts_before_send; /* Milliseconds */
__u32 delay_rts_after_send; /* Milliseconds */
__u32 padding[5]; /* Memory is cheap, new structs
--
1.7.1
next prev parent reply other threads:[~2011-08-23 10:06 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-15 14:28 [PATCH] atmel_serial: RS485: receiving enabled when sending data Bernhard Roth
2011-08-15 14:28 ` Bernhard Roth
2011-08-22 21:18 ` Greg KH
2011-08-22 21:18 ` Greg KH
2011-08-23 8:30 ` Claudio Scordino
2011-08-23 8:30 ` Claudio Scordino
2011-08-23 8:30 ` Claudio Scordino
2011-08-23 9:30 ` Russell King - ARM Linux
2011-08-23 9:30 ` Russell King - ARM Linux
2011-08-23 9:30 ` Russell King - ARM Linux
2011-08-23 10:06 ` Claudio Scordino [this message]
2011-08-23 10:06 ` Claudio Scordino
2011-08-23 10:14 ` Alan Cox
2011-08-23 10:14 ` Alan Cox
2011-08-23 10:21 ` Russell King - ARM Linux
2011-08-23 10:21 ` Russell King - ARM Linux
2011-08-23 10:21 ` Russell King - ARM Linux
2011-08-23 15:39 ` Greg KH
2011-08-23 15:39 ` Greg KH
2011-08-23 15:39 ` Greg KH
2011-08-24 7:48 ` Claudio Scordino
2011-08-24 7:48 ` Claudio Scordino
2011-11-04 8:19 ` [PATCH] RS485: fix inconsistencies in the meaning of some variables Claudio Scordino
2011-11-04 8:19 ` Claudio Scordino
2011-11-04 10:36 ` Jesper Nilsson
2011-11-04 10:36 ` Jesper Nilsson
2011-11-04 10:36 ` Jesper Nilsson
2011-11-08 9:30 ` Nicolas Ferre
2011-11-08 9:30 ` Nicolas Ferre
2011-11-08 9:59 ` Jean-Christophe PLAGNIOL-VILLARD
2011-11-08 9:59 ` Jean-Christophe PLAGNIOL-VILLARD
2011-11-08 9:59 ` Jean-Christophe PLAGNIOL-VILLARD
2011-11-08 10:48 ` Claudio Scordino
2011-11-08 10:48 ` Claudio Scordino
2011-11-08 13:48 ` Alan Cox
2011-11-08 13:48 ` Alan Cox
2011-11-08 14:24 ` Greg KH
2011-11-08 14:24 ` Greg KH
2011-11-09 14:51 ` Claudio Scordino
2011-11-09 14:51 ` Claudio Scordino
2011-11-13 21:53 ` Wolfram Sang
2011-11-13 21:53 ` Wolfram Sang
2011-11-14 0:37 ` Darron Black
2011-11-14 0:37 ` Darron Black
2011-11-14 0:37 ` Darron Black
2011-11-14 11:11 ` Nicolas Ferre
2011-11-14 11:11 ` Nicolas Ferre
2011-11-14 12:07 ` Alan Cox
2011-11-14 12:07 ` Alan Cox
2011-11-14 8:22 ` Claudio Scordino
2011-11-14 8:22 ` Claudio Scordino
2011-11-14 12:18 ` Nicolas Ferre
2011-11-14 12:18 ` Nicolas Ferre
2011-11-08 15:02 ` Nicolas Ferre
2011-11-08 15:02 ` Nicolas Ferre
2011-11-08 15:45 ` Claudio Scordino
2011-11-08 15:45 ` Claudio Scordino
2011-11-08 16:34 ` Jesper Nilsson
2011-11-08 16:34 ` Jesper Nilsson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4E537BB3.60001@evidence.eu.com \
--to=claudio@evidence.eu.com \
--cc=br@pwrnet.de \
--cc=greg@kroah.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=nicolas.ferre@atmel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.