diff mbox

Documentation: tpm: Adds the TPM device tree node documentation

Message ID 1472532277-21933-1-git-send-email-nayna@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nayna Aug. 30, 2016, 4:44 a.m. UTC
This is documenting device tree binding for
I2C based TPM, similar concept which being used
for virtual TPM on POWER7 and POWER8 systems running PowerVM.

Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
---
 Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29 +++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt

Comments

Jarkko Sakkinen Aug. 30, 2016, 6:36 a.m. UTC | #1
On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
> This is documenting device tree binding for
> I2C based TPM, similar concept which being used
> for virtual TPM on POWER7 and POWER8 systems running PowerVM.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29 +++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> new file mode 100644
> index 0000000..8fdee14
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> @@ -0,0 +1,29 @@
> +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
> +---------------------------------------------------------------
> +
> +This node describes a TPM device connected to Processor on i2c bus.
> +
> +Required properties:
> +
> +- compatible : 'manufacturer,model'
> +- label : represents device type
> +- linux,sml-base : base address of the Event Log. It is a physical address.
> +		   sml stands for shared memory log.
> +- linux,sml-size : size of the memory allocated for the Event Log.
> +
> +Optional properties:
> +
> +- status: indicates whether the device is enabled or disabled. "okay" for
> +          enabled and "disabled" for disabled.
> +
> +Example
> +-------
> +
> +tpm@57 {
> +	reg = <0x57>;
> +	label = "tpm";
> +	compatible = "nuvoton,npct650", "nuvoton,npct601";
> +	linux,sml-base = <0x7f 0xfd450000>;
> +	linux,sml-size = <0x10000>;
> +	status = "okay";
> +};

I would rather name the fields event-log-base and event-log-size. They
would be much more readable and obvious names.

Also, enabled should be "enabled", not "okay".

/Jarkko

------------------------------------------------------------------------------
Peter Huewe Aug. 30, 2016, 6:41 a.m. UTC | #2
Am 29. August 2016 23:36:31 GMT-07:00, schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
>On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
>> This is documenting device tree binding for
>> I2C based TPM, similar concept which being used
>> for virtual TPM on POWER7 and POWER8 systems running PowerVM.
>> 
>> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> ---
>>  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29
>+++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> 
>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> new file mode 100644
>> index 0000000..8fdee14
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> @@ -0,0 +1,29 @@
>> +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
>> +---------------------------------------------------------------
>> +
>> +This node describes a TPM device connected to Processor on i2c bus.
>> +
>> +Required properties:
>> +
>> +- compatible : 'manufacturer,model'
>> +- label : represents device type
>> +- linux,sml-base : base address of the Event Log. It is a physical
>address.
>> +		   sml stands for shared memory log.
>> +- linux,sml-size : size of the memory allocated for the Event Log.
>> +
>> +Optional properties:
>> +
>> +- status: indicates whether the device is enabled or disabled.
>"okay" for
>> +          enabled and "disabled" for disabled.
>> +
>> +Example
>> +-------
>> +
>> +tpm@57 {
>> +	reg = <0x57>;
>> +	label = "tpm";
>> +	compatible = "nuvoton,npct650", "nuvoton,npct601";
>> +	linux,sml-base = <0x7f 0xfd450000>;
>> +	linux,sml-size = <0x10000>;
>> +	status = "okay";
>> +};
>
>I would rather name the fields event-log-base and event-log-size. They
>would be much more readable and obvious names.

I agree - I always get stuck upon the sml thing.
>
>Also, enabled should be "enabled", not "okay".

No!
okay/ok is a dt keyword! (Or at least used in everything else)

It has nothing to do whether the TPM is enabled/disabled/activated whatever
Peter

>
>/Jarkko
>
>------------------------------------------------------------------------------
>_______________________________________________
>tpmdd-devel mailing list
>tpmdd-devel@lists.sourceforge.net
>https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
Peter Huewe Aug. 30, 2016, 6:46 a.m. UTC | #3
I agree - I always get stuck upon the sml thing.
>
>Also, enabled should be "enabled", not "okay".

No!
okay/ok is a dt keyword! (Or at least used in everything else)

It has nothing to do whether the TPM is enabled/disabled/activated whatever
See http://www.devicetree.org/specifications-pdf table 2.4

Value Description
"okay" Indicates the device is operational
"disabled" Indicates that the device is not presently operational, but it might become operational in the
future (for example, something is not plugged in, or switched off).
Refer to the device binding for details on what disabled means for a given device.
"fail" Indicates that the device is not operational. A serious error was detected in the device, and it
is unlikely to become operational without repair.
"fail-sss" Indicates that the device is not operational. A serious error was detected in the device and it
is unlikely to become operational without repair. The sss portion of the value is specific to
the device and indicates the error condition detected.

