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 
> <mailto: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
>
>