| Message ID | 20161025232331.GA20339@obsidianresearch.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, Oct 25, 2016 at 05:23:31PM -0600, Jason Gunthorpe wrote: > This provides an open firwmare driver binding for tpm_tis. OF > is useful on arches where ACPI/PNP is not used. > > The tcg,tpm_tis-mmio register map interface is specified by the TCG. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > .../bindings/security/tpm/tpm_tis_mmio.txt | 22 ++++++++++++++++++++++ I'll do a proper review later on but should this change be a separate commit or not? /Jarkko > drivers/char/tpm/Kconfig | 2 +- > drivers/char/tpm/tpm_tis.c | 11 +++++++++++ > 3 files changed, 34 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt > > A bunch of work was done previously on how the tis driver probes and > binds, so DT enablement is now trivial. > > diff --git a/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt b/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt > new file mode 100644 > index 000000000000..67661f6a27cd > --- /dev/null > +++ b/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt > @@ -0,0 +1,22 @@ > +Trusted Computing Group MMIO Trusted Platform Module > + > +The TCG defines multi vendor standard for accessing a TPM chip, this > +is the standard protocol defined to access the TPM via MMIO. Typically > +this interface will be implemented over Intel's LPC bus. > + > +Refer to the 'TCG PC Client Specific TPM Interface Specification (TIS)' TCG > +publication for the specification. > + > +Required properties: > + > +- compatible: should specify the actual hardware chip followed by the > + generic interface name "tcg,tpm_tis-spi"; > +- reg: The location of the MMIO registers, should be at least 0x5000 bytes > +- interrupt: An optional interrupt indicating command completion. > + > +Example: > + > + tpm_tis@90000 { > + compatible = "atmel,at97sc3204", "tcg,tpm_tis-mmio"; > + reg = <0x90000 0x5000>; > + }; > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig > index 9faa0b1e7766..277186d3b668 100644 > --- a/drivers/char/tpm/Kconfig > +++ b/drivers/char/tpm/Kconfig > @@ -32,7 +32,7 @@ config TCG_TIS_CORE > > config TCG_TIS > tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface" > - depends on X86 > + depends on X86 || OF > select TCG_TIS_CORE > ---help--- > If you have a TPM security chip that is compliant with the > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index eaf5730d79eb..5a63fafaca69 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -28,6 +28,8 @@ > #include <linux/wait.h> > #include <linux/acpi.h> > #include <linux/freezer.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > #include "tpm.h" > #include "tpm_tis_core.h" > > @@ -354,12 +356,21 @@ static int tpm_tis_plat_remove(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_OF > +static const struct of_device_id tis_of_platform_match[] = { > + {.compatible = "tcg,tpm_tis-mmio"}, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, tis_of_platform_match); > +#endif > + > static struct platform_driver tis_drv = { > .probe = tpm_tis_plat_probe, > .remove = tpm_tis_plat_remove, > .driver = { > .name = "tpm_tis", > .pm = &tpm_tis_pm, > + .of_match_table = of_match_ptr(tis_of_platform_match), > }, > }; > > -- > 2.1.4 > -- 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
On Tue, Oct 25, 2016 at 05:23:31PM -0600, Jason Gunthorpe wrote: > This provides an open firwmare driver binding for tpm_tis. OF > is useful on arches where ACPI/PNP is not used. > > The tcg,tpm_tis-mmio register map interface is specified by the TCG. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > .../bindings/security/tpm/tpm_tis_mmio.txt | 22 ++++++++++++++++++++++ > drivers/char/tpm/Kconfig | 2 +- > drivers/char/tpm/tpm_tis.c | 11 +++++++++++ > 3 files changed, 34 insertions(+), 1 deletion(-) > create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt > > A bunch of work was done previously on how the tis driver probes and > binds, so DT enablement is now trivial. > > diff --git a/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt b/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt > new file mode 100644 > index 000000000000..67661f6a27cd > --- /dev/null > +++ b/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt > @@ -0,0 +1,22 @@ > +Trusted Computing Group MMIO Trusted Platform Module > + > +The TCG defines multi vendor standard for accessing a TPM chip, this > +is the standard protocol defined to access the TPM via MMIO. Typically > +this interface will be implemented over Intel's LPC bus. > + > +Refer to the 'TCG PC Client Specific TPM Interface Specification (TIS)' TCG > +publication for the specification. > + > +Required properties: > + > +- compatible: should specify the actual hardware chip followed by the > + generic interface name "tcg,tpm_tis-spi"; Please use '-' in comaptible strings rather than '_'. Please also describe chip strings, even if those aren't use by the driver, e.g. compatible: should contain a string below for the chip, followed by "tcg,tpm-tis-spi". Valid chip strings are: * "atmel,at97sc3204" > +- reg: The location of the MMIO registers, should be at least 0x5000 bytes > +- interrupt: An optional interrupt indicating command completion. > + > +Example: > + > + tpm_tis@90000 { > + compatible = "atmel,at97sc3204", "tcg,tpm_tis-mmio"; As above, that first compatible string should be listed, if it's intended to be valid. Otherwise, this looks fine to me. Thanks, Mark. > + reg = <0x90000 0x5000>; > + }; > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig > index 9faa0b1e7766..277186d3b668 100644 > --- a/drivers/char/tpm/Kconfig > +++ b/drivers/char/tpm/Kconfig > @@ -32,7 +32,7 @@ config TCG_TIS_CORE > > config TCG_TIS > tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface" > - depends on X86 > + depends on X86 || OF > select TCG_TIS_CORE > ---help--- > If you have a TPM security chip that is compliant with the > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index eaf5730d79eb..5a63fafaca69 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -28,6 +28,8 @@ > #include <linux/wait.h> > #include <linux/acpi.h> > #include <linux/freezer.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > #include "tpm.h" > #include "tpm_tis_core.h" > > @@ -354,12 +356,21 @@ static int tpm_tis_plat_remove(struct platform_device *pdev) > return 0; > } > > +#ifdef CONFIG_OF > +static const struct of_device_id tis_of_platform_match[] = { > + {.compatible = "tcg,tpm_tis-mmio"}, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, tis_of_platform_match); > +#endif > + > static struct platform_driver tis_drv = { > .probe = tpm_tis_plat_probe, > .remove = tpm_tis_plat_remove, > .driver = { > .name = "tpm_tis", > .pm = &tpm_tis_pm, > + .of_match_table = of_match_ptr(tis_of_platform_match), > }, > }; > > -- > 2.1.4 > > -- > 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 > -- 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
On Wed, Oct 26, 2016 at 12:25:51PM +0100, Mark Rutland wrote: > > +Required properties: > > + > > +- compatible: should specify the actual hardware chip followed by the > > + generic interface name "tcg,tpm_tis-spi"; > > Please use '-' in comaptible strings rather than '_'. Erk, there is also a typo spi vs mmio. Note that the SPI patches were already Ack'd by DT so there is now precedent: Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt: "tcg,tpm_tis-spi" Do you still want to change? Should we change spi too? > Please also describe chip strings, even if those aren't use by the > driver, e.g. Sure, I can give one or two, but the list will not be exhaustive, there are many different TPM chips that adhere to this standard. Thanks, 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
On Wed, Oct 26, 2016 at 01:55:27PM +0300, Jarkko Sakkinen wrote: > On Tue, Oct 25, 2016 at 05:23:31PM -0600, Jason Gunthorpe wrote: > > This provides an open firwmare driver binding for tpm_tis. OF > > is useful on arches where ACPI/PNP is not used. > > > > The tcg,tpm_tis-mmio register map interface is specified by the TCG. > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > .../bindings/security/tpm/tpm_tis_mmio.txt | 22 ++++++++++++++++++++++ > > I'll do a proper review later on but should this change be a separate > commit or not? I've understood it is OK for tiny source patches, large ones should be split. In either case they both go through your tree together. The DT mailing list just does not want to be spammed... 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
On Wed, Oct 26, 2016 at 10:01:53AM -0600, Jason Gunthorpe wrote: > On Wed, Oct 26, 2016 at 12:25:51PM +0100, Mark Rutland wrote: > > > > +Required properties: > > > + > > > +- compatible: should specify the actual hardware chip followed by the > > > + generic interface name "tcg,tpm_tis-spi"; > > > > Please use '-' in comaptible strings rather than '_'. > > Erk, there is also a typo spi vs mmio. Urgh; missed that too. > Note that the SPI patches were already Ack'd by DT so there is now > precedent: > > Documentation/devicetree/bindings/security/tpm/tpm_tis_spi.txt: "tcg,tpm_tis-spi" Urgh, that should not have gone in as-is. > Do you still want to change? Regardless of the bad example in the SPI case, this should use a dash. > Should we change spi too? That was in v4.8, so it's too late to change it now. > > Please also describe chip strings, even if those aren't use by the > > driver, e.g. > > Sure, I can give one or two, but the list will not be exhaustive, > there are many different TPM chips that adhere to this standard. Just the one from the example for now is fine; we can add new ones as we add DTs that use them. Thanks, Mark. -- 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
On Wed, Oct 26, 2016 at 10:05:05AM -0600, Jason Gunthorpe wrote: > On Wed, Oct 26, 2016 at 01:55:27PM +0300, Jarkko Sakkinen wrote: > > On Tue, Oct 25, 2016 at 05:23:31PM -0600, Jason Gunthorpe wrote: > > > This provides an open firwmare driver binding for tpm_tis. OF > > > is useful on arches where ACPI/PNP is not used. > > > > > > The tcg,tpm_tis-mmio register map interface is specified by the TCG. > > > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > .../bindings/security/tpm/tpm_tis_mmio.txt | 22 ++++++++++++++++++++++ > > > > I'll do a proper review later on but should this change be a separate > > commit or not? > > I've understood it is OK for tiny source patches, large ones should be > split. In either case they both go through your tree together. The DT > mailing list just does not want to be spammed... In general we ask that binding modifications are in a separate patch, earlier in the series (see [1]), but in cases where the additions are small, we're not too bothered. As for why, it's about making it easier to spot the parts we need to act on. We still would like to see the entire series (and ask that we're cc'd [1]), as this can be useful context for review, and there are often mismatches between the documented binding and code that we may spot. Thanks, Mark. [1] Documentation/devicetree/bindings/submitting-patches.txt -- 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
diff --git a/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt b/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt new file mode 100644 index 000000000000..67661f6a27cd --- /dev/null +++ b/Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt @@ -0,0 +1,22 @@ +Trusted Computing Group MMIO Trusted Platform Module + +The TCG defines multi vendor standard for accessing a TPM chip, this +is the standard protocol defined to access the TPM via MMIO. Typically +this interface will be implemented over Intel's LPC bus. + +Refer to the 'TCG PC Client Specific TPM Interface Specification (TIS)' TCG +publication for the specification. + +Required properties: + +- compatible: should specify the actual hardware chip followed by the + generic interface name "tcg,tpm_tis-spi"; +- reg: The location of the MMIO registers, should be at least 0x5000 bytes +- interrupt: An optional interrupt indicating command completion. + +Example: + + tpm_tis@90000 { + compatible = "atmel,at97sc3204", "tcg,tpm_tis-mmio"; + reg = <0x90000 0x5000>; + }; diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig index 9faa0b1e7766..277186d3b668 100644 --- a/drivers/char/tpm/Kconfig +++ b/drivers/char/tpm/Kconfig @@ -32,7 +32,7 @@ config TCG_TIS_CORE config TCG_TIS tristate "TPM Interface Specification 1.2 Interface / TPM 2.0 FIFO Interface" - depends on X86 + depends on X86 || OF select TCG_TIS_CORE ---help--- If you have a TPM security chip that is compliant with the diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c index eaf5730d79eb..5a63fafaca69 100644 --- a/drivers/char/tpm/tpm_tis.c +++ b/drivers/char/tpm/tpm_tis.c @@ -28,6 +28,8 @@ #include <linux/wait.h> #include <linux/acpi.h> #include <linux/freezer.h> +#include <linux/of.h> +#include <linux/of_device.h> #include "tpm.h" #include "tpm_tis_core.h" @@ -354,12 +356,21 @@ static int tpm_tis_plat_remove(struct platform_device *pdev) return 0; } +#ifdef CONFIG_OF +static const struct of_device_id tis_of_platform_match[] = { + {.compatible = "tcg,tpm_tis-mmio"}, + {}, +}; +MODULE_DEVICE_TABLE(of, tis_of_platform_match); +#endif + static struct platform_driver tis_drv = { .probe = tpm_tis_plat_probe, .remove = tpm_tis_plat_remove, .driver = { .name = "tpm_tis", .pm = &tpm_tis_pm, + .of_match_table = of_match_ptr(tis_of_platform_match), }, };
This provides an open firwmare driver binding for tpm_tis. OF is useful on arches where ACPI/PNP is not used. The tcg,tpm_tis-mmio register map interface is specified by the TCG. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- .../bindings/security/tpm/tpm_tis_mmio.txt | 22 ++++++++++++++++++++++ drivers/char/tpm/Kconfig | 2 +- drivers/char/tpm/tpm_tis.c | 11 +++++++++++ 3 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 Documentation/devicetree/bindings/security/tpm/tpm_tis_mmio.txt A bunch of work was done previously on how the tis driver probes and binds, so DT enablement is now trivial.