| Message ID | 20230608154129.133169-1-martin@kaiser.cx |
|---|---|
| State | New |
| Headers | show |
| Series | imx_serial: set wake bit when we receive a data byte | expand |
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; ---
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; > ---
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); }
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(+)