Re: [PATCH v3 4/4] documentation: Add secure monitor binding documentation

[Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>]

On 23/05/16 18:11, Mark Rutland wrote:
> On Mon, May 23, 2016 at 06:59:31PM +0200, Carlo Caione wrote:
> > On 23/05/16 17:38, Mark Rutland wrote:
> > > > +Required properties for the secure monitor node:
> > > > +- compatible: Should be "amlogic,meson-sm"
> > > > +- amlogic,sm-cmd-input-base: SMC32 function identifier to read the physical
> > > > +                             address of the input buffer
> > > > +- amlogic,sm-cmd-output-base: SMC32 function identifier to read the physical
> > > > +                              address of the output buffer
> > > 
> > > Do the IDs for these actually differ per board?
> > 
> > I expect these to differ per SoC (GXBB in this case), not per board. The
> > driver is generic enough to be (hopefully) used for several SoCs just
> > changing the related header file that defines the SCM commands.
> > 
> > > Are some functions simply not implemented on some boards?
> > 
> > I don't think this is possible.
> 
> Given that, I think it may be better to just have a
> "amlogic,meson-gxbb-sm" compatible string, and derive the set of
> functions and associated IDs from that.

I can reference a specific SMC function using an index in the DT, the
same index for all the SoCs. The index is then associated to the actual
SoC-specific command ID in the driver according to the compatible string
used for the secure-monitor node.

Something like:

  // Not SoC-specific
  #include <dt-bindings/firmware/meson.h>

  sm: sm {
  	compatible = "amlogic,meson-gxbb-sm";
  };
  
  efuse {
  	compatible = "amlogic,meson-gxbb-efuse";
  	secure-monitor = <&sm>;
  	amlogic,cmd-read-efuse = <READ_EFUSE>;
  	...
  };

Is this any better?

At this point I wonder if it makes sense having the driver-specific
function IDs (like 'amlogic,cmd-read-efuse' above) defined in the DT.

> That is, unless you know that future revisions have functions with the
> exact same semantics but differing IDs.

This is most probably the case.

Also the driver exports really generic functions to access the
secure-monitor on purpose, so that the driver using it can define the
semantic of the SMC call. I really would like to avoid fixing the
semantic in the SM driver itself since we will end up with: different
semantics for each SMC call and for each SoC.

> Regardless, it would make sense to have a GXBB-specific compatible
> string prepended to the list. That way in the future we can handle
> anything specific to the GXBB variant of the secure monitor if
> necessary.

Agree on this.

Cheers,

-- 
Carlo Caione
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html