diff mbox

[v5,2/3] documentation: Add secure monitor binding documentation

Message ID 1464520323-19531-3-git-send-email-carlo@caione.org (mailing list archive)
State New, archived
Headers show

Commit Message

Carlo Caione May 29, 2016, 11:12 a.m. UTC
From: Carlo Caione <carlo@endlessm.com>

Add the binding documentation for the Amlogic secure monitor driver.

Signed-off-by: Carlo Caione <carlo@endlessm.com>
---
 .../bindings/firmware/meson/meson_sm.txt           | 33 ++++++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt

Comments

Rob Herring (Arm) June 2, 2016, 5:14 p.m. UTC | #1
On Sun, May 29, 2016 at 01:12:02PM +0200, Carlo Caione wrote:
> From: Carlo Caione <carlo@endlessm.com>
> 
> Add the binding documentation for the Amlogic secure monitor driver.
> 
> Signed-off-by: Carlo Caione <carlo@endlessm.com>
> ---
>  .../bindings/firmware/meson/meson_sm.txt           | 33 ++++++++++++++++++++++
>  1 file changed, 33 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> 
> diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> new file mode 100644
> index 0000000..6f0b5bc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> @@ -0,0 +1,33 @@
> +* Amlogic Secure Monitor
> +
> +In the Amlogic SoCs the Secure Monitor code is used to provide access to the
> +NVMEM, enable JTAG, set USB boot, etc...
> +
> +Required properties for the secure monitor node:
> +- compatible: Should be "amlogic,meson-gxbb-sm"
> +
> +Example:
> +
> +	firmware {
> +		compatible = "simple-bus";

Drop this. Sorry, but no abusing simple-bus to get automagic creation of 
your platform device.

> +
> +		sm: secure-monitor {
> +			compatible = "amlogic,meson-gxbb-sm";
> +		};
> +	};
> +
> +Example of the node using the secure monitor:
> +
> +	#include <dt-bindings/firmware/meson-sm.h>
> +
> +	...
> +
> +	efuse: efuse {
> +		compatible = "amlogic,meson-gxbb-efuse";
> +		secure-monitor = <&sm>;

Why do you need this? Given there can only be one node, just use 
of_find_compatible_node();

> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +
> +		...
> +	};
> +
> -- 
> 2.7.4
>
Carlo Caione June 3, 2016, 9:25 a.m. UTC | #2
On 02/06/16 12:14, Rob Herring wrote:
> On Sun, May 29, 2016 at 01:12:02PM +0200, Carlo Caione wrote:
> > From: Carlo Caione <carlo@endlessm.com>
> > 
> > Add the binding documentation for the Amlogic secure monitor driver.
> > 
> > Signed-off-by: Carlo Caione <carlo@endlessm.com>
> > ---
> >  .../bindings/firmware/meson/meson_sm.txt           | 33 ++++++++++++++++++++++
> >  1 file changed, 33 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> > new file mode 100644
> > index 0000000..6f0b5bc
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
> > @@ -0,0 +1,33 @@
> > +* Amlogic Secure Monitor
> > +
> > +In the Amlogic SoCs the Secure Monitor code is used to provide access to the
> > +NVMEM, enable JTAG, set USB boot, etc...
> > +
> > +Required properties for the secure monitor node:
> > +- compatible: Should be "amlogic,meson-gxbb-sm"
> > +
> > +Example:
> > +
> > +	firmware {
> > +		compatible = "simple-bus";
> 
> Drop this. Sorry, but no abusing simple-bus to get automagic creation of 
> your platform device.

Sounds reasonable. I can hook this up to some *_initcall. What about
introducing a new FIRMWARE_OF_DECLARE() ?

Cheers,
Rob Herring (Arm) June 6, 2016, 1:19 p.m. UTC | #3
On Fri, Jun 3, 2016 at 4:25 AM, Carlo Caione <carlo@caione.org> wrote:
> On 02/06/16 12:14, Rob Herring wrote:
>> On Sun, May 29, 2016 at 01:12:02PM +0200, Carlo Caione wrote:
>> > From: Carlo Caione <carlo@endlessm.com>
>> >
>> > Add the binding documentation for the Amlogic secure monitor driver.
>> >
>> > Signed-off-by: Carlo Caione <carlo@endlessm.com>
>> > ---
>> >  .../bindings/firmware/meson/meson_sm.txt           | 33 ++++++++++++++++++++++
>> >  1 file changed, 33 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
>> > new file mode 100644
>> > index 0000000..6f0b5bc
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
>> > @@ -0,0 +1,33 @@
>> > +* Amlogic Secure Monitor
>> > +
>> > +In the Amlogic SoCs the Secure Monitor code is used to provide access to the
>> > +NVMEM, enable JTAG, set USB boot, etc...
>> > +
>> > +Required properties for the secure monitor node:
>> > +- compatible: Should be "amlogic,meson-gxbb-sm"
>> > +
>> > +Example:
>> > +
>> > +   firmware {
>> > +           compatible = "simple-bus";
>>
>> Drop this. Sorry, but no abusing simple-bus to get automagic creation of
>> your platform device.
>
> Sounds reasonable. I can hook this up to some *_initcall. What about
> introducing a new FIRMWARE_OF_DECLARE() ?

Please don't add FIRMWARE_OF_DECLARE unless it needs to be early.

Rob
Carlo Caione June 9, 2016, 11:42 a.m. UTC | #4
On 02/06/16 12:14, Rob Herring wrote:
> On Sun, May 29, 2016 at 01:12:02PM +0200, Carlo Caione wrote:

[...]
> > +Example of the node using the secure monitor:
> > +
> > +	#include <dt-bindings/firmware/meson-sm.h>
> > +
> > +	...
> > +
> > +	efuse: efuse {
> > +		compatible = "amlogic,meson-gxbb-efuse";
> > +		secure-monitor = <&sm>;
> 
> Why do you need this? Given there can only be one node, just use 
> of_find_compatible_node();

of_find_compatible_node() in the driver works fine if we have only one
possible compatible property for the secure-monitor node. In this case
we have a different compatible property for each SoC (GXBB in this case)
so I think having the phandle in the DT is still the best thing to do.

Any thought on this Rob?
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
new file mode 100644
index 0000000..6f0b5bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/meson/meson_sm.txt
@@ -0,0 +1,33 @@ 
+* Amlogic Secure Monitor
+
+In the Amlogic SoCs the Secure Monitor code is used to provide access to the
+NVMEM, enable JTAG, set USB boot, etc...
+
+Required properties for the secure monitor node:
+- compatible: Should be "amlogic,meson-gxbb-sm"
+
+Example:
+
+	firmware {
+		compatible = "simple-bus";
+
+		sm: secure-monitor {
+			compatible = "amlogic,meson-gxbb-sm";
+		};
+	};
+
+Example of the node using the secure monitor:
+
+	#include <dt-bindings/firmware/meson-sm.h>
+
+	...
+
+	efuse: efuse {
+		compatible = "amlogic,meson-gxbb-efuse";
+		secure-monitor = <&sm>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		...
+	};
+