Re: [PATCH v2 1/2] remoteproc: elf_loader: Create new callback for copying segments to remote cpu

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Tue, Nov 03, 2026 at 06:49:59PM +0100, Uwe Kleine-König wrote:
> Hello Mathieu,
> 
> On Tue, Nov 03, 2026 at 09:40:22AM -0600, Mathieu Poirier wrote:
> > On Tue, Oct 27, 2026 at 05:25:20PM +0100, Uwe Kleine-König wrote:
> > > diff --git a/drivers/remoteproc/remoteproc_elf_loader.c b/drivers/remoteproc/remoteproc_elf_loader.c
> > > index da3cddbe7d4c..3c749a39e5da 100644
> > > --- a/drivers/remoteproc/remoteproc_elf_loader.c
> > > +++ b/drivers/remoteproc/remoteproc_elf_loader.c
> > > @@ -227,35 +227,45 @@ int rproc_elf_load_segments(struct rproc *rproc, const struct firmware *fw)
> > >  			break;
> > >  		}
> > >  
> > > -		/* grab the kernel address for this device address */
> > > -		ptr = rproc_da_to_va(rproc, da, memsz, &is_iomem);
> > > -		if (!ptr) {
> > > -			dev_err(dev, "bad phdr da 0x%llx mem 0x%llx\n", da,
> > > -				memsz);
> > > -			ret = -EINVAL;
> > > -			break;
> > > -		}
> > > +		if (rproc->ops->memcpy_tova) {
> > > +			ret = rproc->ops->memcpy_tova(rproc, da, elf_data + offset, filesz, memsz);
> > > +			if (ret) {
> > > +				dev_err(dev, "Failed to copy segment %d to device address 0x%llx: %pe\n",
> > > +					i, da, ERR_PTR(ret));
> > > +				break;
> > > +			}
> > 
> > I really like your patch, but I'm wondering why you decided to move
> > rproc_da_to_va() in ops->memcpy_tova().  Right now it is called in both path,
> > i.e with and without ops->memcpy_tova().  Wouldn't it be better to
> > unconditionally leave rproc_da_to_va() in rproc_elf_load_segments()?
> 
> The reason is that on ADI's SC846 the memory for the remote processor
> isn't directly accessible by Linux. So there is no virtual address for
> the given device address and thus the implementation of .memcpy_tova()
> on that machine won't use rproc_da_to_va(). 

That is a valid use case - please add something to that effect in the
changelog.

> 
> Best regards
> Uwe