RE: RE: [PATCH v14 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver

[Ben Levinsky]

Hi Michael, Punit,

> -----Original Message-----
> From: Michael Auchter <michael.auchter@xxxxxx>
> Sent: Friday, September 18, 2020 9:07 AM
> To: Ben Levinsky <BLEVINSK@xxxxxxxxxx>
> Cc: devicetree@xxxxxxxxxxxxxxx; linux-remoteproc@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: RE: [PATCH v14 5/5] remoteproc: Add initial zynqmp R5
> remoteproc driver
> 
> On Thu, Sep 17, 2020 at 10:50:42PM +0000, Ben Levinsky wrote:
> > In addition to device tree, is there particular linker script you use
> > for your R5 application? For example with OCM? As presently this
> > driver only has DDR and TCM as supported regions to load into
> 
> The firmware is being loaded to TCM.
> 
> I'm able to use this driver to load and run my firmware on both R5
> cores, but only after I change the incorrect:
> 
> 	rpu_mode = lockstep_mode
> 
> assignment to:
> 
> 	rpu_mode = lockstep_mode ? PM_RPU_MODE_LOCKSTEP
> 				 : PM_RPU_MODE_SPLIT;
There was a point raised by Punit that as "it is possible to set R5 to operatore in split or lock-step mode dynamically" which is true and can be done via sysfs and the Xilinx firmware kernel code. A suggestion that might clean up the driver so that the whole rpu_mode, tcm_mode configuration can be simplified and pulled out of the driver:
- as Punit suggested, remove the lockstep-mode property
- the zynqmp_remoteproc_r5 driver ONLY loads firmware and does start/stop.
- the zynqmp_remoteproc_r5 driver does not configure and memory regions or the RPU. Let the Xilinx firmware sysfs interface handle this.

Few advantages to this:
1. no extra configuration code in the zynqmp r5 remoteproc probe() for either R5 in the RPU cluster
2. less state to manage in the remoteproc driver
3. simpler documentation in the device tree binding

Again thank you both for the thoughtful review comments on this
Thanks
Ben