Hi Geert,
Thank you for the quick reply.
On Fri, Nov 25, 2016 at 9:47 AM, Geert Uytterhoeven
<geert@xxxxxxxxxxxxxx> wrote:
> Yes, that's a bug. Fortunately it only happens inside an error path, for
> a case that shouldn't really happen.
>
> Do you have a suggestion how to fix this?
It does not seem trivial to fix, and I'm not familiar with this code.
Does function `sci_submit_rx' must be called with `port->lock' held?
(It writes to some fields of the struct `s' that seem to be protected
by that lock.) If so, this perhaps could be fixed by requiring
`sci_rx_dma_release' to be called with `port->lock' held, and checking
that all the callers indeed do that. There are only three callers of
this function.
But if this error path is not even supposed to happen then I could
suggest adding just a warning:
fail:
+ struct uart_port *port = &s->port;
if (i)
dmaengine_terminate_all(chan);
for (i = 0; i < 2; i++)
s->cookie_rx[i] = -EINVAL;
s->active_rx = -EINVAL;
+ /* Should not call sci_rx_dma_release with port->lock held. */
+ WARN_ON(spin_is_locked(&port->lock))
dev_warn(s->port.dev, "Failed to re-start Rx DMA, using PIO\n");
sci_rx_dma_release(s, true);
}
Just in case one day the impossible happens. We could also release
Iago
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