------------------------------------------------------------------------------
Jarkko Sakkinen Aug. 30, 2016, 7:06 a.m. UTC | #4
On Mon, Aug 29, 2016 at 11:41:51PM -0700, Peter Huewe wrote:
> 
> 
> Am 29. August 2016 23:36:31 GMT-07:00, schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
> >On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
> >> This is documenting device tree binding for
> >> I2C based TPM, similar concept which being used
> >> for virtual TPM on POWER7 and POWER8 systems running PowerVM.
> >> 
> >> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> >> ---
> >>  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29
> >+++++++++++++++++++++++
> >>  1 file changed, 29 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> >> 
> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> >b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> >> new file mode 100644
> >> index 0000000..8fdee14
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> >> @@ -0,0 +1,29 @@
> >> +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
> >> +---------------------------------------------------------------
> >> +
> >> +This node describes a TPM device connected to Processor on i2c bus.
> >> +
> >> +Required properties:
> >> +
> >> +- compatible : 'manufacturer,model'
> >> +- label : represents device type
> >> +- linux,sml-base : base address of the Event Log. It is a physical
> >address.
> >> +		   sml stands for shared memory log.
> >> +- linux,sml-size : size of the memory allocated for the Event Log.
> >> +
> >> +Optional properties:
> >> +
> >> +- status: indicates whether the device is enabled or disabled.
> >"okay" for
> >> +          enabled and "disabled" for disabled.
> >> +
> >> +Example
> >> +-------
> >> +
> >> +tpm@57 {
> >> +	reg = <0x57>;
> >> +	label = "tpm";
> >> +	compatible = "nuvoton,npct650", "nuvoton,npct601";
> >> +	linux,sml-base = <0x7f 0xfd450000>;
> >> +	linux,sml-size = <0x10000>;
> >> +	status = "okay";
> >> +};
> >
> >I would rather name the fields event-log-base and event-log-size. They
> >would be much more readable and obvious names.
> 
> I agree - I always get stuck upon the sml thing.
> >
> >Also, enabled should be "enabled", not "okay".
> 
> No!
> okay/ok is a dt keyword! (Or at least used in everything else)
> 
> It has nothing to do whether the TPM is enabled/disabled/activated whatever
> Peter

OK, just to educate myself, where can I find these standard keywords?

The granularity is wrong (section 8.7 of TPM 2.0 Structures
specification). There are four different things that you can
enabled/disable.

/Jarkko

------------------------------------------------------------------------------
Peter Huewe Aug. 30, 2016, 8:55 a.m. UTC | #5
Am 30. August 2016 00:06:49 GMT-07:00, schrieb Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>:
>On Mon, Aug 29, 2016 at 11:41:51PM -0700, Peter Huewe wrote:
>> 
>> 
>> Am 29. August 2016 23:36:31 GMT-07:00, schrieb Jarkko Sakkinen
><jarkko.sakkinen@linux.intel.com>:
>> >On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
>> >> This is documenting device tree binding for
>> >> I2C based TPM, similar concept which being used
>> >> for virtual TPM on POWER7 and POWER8 systems running PowerVM.
>> >> 
>> >> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29
>> >+++++++++++++++++++++++
>> >>  1 file changed, 29 insertions(+)
>> >>  create mode 100644
>Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> >> 
>> >> diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> >b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> >> new file mode 100644
>> >> index 0000000..8fdee14
>> >> --- /dev/null
>> >> +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
>> >> @@ -0,0 +1,29 @@
>> >> +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
>> >> +---------------------------------------------------------------
>> >> +
>> >> +This node describes a TPM device connected to Processor on i2c
>bus.
>> >> +
>> >> +Required properties:
>> >> +
>> >> +- compatible : 'manufacturer,model'
>> >> +- label : represents device type
>> >> +- linux,sml-base : base address of the Event Log. It is a
>physical
>> >address.
>> >> +		   sml stands for shared memory log.
>> >> +- linux,sml-size : size of the memory allocated for the Event
>Log.
>> >> +
>> >> +Optional properties:
>> >> +
>> >> +- status: indicates whether the device is enabled or disabled.
>> >"okay" for
>> >> +          enabled and "disabled" for disabled.
>> >> +
>> >> +Example
>> >> +-------
>> >> +
>> >> +tpm@57 {
>> >> +	reg = <0x57>;
>> >> +	label = "tpm";
>> >> +	compatible = "nuvoton,npct650", "nuvoton,npct601";
>> >> +	linux,sml-base = <0x7f 0xfd450000>;
>> >> +	linux,sml-size = <0x10000>;
>> >> +	status = "okay";
>> >> +};
>> >
>> >I would rather name the fields event-log-base and event-log-size.
>They
>> >would be much more readable and obvious names.
>> 
>> I agree - I always get stuck upon the sml thing.
>> >
>> >Also, enabled should be "enabled", not "okay".
>> 
>> No!
>> okay/ok is a dt keyword! (Or at least used in everything else)
>> 
>> It has nothing to do whether the TPM is enabled/disabled/activated
>whatever
>> Peter
>
>OK, just to educate myself, where can I find these standard keywords?


