On 24/03/2016 18:10, Jason Gunthorpe wrote:

On Thu, Mar 24, 2016 at 05:31:21PM +0100, Christophe Ricard wrote:

   I got confused like you are but when looking a little bit better at the
   code i think we have 2 differents "struct device" available on a tpm
   driver.
   One is coming from the physical layer driver (aka: client->dev,
   spi->dev, pdev->dev) and the other one from chip->dev which is
   initialized in tpm_chip_alloc.

Yes, client->dev's drvdata (aka chip->dev.parent) should point to the
chip. In an ideal world the TPM core will never touch this, but we are
not quite there yet, sysfs in particular is broken.

chip->dev's drvdata could/should point to the driver's priv, that
would make sense. With Jarkko's tree today that is OK.

But Stefan's vtpm driver adds a use:

 https://github.com/stefanberger/linux/commit/3ab4e3aabf50137068e2807910e5f867668b4e23

Which is an ugly hacky thing..

I ran through the changes using client->dev's drvdata pointing to the chip and
&chip->dev's drvdata pointing to the driver's priv.
I have looked to stefanberger githubs and i am really sceptical about the purpose
of doing dev_set_drvdata(&chip->dev, chip); in tpm_sysfs_add_device.
I haven't seen any dev_get_drvdata(&chip->dev) in the tree even on top of the history. Did i overlooked at it ?
Shouldn't dev_set_drvdata(&chip->dev, chip); in tpm_sysfs_add_device be removed ?

I plan to release the next serie about tpm_vendor_specific cleanup by tomorrow at least for further discussion :).

So, if you are planning a tpm_get_drvdata or whatever then we should
probably patch vtpm to put priv back in or patch vtpm to not need that
hack.

Jason

Best Regards
Christophe