Re: [PATCH 2/2] tpm2: Add a TPMv2 MMIO TIS driver

[Ilias Apalodimas <ilias.apalodimas@linaro.org>]

On Tue, Jul 13, 2021 at 08:49:21PM -0600, Simon Glass wrote:
> Hi Ilias,
> 
> On Tue, 13 Jul 2021 at 14:11, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> >
> > [...]
> > > > > 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.
> 
> A uclass is basically a software construct. It is an interface between
> clients and the driver, typically. Quite often the uclass is an
> interface on top of the hardware (actually the driver). But quite
> often it is not. For example, we use an GPIO uclass to access a pmic's
> GPIOs, we use an I2C uclass to access the cros_ec I2C tunnel. Anywhere
> where it makes sense to have an abstraction, we use a uclass.
> 
> >
> > > 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?
> 
> You have already added another API, right? All we are discussing is
> whether it should be a uclass or not. Unless there is a very good
> reason, we should avoid creating custom interfaces that don't use
> driver model. I actually think the interface you've created (MMIO)
> will be very useful as a uclass.
> 

So you are basically looking into adding something similar to
dm_i2c_read/dm_i2c_write etc?
I assume this is gong to be the default read method passed on the TIS API
when we want to support i2c TPMs.
For the MMIO case that would essentially mean, move the functions on a
different file, add them on a header and define a UCLASS_DRIVER with only
the .id and .name defined?

Thanks
/Ilias

> [..]
> 
> Regards,
> Simon