See http://www.devicetree.org/specifications-pdf table 2.4
>
>The granularity is wrong (section 8.7 of TPM 2.0 Structures
>specification). There are four different things that you can
>enabled/disable.
>
>/Jarkko
Wolfram Sang Aug. 30, 2016, 8:51 p.m. UTC | #6
On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
> This is documenting device tree binding for
> I2C based TPM, similar concept which being used
> for virtual TPM on POWER7 and POWER8 systems running PowerVM.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29 +++++++++++++++++++++++

Is the driver in 'drivers/i2c/*'? No, so the binding shouldn't be in i2c
as well.

> +- compatible : 'manufacturer,model'

You need to specify them here!

> +Optional properties:
> +
> +- status: indicates whether the device is enabled or disabled. "okay" for
> +          enabled and "disabled" for disabled.

Optional? Rather "implicit" I'd say.

This is far from ready. You should really wait for Rob's review. Or
better: Have a look at other bindings.
------------------------------------------------------------------------------
Rob Herring (Arm) Sept. 2, 2016, 2:51 p.m. UTC | #7
On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
> This is documenting device tree binding for
> I2C based TPM, similar concept which being used
> for virtual TPM on POWER7 and POWER8 systems running PowerVM.
> 
> Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29 +++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> new file mode 100644
> index 0000000..8fdee14
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> @@ -0,0 +1,29 @@
> +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
> +---------------------------------------------------------------
> +
> +This node describes a TPM device connected to Processor on i2c bus.
> +
> +Required properties:
> +
> +- compatible : 'manufacturer,model'

Needs specific compatible strings like your example has.

> +- label : represents device type

Why do you need this? label is human readable things like connectors on 
boards.

> +- linux,sml-base : base address of the Event Log. It is a physical address.
> +		   sml stands for shared memory log.

How is it a physical address on an i2c device? Why 2 cells (which needs 
to be documented also)?

Just 'log' would be more descriptive than sml.

> +- linux,sml-size : size of the memory allocated for the Event Log.
> +
> +Optional properties:
> +
> +- status: indicates whether the device is enabled or disabled. "okay" for
> +          enabled and "disabled" for disabled.

status is always valid, so you don't need to document it.

------------------------------------------------------------------------------
Rob Herring (Arm) Sept. 2, 2016, 2:52 p.m. UTC | #8
On Tue, Aug 30, 2016 at 09:36:31AM +0300, Jarkko Sakkinen wrote:
> On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
> > This is documenting device tree binding for
> > I2C based TPM, similar concept which being used
> > for virtual TPM on POWER7 and POWER8 systems running PowerVM.
> > 
> > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29 +++++++++++++++++++++++
> >  1 file changed, 29 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> > new file mode 100644
> > index 0000000..8fdee14
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> > @@ -0,0 +1,29 @@
> > +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
> > +---------------------------------------------------------------
> > +
> > +This node describes a TPM device connected to Processor on i2c bus.
> > +
> > +Required properties:
> > +
> > +- compatible : 'manufacturer,model'
> > +- label : represents device type
> > +- linux,sml-base : base address of the Event Log. It is a physical address.
> > +		   sml stands for shared memory log.
> > +- linux,sml-size : size of the memory allocated for the Event Log.
> > +
> > +Optional properties:
> > +
> > +- status: indicates whether the device is enabled or disabled. "okay" for
> > +          enabled and "disabled" for disabled.
> > +
> > +Example
> > +-------
> > +
> > +tpm@57 {
> > +	reg = <0x57>;
> > +	label = "tpm";
> > +	compatible = "nuvoton,npct650", "nuvoton,npct601";
> > +	linux,sml-base = <0x7f 0xfd450000>;
> > +	linux,sml-size = <0x10000>;
> > +	status = "okay";
> > +};
> 
> I would rather name the fields event-log-base and event-log-size. They
> would be much more readable and obvious names.
> 
> Also, enabled should be "enabled", not "okay".

