On Wed, Dec 13, 2023 at 10:23 AM Lukas Wunner <lukas@xxxxxxxxx> wrote:
>
> On Mon, Nov 27, 2023 at 10:31:06AM -0600, Rob Herring wrote:
> > On Mon, Nov 27, 2023 at 8:09AM Lukas Wunner <lukas@xxxxxxxxx> wrote:
> > > A significant number of Trusted Platform Modules conform to the "TIS"
> > > specification published by the Trusted Computing Group ("TCG PC Client
> > > Specific TPM Interface Specification"). These chips typically use an
> > > SPI, I涎 or LPC bus as transport (via MMIO in the latter case). Some
> > > of them even support multiple of those buses (selectable through a
> > > config strap) or the same chip is available in multiple SKUs, each with
> > > a different bus interface.
> > >
> > > The devicetree bindings for these TPMs have not been converted to DT
> > > schema yet and are spread out across 3 generic files and 3 chip-specific
> > > files. A few TPM compatible strings were added to trivial-devices.yaml
> > > even though additional properties are documented in the plaintext
> > > bindings.
> > >
> > > Consolidate the devicetree bindings into 3 files, one per bus.
> [...]
> > > Changes v1 -> v2:
> > > * Drop google,cr50 SPI example (Rob).
> >
> > That's going to avoid a warning in the examples, but it's going to
> > fail any actual google,c50 SPI user. What's going to happen is both
> > the SPI and I2C TPM schemas will be applied. Any SPI based cases will
> > fail if they have SPI properties because the I2C schema won't allow
> > them. If there is no fallback for google,cr50, then you must do a
> > separate schema doc (well, you could do an if/then schema in
> > tcg,tpm-tis-i2c.yaml to reference spi-peripheral-props.yaml, but that
> > would look kind of odd).
>
> I'm wondering if a "select:" property in the schema would be a viable
> (and acceptable) way to solve this.
>
> Ideally the validator would match a regex against the $nodename of the
> parent and see if it contains "spi" or "i2c". But I think matching
> against the parent's $nodename isn't possible, is it?
No. I've thought of adding something like that, but haven't.
> I can only
> match the TPM's $nodename, right?
Right.
> All the devicetree nodes in arch/arm64/boot/dts/* containing a
> google,cr50 compatible string have an spi-max-frequency property if
> they're attached to SPI. So I think it may be possible to select the
> i2c or spi schema based on presence of that property if the compatible
> string is google,cr50. A bit kludgy perhaps but if there's no better
> option?
I don't think we should make spi-max-frequency required.
> What I don't like about creating a custom schema for google,cr50 is that
> there may be other chips in the future which support multiple buses,
> so we'd need an spi+i2c schema and probably also an spi+i2c+mmio,
> i2c+mmio, spi+mmio schema. It gets messy. Granted we could enforce
> that these newly added chips use a fallback compatible that we could
> select the schema with. Still, automatically selecting the right
> schema would be better, in particular if I could somehow match against
> the $nodename of the parent.
Seems like more of a theoretical problem than realistic. I think cr50
is a bit of a special case. It also has other functions AIUI and maybe
we'll need other DT properties. If we do have new ones, I think
enforcing the fallback should be enough. After all, that's what you
tried to do in v1, but we're stuck with it for cr50.
Another option is combine the SPI and I2C schemas. Then you just need:
if:
properties:
compatible:
contains:
enum:
- tcg,tpm_tis-spi
- google,cr50
then:
$ref: spi-peripheral-props.yaml
I would leave MMIO separate in that case. Seems unlikely something
would have MMIO and a serial interface.
I still lean towards a separate schema for cr50 over this.
BTW, there's now another conversion patch[1] which I forgot to Cc you
on my reply.
Rob
[1] https://lore.kernel.org/all/20231213161347.GA1204384-robh@xxxxxxxxxx/
