On Thu, Apr 12, 2018 at 11:24:07AM -0400, Mauro Carvalho Chehab wrote:
> As warned by smatch:
> drivers/media/rc/st_rc.c:110 st_rc_rx_interrupt() warn: this loop depends on readl() succeeding
>
> If something goes wrong at readl(), the logic will stay there
> inside an IRQ code forever. This is not the nicest thing to
> do :-)
>
> So, add a timeout there, preventing staying inside the IRQ
> for more than 10ms.
If we knew how large the fifo was, then we could limit the loop to that many
iterations (maybe a few extra in case IR arrives while we a reading, but
IR is much slower than a cpu executing this loop of course).
Patrice is something you could help with?
Thanks
Sean
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxxx>
> ---
> drivers/media/rc/st_rc.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/media/rc/st_rc.c b/drivers/media/rc/st_rc.c
> index d2efd7b2c3bc..c855b177103c 100644
> --- a/drivers/media/rc/st_rc.c
> +++ b/drivers/media/rc/st_rc.c
> @@ -96,19 +96,24 @@ static void st_rc_send_lirc_timeout(struct rc_dev *rdev)
>
> static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
> {
> + unsigned long timeout;
> unsigned int symbol, mark = 0;
> struct st_rc_device *dev = data;
> int last_symbol = 0;
> - u32 status;
> + u32 status, int_status;
> DEFINE_IR_RAW_EVENT(ev);
>
> if (dev->irq_wake)
> pm_wakeup_event(dev->dev, 0);
>
> - status = readl(dev->rx_base + IRB_RX_STATUS);
> + /* FIXME: is 10ms good enough ? */
> + timeout = jiffies + msecs_to_jiffies(10);
> + do {
> + status = readl(dev->rx_base + IRB_RX_STATUS);
> + if (!(status & (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW)))
> + break;
>
> - while (status & (IRB_FIFO_NOT_EMPTY | IRB_OVERFLOW)) {
> - u32 int_status = readl(dev->rx_base + IRB_RX_INT_STATUS);
> + int_status = readl(dev->rx_base + IRB_RX_INT_STATUS);
> if (unlikely(int_status & IRB_RX_OVERRUN_INT)) {
> /* discard the entire collection in case of errors! */
> ir_raw_event_reset(dev->rdev);
> @@ -148,8 +153,7 @@ static irqreturn_t st_rc_rx_interrupt(int irq, void *data)
>
> }
> last_symbol = 0;
> - status = readl(dev->rx_base + IRB_RX_STATUS);
> - }
> + } while (time_is_after_jiffies(timeout));
>
> writel(IRB_RX_INTS, dev->rx_base + IRB_RX_INT_CLEAR);
>
> --
> 2.14.3