No, okay is correct. But as I mentioned, don't document it here.

Rob

------------------------------------------------------------------------------
Jarkko Sakkinen Sept. 2, 2016, 4 p.m. UTC | #9
On Fri, Sep 02, 2016 at 09:52:38AM -0500, Rob Herring wrote:
> On Tue, Aug 30, 2016 at 09:36:31AM +0300, Jarkko Sakkinen wrote:
> > On Tue, Aug 30, 2016 at 12:44:37AM -0400, Nayna Jain wrote:
> > > This is documenting device tree binding for
> > > I2C based TPM, similar concept which being used
> > > for virtual TPM on POWER7 and POWER8 systems running PowerVM.
> > > 
> > > Signed-off-by: Nayna Jain <nayna@linux.vnet.ibm.com>
> > > ---
> > >  Documentation/devicetree/bindings/i2c/i2c-tpm.txt | 29 +++++++++++++++++++++++
> > >  1 file changed, 29 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> > > new file mode 100644
> > > index 0000000..8fdee14
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
> > > @@ -0,0 +1,29 @@
> > > +Device Tree Bindings for I2C based Trusted Platform Module(TPM)
> > > +---------------------------------------------------------------
> > > +
> > > +This node describes a TPM device connected to Processor on i2c bus.
> > > +
> > > +Required properties:
> > > +
> > > +- compatible : 'manufacturer,model'
> > > +- label : represents device type
> > > +- linux,sml-base : base address of the Event Log. It is a physical address.
> > > +		   sml stands for shared memory log.
> > > +- linux,sml-size : size of the memory allocated for the Event Log.
> > > +
> > > +Optional properties:
> > > +
> > > +- status: indicates whether the device is enabled or disabled. "okay" for
> > > +          enabled and "disabled" for disabled.
> > > +
> > > +Example
> > > +-------
> > > +
> > > +tpm@57 {
> > > +	reg = <0x57>;
> > > +	label = "tpm";
> > > +	compatible = "nuvoton,npct650", "nuvoton,npct601";
> > > +	linux,sml-base = <0x7f 0xfd450000>;
> > > +	linux,sml-size = <0x10000>;
> > > +	status = "okay";
> > > +};
> > 
> > I would rather name the fields event-log-base and event-log-size. They
> > would be much more readable and obvious names.
> > 
> > Also, enabled should be "enabled", not "okay".
> 
> No, okay is correct. But as I mentioned, don't document it here.

We'll stick to sml-base and sml-size because the existing code binds
already to those names.

/Jakrko

------------------------------------------------------------------------------
Jason Gunthorpe Sept. 2, 2016, 5:06 p.m. UTC | #10
On Fri, Sep 02, 2016 at 09:51:21AM -0500, Rob Herring wrote:
> > +- linux,sml-base : base address of the Event Log. It is a physical address.
> > +		   sml stands for shared memory log.
> 
> How is it a physical address on an i2c device? Why 2 cells (which needs 
> to be documented also)?

To be clear, as I understand it, this mechanism is a hand off from the
boot firmware to Linux.

The boot firmware talks i2c to the device, does some stuff, writes it
to memory and then linux reads that stuff. I agree it seems crazy to
include a random physical address like that.

The linux,sml-* names appear to have been used by IBM for a long time
on their enterprise PPC platforms (see drivers/char/tpm/tpm_of.c), so
I've expected we have to keep them?

I asked Nayna to document this stuff IBM is doing so the rest of us
in TPM land can have a hope of maintaining it...

Jason

------------------------------------------------------------------------------
Rob Herring (Arm) Sept. 2, 2016, 5:57 p.m. UTC | #11
On Fri, Sep 2, 2016 at 12:06 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> On Fri, Sep 02, 2016 at 09:51:21AM -0500, Rob Herring wrote:
>> > +- linux,sml-base : base address of the Event Log. It is a physical address.
>> > +              sml stands for shared memory log.
>>
>> How is it a physical address on an i2c device? Why 2 cells (which needs
>> to be documented also)?
>
> To be clear, as I understand it, this mechanism is a hand off from the
> boot firmware to Linux.
>
> The boot firmware talks i2c to the device, does some stuff, writes it
> to memory and then linux reads that stuff. I agree it seems crazy to
> include a random physical address like that.

