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 <[email protected]>

> 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 <[email protected]>
> ---
>  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
> 

Reply via email to