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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.


Signed-off-by: Suman Anna <s-anna@xxxxxx>
---
  drivers/remoteproc/remoteproc_core.c    | 25 +++++++++++++++----------
  drivers/remoteproc/remoteproc_debugfs.c | 17 ++++++++++-------
  include/linux/remoteproc.h              |  8 +++++++-
  3 files changed, 32 insertions(+), 18 deletions(-)

[..]
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 77788a4bb94e..526d3cb45e37 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -86,7 +86,13 @@ struct resource_table {
   * this header, and it should be parsed according to the resource type.
   */
  struct fw_rsc_hdr {
-	u32 type;
+	union {
+		u32 type;
+		struct {
+			u16 t;
+			u16 v;
+		} st;

I see your "type" is little endian...

Yeah, definitely a draw-back if we want to support big-endian rprocs. Do you have any remoteprocs following big-endian? All TI remoteprocs are little-endian except for really old ones.

regards
Suman



[Index of Archives]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Photo Sharing]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux