Re: [PATCH 2/4] remoteproc: introduce version element into resource type field

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Suman Anna <s-anna@ti.com>
Cc: Rob Herring <robh+dt@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Clement Leger <cleger@kalray.eu>,
	Loic Pallardy <loic.pallardy@st.com>,
	Arnaud Pouliquen <arnaud.pouliquen@st.com>,
	Lokesh Vutla <lokeshvutla@ti.com>,
	linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/4] remoteproc: introduce version element into resource type field
Date: Thu, 21 May 2020 12:21:46 -0700	[thread overview]
Message-ID: <20200521192146.GO408178@builder.lan> (raw)
In-Reply-To: <b338480e-c586-f988-f5b6-784551b7beb6@ti.com>

On Thu 21 May 12:06 PDT 2020, Suman Anna wrote:

> Hi Bjorn,
> 
> On 5/21/20 12:54 PM, Bjorn Andersson wrote:
> > On Wed 25 Mar 13:46 PDT 2020, Suman Anna wrote:
> > 
> > > The current remoteproc core has supported only 32-bit remote
> > > processors and as such some of the current resource structures
> > > may not scale well for 64-bit remote processors, and would
> > > require new versions of resource types. Each resource is currently
> > > identified by a 32-bit type field. Introduce the concept of version
> > > for these resource types by overloading this 32-bit type field
> > > into two 16-bit version and type fields with the existing resources
> > > behaving as version 0 thereby providing backward compatibility.
> > > 
> > > The version field is passed as an additional argument to each of
> > > the handler functions, and all the existing handlers are updated
> > > accordingly. Each specific handler will be updated on a need basis
> > > when a new version of the resource type is added.
> > > 
> > 
> > I really would prefer that we add additional types for the new
> > structures, neither side will be compatible with new versions without
> > enhancements to their respective implementations anyways.
> 
> OK.
> 
> > 
> > > An alternate way would be to introduce the new types as completely
> > > new resource types which would require additional customization of
> > > the resource handlers based on the 32-bit or 64-bit mode of a remote
> > > processor, and introduction of an additional mode flag to the rproc
> > > structure.
> > > 
> > 
> > What would this "mode" indicate? If it's version 0 or 1?
> 
> No, for indicating if the remoteproc is 32-bit or 64-bit and adjust the
> loading handlers if the resource types need to be segregated accordingly.
> 

Sorry, I think I'm misunderstanding something. Wouldn't your 64-bit
remote processor need different firmware from your 32-bit processor
anyways, if you want to support the wider resource? And you would pack
your firmware with the appropriate resource types?

Afaict the bit width of your remote processor, busses or memory is
unrelated to the choice of number of bits used to express things in the
resource table.

Regards,
Bjorn