* [PATCH] imx_serial: set wake bit when we receive a data byte
@ 2023-06-08 15:41 Martin Kaiser
2023-06-12 13:23 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Martin Kaiser @ 2023-06-08 15:41 UTC (permalink / raw)
To: Peter Maydell, Marc-André Lureau, Paolo Bonzini
Cc: qemu-arm, qemu-devel, Martin Kaiser
The linux kernel added a flood check for rx data recently in commmit
496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
check uses the wake bit in the uart status register 2. The wake bit
indicates that the receiver detected a start bit. If the kernel sees a
number of rx interrupts without the wake bit being set, it treats this as
spurious data and resets the uart port. imx_serial does never set the
wake bit and triggers the kernel's flood check.
This patch adds support for the wake bit. wake is set when we receive a
new character (it's not set for break events). It seems that wake is
cleared by the kernel driver, the hardware does not have to clear it
automatically after data was read.
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
hw/char/imx_serial.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index ee1375e26d..44125d5f47 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -321,6 +321,9 @@ static void imx_put_data(void *opaque, uint32_t value)
static void imx_receive(void *opaque, const uint8_t *buf, int size)
{
+ IMXSerialState *s = (IMXSerialState *)opaque;
+
+ s->usr2 |= USR2_WAKE;
imx_put_data(opaque, *buf);
}
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] imx_serial: set wake bit when we receive a data byte
2023-06-08 15:41 [PATCH] imx_serial: set wake bit when we receive a data byte Martin Kaiser
@ 2023-06-12 13:23 ` Philippe Mathieu-Daudé
2023-06-15 9:27 ` Martin Kaiser
2023-06-15 9:30 ` [PATCH v2] " Martin Kaiser
2023-06-15 14:22 ` [PATCH v3] " Martin Kaiser
2 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-12 13:23 UTC (permalink / raw)
To: Martin Kaiser, Peter Maydell, Marc-André Lureau, Paolo Bonzini
Cc: qemu-arm, qemu-devel
Hi Martin,
On 8/6/23 17:41, Martin Kaiser wrote:
> The linux kernel added a flood check for rx data recently in commmit
> 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
> check uses the wake bit in the uart status register 2. The wake bit
> indicates that the receiver detected a start bit. If the kernel sees a
> number of rx interrupts without the wake bit being set, it treats this as
> spurious data and resets the uart port. imx_serial does never set the
> wake bit and triggers the kernel's flood check.
>
> This patch adds support for the wake bit. wake is set when we receive a
> new character (it's not set for break events). It seems that wake is
> cleared by the kernel driver, the hardware does not have to clear it
> automatically after data was read.
>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> hw/char/imx_serial.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
> index ee1375e26d..44125d5f47 100644
> --- a/hw/char/imx_serial.c
> +++ b/hw/char/imx_serial.c
> @@ -321,6 +321,9 @@ static void imx_put_data(void *opaque, uint32_t value)
>
> static void imx_receive(void *opaque, const uint8_t *buf, int size)
> {
> + IMXSerialState *s = (IMXSerialState *)opaque;
> +
> + s->usr2 |= USR2_WAKE;
> imx_put_data(opaque, *buf);
> }
>
Shouldn't we mask this bit for interruptions now?
-- >8 --
diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index 91c9894ad5..b823f94519 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL)
#define UCR4_DREN BIT(0) /* Receive Data Ready interrupt
enable */
#define UCR4_TCEN BIT(3) /* TX complete interrupt enable */
+#define UCR4_WKEN BIT(7) /* WAKE interrupt enable */
#define UTS1_TXEMPTY (1<<6)
#define UTS1_RXEMPTY (1<<5)
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index ee1375e26d..c8ec247350 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s)
* TCEN and TXDC are both bit 3
* RDR and DREN are both bit 0
*/
- mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN);
+ mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN);
usr2 = s->usr2 & mask;
---
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] imx_serial: set wake bit when we receive a data byte
2023-06-12 13:23 ` Philippe Mathieu-Daudé
@ 2023-06-15 9:27 ` Martin Kaiser
0 siblings, 0 replies; 8+ messages in thread
From: Martin Kaiser @ 2023-06-15 9:27 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, Marc-André Lureau, Paolo Bonzini, qemu-arm,
qemu-devel
Hi Philippe,
thanks for reviewing my patch.
Philippe Mathieu-Daudé (philmd@linaro.org) wrote:
> Shouldn't we mask this bit for interruptions now?
yes, we should support interrupts from the wake bit. I'll add your
snippet and send a v2.
Thanks,
Martin
> -- >8 --
> diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
> index 91c9894ad5..b823f94519 100644
> --- a/include/hw/char/imx_serial.h
> +++ b/include/hw/char/imx_serial.h
> @@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL)
> #define UCR4_DREN BIT(0) /* Receive Data Ready interrupt enable */
> #define UCR4_TCEN BIT(3) /* TX complete interrupt enable */
> +#define UCR4_WKEN BIT(7) /* WAKE interrupt enable */
> #define UTS1_TXEMPTY (1<<6)
> #define UTS1_RXEMPTY (1<<5)
> diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
> index ee1375e26d..c8ec247350 100644
> --- a/hw/char/imx_serial.c
> +++ b/hw/char/imx_serial.c
> @@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s)
> * TCEN and TXDC are both bit 3
> * RDR and DREN are both bit 0
> */
> - mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN);
> + mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN);
> usr2 = s->usr2 & mask;
> ---
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] imx_serial: set wake bit when we receive a data byte
2023-06-08 15:41 [PATCH] imx_serial: set wake bit when we receive a data byte Martin Kaiser
2023-06-12 13:23 ` Philippe Mathieu-Daudé
@ 2023-06-15 9:30 ` Martin Kaiser
2023-06-15 10:25 ` Philippe Mathieu-Daudé
2023-06-15 14:22 ` [PATCH v3] " Martin Kaiser
2 siblings, 1 reply; 8+ messages in thread
From: Martin Kaiser @ 2023-06-15 9:30 UTC (permalink / raw)
To: Peter Maydell, Philippe Mathieu-Daudé,
Marc-André Lureau, Paolo Bonzini
Cc: qemu-arm, qemu-devel, Martin Kaiser
The linux kernel added a flood check for rx data recently in commmit
496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
check uses the wake bit in the uart status register 2. The wake bit
indicates that the receiver detected a start bit on the rx line. If the
kernel sees a number of rx interrupts without the wake bit being set, it
treats this as spurious data and resets the uart port. imx_serial does
never set the wake bit and triggers the kernel's flood check.
This patch adds support for the wake bit. wake is set when we receive a
new character (it's not set for break events). It seems that wake is
cleared by the kernel driver, the hardware does not have to clear it
automatically after data was read.
The wake bit can be configured as an interrupt source. Support this
mechanism as well.
Co-developed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
- support interrupts from wake
- clean up the commit message
hw/char/imx_serial.c | 5 ++++-
include/hw/char/imx_serial.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index ee1375e26d..1b75a89588 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s)
* TCEN and TXDC are both bit 3
* RDR and DREN are both bit 0
*/
- mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN);
+ mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN);
usr2 = s->usr2 & mask;
@@ -321,6 +321,9 @@ static void imx_put_data(void *opaque, uint32_t value)
static void imx_receive(void *opaque, const uint8_t *buf, int size)
{
+ IMXSerialState *s = (IMXSerialState *)opaque;
+
+ s->usr2 |= USR2_WAKE;
imx_put_data(opaque, *buf);
}
diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index 91c9894ad5..b823f94519 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL)
#define UCR4_DREN BIT(0) /* Receive Data Ready interrupt enable */
#define UCR4_TCEN BIT(3) /* TX complete interrupt enable */
+#define UCR4_WKEN BIT(7) /* WAKE interrupt enable */
#define UTS1_TXEMPTY (1<<6)
#define UTS1_RXEMPTY (1<<5)
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] imx_serial: set wake bit when we receive a data byte
2023-06-15 9:30 ` [PATCH v2] " Martin Kaiser
@ 2023-06-15 10:25 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-15 10:25 UTC (permalink / raw)
To: Martin Kaiser, Peter Maydell, Marc-André Lureau, Paolo Bonzini
Cc: qemu-arm, qemu-devel
On 15/6/23 11:30, Martin Kaiser wrote:
> The linux kernel added a flood check for rx data recently in commmit
"Linux", "commit"
Also maybe s/rx/RX/ s/uart/UART/.
> 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
> check uses the wake bit in the uart status register 2. The wake bit
> indicates that the receiver detected a start bit on the rx line. If the
> kernel sees a number of rx interrupts without the wake bit being set, it
> treats this as spurious data and resets the uart port. imx_serial does
> never set the wake bit and triggers the kernel's flood check.
>
> This patch adds support for the wake bit. wake is set when we receive a
> new character (it's not set for break events). It seems that wake is
> cleared by the kernel driver, the hardware does not have to clear it
> automatically after data was read.
>
> The wake bit can be configured as an interrupt source. Support this
> mechanism as well.
>
> Co-developed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thanks ;)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> v2:
> - support interrupts from wake
> - clean up the commit message
>
> hw/char/imx_serial.c | 5 ++++-
> include/hw/char/imx_serial.h | 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] imx_serial: set wake bit when we receive a data byte
2023-06-08 15:41 [PATCH] imx_serial: set wake bit when we receive a data byte Martin Kaiser
2023-06-12 13:23 ` Philippe Mathieu-Daudé
2023-06-15 9:30 ` [PATCH v2] " Martin Kaiser
@ 2023-06-15 14:22 ` Martin Kaiser
2023-06-19 10:36 ` Peter Maydell
2 siblings, 1 reply; 8+ messages in thread
From: Martin Kaiser @ 2023-06-15 14:22 UTC (permalink / raw)
To: Peter Maydell, Philippe Mathieu-Daudé,
Marc-André Lureau, Paolo Bonzini
Cc: qemu-arm, qemu-devel, Martin Kaiser
The Linux kernel added a flood check for RX data recently in commit
496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
check uses the wake bit in the UART status register 2. The wake bit
indicates that the receiver detected a start bit on the RX line. If the
kernel sees a number of RX interrupts without the wake bit being set, it
treats this as spurious data and resets the UART port. imx_serial does
never set the wake bit and triggers the kernel's flood check.
This patch adds support for the wake bit. wake is set when we receive a
new character (it's not set for break events). It seems that wake is
cleared by the kernel driver, the hardware does not have to clear it
automatically after data was read.
The wake bit can be configured as an interrupt source. Support this
mechanism as well.
Co-developed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v3:
- fix some spelling mistakes in the commit message
- add Philippe's Reviewed-by
v2:
- support interrupts from wake
- clean up the commit message
hw/char/imx_serial.c | 5 ++++-
include/hw/char/imx_serial.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c
index ee1375e26d..1b75a89588 100644
--- a/hw/char/imx_serial.c
+++ b/hw/char/imx_serial.c
@@ -80,7 +80,7 @@ static void imx_update(IMXSerialState *s)
* TCEN and TXDC are both bit 3
* RDR and DREN are both bit 0
*/
- mask |= s->ucr4 & (UCR4_TCEN | UCR4_DREN);
+ mask |= s->ucr4 & (UCR4_WKEN | UCR4_TCEN | UCR4_DREN);
usr2 = s->usr2 & mask;
@@ -321,6 +321,9 @@ static void imx_put_data(void *opaque, uint32_t value)
static void imx_receive(void *opaque, const uint8_t *buf, int size)
{
+ IMXSerialState *s = (IMXSerialState *)opaque;
+
+ s->usr2 |= USR2_WAKE;
imx_put_data(opaque, *buf);
}
diff --git a/include/hw/char/imx_serial.h b/include/hw/char/imx_serial.h
index 91c9894ad5..b823f94519 100644
--- a/include/hw/char/imx_serial.h
+++ b/include/hw/char/imx_serial.h
@@ -71,6 +71,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(IMXSerialState, IMX_SERIAL)
#define UCR4_DREN BIT(0) /* Receive Data Ready interrupt enable */
#define UCR4_TCEN BIT(3) /* TX complete interrupt enable */
+#define UCR4_WKEN BIT(7) /* WAKE interrupt enable */
#define UTS1_TXEMPTY (1<<6)
#define UTS1_RXEMPTY (1<<5)
--
2.30.2
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] imx_serial: set wake bit when we receive a data byte
2023-06-15 14:22 ` [PATCH v3] " Martin Kaiser
@ 2023-06-19 10:36 ` Peter Maydell
2023-06-19 13:09 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 8+ messages in thread
From: Peter Maydell @ 2023-06-19 10:36 UTC (permalink / raw)
To: Martin Kaiser
Cc: Philippe Mathieu-Daudé,
Marc-André Lureau, Paolo Bonzini, qemu-arm, qemu-devel
On Thu, 15 Jun 2023 at 15:24, Martin Kaiser <martin@kaiser.cx> wrote:
>
> The Linux kernel added a flood check for RX data recently in commit
> 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
> check uses the wake bit in the UART status register 2. The wake bit
> indicates that the receiver detected a start bit on the RX line. If the
> kernel sees a number of RX interrupts without the wake bit being set, it
> treats this as spurious data and resets the UART port. imx_serial does
> never set the wake bit and triggers the kernel's flood check.
>
> This patch adds support for the wake bit. wake is set when we receive a
> new character (it's not set for break events). It seems that wake is
> cleared by the kernel driver, the hardware does not have to clear it
> automatically after data was read.
>
> The wake bit can be configured as an interrupt source. Support this
> mechanism as well.
>
> Co-developed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
Applied to target-arm.next, thanks.
-- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] imx_serial: set wake bit when we receive a data byte
2023-06-19 10:36 ` Peter Maydell
@ 2023-06-19 13:09 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-06-19 13:09 UTC (permalink / raw)
To: Peter Maydell, Martin Kaiser
Cc: Marc-André Lureau, Paolo Bonzini, qemu-arm, qemu-devel
On 19/6/23 12:36, Peter Maydell wrote:
> On Thu, 15 Jun 2023 at 15:24, Martin Kaiser <martin@kaiser.cx> wrote:
>>
>> The Linux kernel added a flood check for RX data recently in commit
>> 496a4471b7c3 ("serial: imx: work-around for hardware RX flood"). This
>> check uses the wake bit in the UART status register 2. The wake bit
>> indicates that the receiver detected a start bit on the RX line. If the
>> kernel sees a number of RX interrupts without the wake bit being set, it
>> treats this as spurious data and resets the UART port. imx_serial does
>> never set the wake bit and triggers the kernel's flood check.
>>
>> This patch adds support for the wake bit. wake is set when we receive a
>> new character (it's not set for break events). It seems that wake is
>> cleared by the kernel driver, the hardware does not have to clear it
>> automatically after data was read.
>>
>> The wake bit can be configured as an interrupt source. Support this
>> mechanism as well.
>>
>> Co-developed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
>
>
>
> Applied to target-arm.next, thanks.
>
> -- PMM
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-19 13:10 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-08 15:41 [PATCH] imx_serial: set wake bit when we receive a data byte Martin Kaiser
2023-06-12 13:23 ` Philippe Mathieu-Daudé
2023-06-15 9:27 ` Martin Kaiser
2023-06-15 9:30 ` [PATCH v2] " Martin Kaiser
2023-06-15 10:25 ` Philippe Mathieu-Daudé
2023-06-15 14:22 ` [PATCH v3] " Martin Kaiser
2023-06-19 10:36 ` Peter Maydell
2023-06-19 13:09 ` Philippe Mathieu-Daudé
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.