On 03/31/2016 09:06 AM, Christophe Ricard wrote:
Hi Stefan,

[Adding tpmdd mailing list].
I agree that it would work when calling tpm_chip_alloc with NULL, my only worry is that it is not chain the same way in other drivers.
Currently we are not using chip->dev.drvdata to store chip but platform data.
chip is stored into the device platform data from the physical layer (e.g: i2c, spi, pdev...).

Yes, aware of that.


At the end, I think this would not cause any issue as TPM_CHIP_FLAG_VIRTUAL will covert it. However in the current form i believe
It looks like priv field will be required only for the purpose of vtpm_proxy.

Yes. And a priv field could be introduced in the chip structure to be used by the tpm_vtpm_proxy driver.


After previous discussion with Jarkko and Jason, priv is currently on its way to get removed... https://patchwork.kernel.org/patch/8705291/

Seen it.

Would it be reasonable to use chip->dev.platform_data to store the newly allocated proxy_dev instead of using a dedicated field (priv)?

That's a possibility we could look into.

      Stefan



Best Regards
Christophe


2016-03-31 13:43 GMT+02:00 Stefan Berger <stefanb-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>:
On 03/30/2016 05:56 AM, Christophe Ricard wrote:
Hi Stefan,

One question here:
  int tpm_sysfs_add_device(struct tpm_chip *chip)
  {
- int err;
- err = sysfs_create_group(&chip->dev.parent->kobj,
- &tpm_dev_group);
+ int err = 0;
+
+ if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL))
+ err = sysfs_create_group(&chip->dev.parent->kobj,
+ &tpm_dev_group);
+ else {
+ dev_set_drvdata(&chip->dev, chip);
What is the purpose of setting chip->dev.drvdata with chip ?
I don't see any place with a dev_get_drvdata. Can you explain ?

 The subsequent patches implement a device driver that calls tpm_chip_alloc() with NULL for the driver parameter and dev_set_drvdata(), that typically gets called in tpmm_chip_alloc(), is not called. There are dev_get_drvdata() calls in tpm-sysfs.c that would not work without us calling dev_set_drvdata() here.

   Stefan