I'd put that in reserved-memory then if designing this from scratch...

Must not be completely random as somehow the kernel doesn't use that memory.

> The linux,sml-* names appear to have been used by IBM for a long time
> on their enterprise PPC platforms (see drivers/char/tpm/tpm_of.c), so
> I've expected we have to keep them?

Yes. I wasn't aware of that.

> I asked Nayna to document this stuff IBM is doing so the rest of us
> in TPM land can have a hope of maintaining it...
>
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

------------------------------------------------------------------------------
Nayna Sept. 28, 2016, 8:40 a.m. UTC | #12
On 09/02/2016 11:27 PM, Rob Herring wrote:
> On Fri, Sep 2, 2016 at 12:06 PM, Jason Gunthorpe
> <jgunthorpe@obsidianresearch.com> wrote:
>> On Fri, Sep 02, 2016 at 09:51:21AM -0500, Rob Herring wrote:
>>>> +- linux,sml-base : base address of the Event Log. It is a physical address.
>>>> +              sml stands for shared memory log.
>>>
>>> How is it a physical address on an i2c device? Why 2 cells (which needs
>>> to be documented also)?
>>
>> To be clear, as I understand it, this mechanism is a hand off from the
>> boot firmware to Linux.
>>
>> The boot firmware talks i2c to the device, does some stuff, writes it
>> to memory and then linux reads that stuff. I agree it seems crazy to
>> include a random physical address like that.
>
> I'd put that in reserved-memory then if designing this from scratch...

Thanks for the review comments.

Yes, it is in reserved-memory. I modified the explanation for 
linux,sml-base property to be more descriptive now in my v2 version of 
the patch, posted just now.
>
> Must not be completely random as somehow the kernel doesn't use that memory.
>
>> The linux,sml-* names appear to have been used by IBM for a long time
>> on their enterprise PPC platforms (see drivers/char/tpm/tpm_of.c), so
>> I've expected we have to keep them?
>
> Yes. I wasn't aware of that.
>
>> I asked Nayna to document this stuff IBM is doing so the rest of us
>> in TPM land can have a hope of maintaining it...
>>
>> Jason
>> --
>> To unsubscribe from this list: send the line "unsubscribe devicetree" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


------------------------------------------------------------------------------
Nayna Sept. 28, 2016, 8:44 a.m. UTC | #13
On 09/02/2016 10:36 PM, Jason Gunthorpe wrote:
> On Fri, Sep 02, 2016 at 09:51:21AM -0500, Rob Herring wrote:
>>> +- linux,sml-base : base address of the Event Log. It is a physical address.
>>> +		   sml stands for shared memory log.
>>
>> How is it a physical address on an i2c device? Why 2 cells (which needs
>> to be documented also)?
>
> To be clear, as I understand it, this mechanism is a hand off from the
> boot firmware to Linux.
>
> The boot firmware talks i2c to the device, does some stuff, writes it
> to memory and then linux reads that stuff. I agree it seems crazy to
> include a random physical address like that.
>
> The linux,sml-* names appear to have been used by IBM for a long time
> on their enterprise PPC platforms (see drivers/char/tpm/tpm_of.c), so
> I've expected we have to keep them?
>
> I asked Nayna to document this stuff IBM is doing so the rest of us
> in TPM land can have a hope of maintaining it...

Thanks Jason !!

In my v2 version for device tree documentation, I have posted the 
documentation for both vtpm and physical TPM.

>
> Jason
>


------------------------------------------------------------------------------
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/i2c/i2c-tpm.txt b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
new file mode 100644
index 0000000..8fdee14
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-tpm.txt
@@ -0,0 +1,29 @@ 
+Device Tree Bindings for I2C based Trusted Platform Module(TPM)
+---------------------------------------------------------------
+
+This node describes a TPM device connected to Processor on i2c bus.
+
+Required properties:
+
+- compatible : 'manufacturer,model'
+- label : represents device type
+- linux,sml-base : base address of the Event Log. It is a physical address.
+		   sml stands for shared memory log.
+- linux,sml-size : size of the memory allocated for the Event Log.
+
+Optional properties:
+
+- status: indicates whether the device is enabled or disabled. "okay" for
+          enabled and "disabled" for disabled.
+
+Example
+-------
+
+tpm@57 {
+	reg = <0x57>;
+	label = "tpm";
+	compatible = "nuvoton,npct650", "nuvoton,npct601";
+	linux,sml-base = <0x7f 0xfd450000>;
+	linux,sml-size = <0x10000>;
+	status = "okay";
+};