On Tue, Mar 23, 2021 at 05:38:38PM -0500, Suman Anna wrote:
> The irq_create_fwspec_mapping() returns a proper virq value on success
> and 0 upon any failure. The pru_handle_intrmap() treats this as an error
> and disposes all firmware event mappings correctly, but is returning
> this incorrect value as is, letting the pru_rproc_start() interpret it
> as a success and boot the PRU.
>
Very subtle... I had to look twice to make sure.
Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> Fix this by returning an error value back upon any such failure. While
> at this, revise the error trace to print some meaningful info about the
> failed event.
>
> Fixes: c75c9fdac66e ("remoteproc: pru: Add support for PRU specific interrupt configuration")
> Signed-off-by: Suman Anna <s-anna@xxxxxx>
> ---
> drivers/remoteproc/pru_rproc.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/remoteproc/pru_rproc.c b/drivers/remoteproc/pru_rproc.c
> index a9d07c0751be..87b43976c51b 100644
> --- a/drivers/remoteproc/pru_rproc.c
> +++ b/drivers/remoteproc/pru_rproc.c
> @@ -339,8 +339,10 @@ static int pru_handle_intrmap(struct rproc *rproc)
>
> pru->mapped_irq[i] = irq_create_fwspec_mapping(&fwspec);
> if (!pru->mapped_irq[i]) {
> - dev_err(dev, "failed to get virq\n");
> - ret = pru->mapped_irq[i];
> + dev_err(dev, "failed to get virq for fw mapping %d: event %d chnl %d host %d\n",
> + i, fwspec.param[0], fwspec.param[1],
> + fwspec.param[2]);
> + ret = -EINVAL;
> goto map_fail;
> }
> }
> --
> 2.30.1
>
