Hi Ilias,
On Tue, 13 Jul 2021 at 23:23, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
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.
Well I'm not quite sure, but if MMIO can sit on top of I2C, then yes.
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?
Yes. Something like:
- add a new uclass ID to dm/uciass-id.h - add include/mmio.h (or whatever it is called) with operations (see pch.h as an example) - add drivers/misc/mmio-uclass.c with the UCLASS_DRIVER and stubs for the API - add drivers/misc/sandbox_mmio.c with a sandbox driver that does very little (perhaps just read canned data?) - add test/dm/mmio.c sandbox test that calls each API function and checks the result
As I said pch is a good example since it is fairly simple.
Regards, Simon