[PATCH 2/2] tpm2: Add a TPMv2 MMIO TIS driver
Ilias Apalodimas
ilias.apalodimas at linaro.org
Tue Jul 13 22:11:48 CEST 2021
[...]
> > > Should be a uclass interface.
> > >
> >
> > Why? A uclass is supposed to describe and abstract hardware. This is just
> > a specific implementation of a TPM, not all TPMs are TIS compliant. We already
> > have a uclass for those.
>
> Who told you that a uclass is supposed to describe and abstract hardware? :-)
>
That's what I've mostly seen it used for, maybe i got the idea wrong.
> The uclass is how driver model does APIs, so normally a uclass would
> be used for any API. There are exceptions, but this one actually looks
> like a useful interface we should have.
>
the point is we already have a uclass for tpm devices. So why should the
we add another one that just describes the TIS interface?
> >
> > > > + .read_bytes = mmio_read_bytes,
> > > > + .write_bytes = mmio_write_bytes,
> > > > + .read16 = mmio_read16,
> > > > + .read32 = mmio_read32,
> > > > + .write32 = mmio_write32,
> > > > +};
> > > > +
> > > > +static int tpm_tis_probe(struct udevice *udev)
> > > > +{
> > > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > > > + struct tpm_chip_priv *priv = dev_get_uclass_priv(udev);
> > > > + int ret = 0;
> > > > + fdt_addr_t ioaddr;
> > > > + u64 sz;
> > > > +
> > > > + ioaddr = dev_read_addr(udev);
> > > > + if (ioaddr == FDT_ADDR_T_NONE)
> > > > + return -EINVAL;
> > >
> > > consider this for easier debugging:
> > >
> > > return log_msg_ret("ioaddr", -EINVAL);
> > >
> >
> > Sure
> >
> > > > +
> > > > + ret = dev_read_u64(udev, "reg", &sz);
> > > > + if (ret)
> > > > + return -EINVAL;
> > > > +
> > > > + drv_data->iobase = ioremap(ioaddr, sz);
> > > > + log_info("Remapped TPM2 base: 0x%llx size: 0x%llx\n", ioaddr, sz);
> > >
> > > log_debug() I think?
> > >
> > Yea, that's a leftover of my initial code, were I needed to make sure the
> > ioremap worked.
> >
> > > > + tpm_tis_ops_register(udev, &phy_ops);
> > > > + ret = tpm_tis_init(udev);
> > > > + if (ret)
> > > > + goto iounmap;
> > > > +
> > > > + priv->pcr_count = drv_data->pcr_count;
> > > > + priv->pcr_select_min = drv_data->pcr_select_min;
> > > > + /*
> > > > + * Although the driver probably works with a TPMv1 our Kconfig
> > > > + * limits the driver to TPMv2 only
> > > > + */
> > > > + priv->version = TPM_V2;
> > > > +
> > > > + return ret;
> > > > +iounmap:
> > > > + iounmap(drv_data->iobase);
> > > > + return -EINVAL;
> > > > +}
> > > > +
> > > > +static int tpm_tis_remove(struct udevice *udev)
> > > > +{
> > > > + struct tpm_tis_chip_data *drv_data = (void *)dev_get_driver_data(udev);
> > > > +
> > > > + iounmap(drv_data->iobase);
> > > > + return tpm_tis_cleanup(udev);
> > > > +}
> > > > +
> > > > +static const struct tpm_ops tpm_tis_ops = {
> > > > + .open = tpm_tis_open,
> > > > + .close = tpm_tis_close,
> > > > + .get_desc = tpm_tis_get_desc,
> > > > + .send = tpm_tis_send,
> > > > + .recv = tpm_tis_recv,
> > > > + .cleanup = tpm_tis_cleanup,
> > > > +};
> > > > +
> > > > +static const struct tpm_tis_chip_data tpm_tis_std_chip_data = {
> > > > + .pcr_count = 24,
> > > > + .pcr_select_min = 3,
> > > > +};
> > > > +
> > > > +static const struct udevice_id tpm_tis_ids[] = {
> > > > + {
> > > > + .compatible = "tcg,tpm-tis-mmio",
> > > > + .data = (ulong)&tpm_tis_std_chip_data,
> > > > + },
> > > > + { }
> > > > +};
> > > > +
> > > > +U_BOOT_DRIVER(tpm_tis_mmio) = {
> > > > + .name = "tpm_tis_mmio",
> > > > + .id = UCLASS_TPM,
> > > > + .of_match = tpm_tis_ids,
> > > > + .ops = &tpm_tis_ops,
> > > > + .probe = tpm_tis_probe,
> > > > + .remove = tpm_tis_remove,
> > > > + .priv_auto = sizeof(struct tpm_chip),
> > > > +};
> > > > --
> > > > 2.32.0.rc0
> > > >
> > >
> > > Regards,
> > > Simon
> >
> > Thanks for looking at this
> > /Ilias
>
> Likewise.
>
> Regards,
> Simon
More information about the U-Boot
mailing list