On Wed, 24 Nov 2021 at 04:07, Julien Massot <julien.massot@xxxxxxx> wrote:
>
> Hi Mathieu,
> Thanks for the review !
>
> >> +config RCAR_REMOTEPROC
> >> + tristate "Renesas R-CAR Gen3 remoteproc support"
> >> + depends on ARCH_RENESAS
> >> + depends on REMOTEPROC
> >
> > You should be able to remove the dependency on REMOTEPROC since this is already in
> > the "if REMOTEPROC" block.
> Will fix.
>
> ...
> >
> >> +
> >> + dev_dbg(dev, "map memory: %pa+%lx\n", &mem->dma, mem->len);
> >> + va = ioremap_wc(mem->dma, mem->len);
> >> + if (IS_ERR_OR_NULL(va)) {
> >> + dev_err(dev, "Unable to map memory region: %pa+%lx\n",
> >
> > The sparse checker doesn't like %lx so probably be better to go with just %x.
> > Apologies for suggesting to use %lx.
>
> With %x gcc complains on arm64 build will go back to %zx.
Ok
>
> >
> >> + &mem->dma, mem->len);
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + /* Update memory entry va */
> >> + mem->va = va;
> >
> > Talking about the sparse checker, you will see complaints about @va not being of
> > type "void __iomem *". You can ignore those as this would likely require to
> > refactor the rproc_mem_entry structure, which is outside the scope of this work.
>
> Ok, to be honest, I was not aware of the sparse tool, thanks a lot to point me to
> this tool.
>
> >
> > This set is just as clean as the RFC. If it wasn't for the DTS bindings that
> > need to be ack'ed by Rob, I probably would have made the above modifications and
> > applied this patch.
> >
> > Thanks,
> > Mathieu
>
> No problem will send a v2.
>
> Regards,
> Julien
> --
> Julien Massot [IoT.bzh]
>
