From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Berger Subject: Re: tpm: Introduce TPM_CHIP_FLAG_VIRTUAL Date: Thu, 31 Mar 2016 09:57:57 -0400 Message-ID: <56FD2CE5.2080703@linux.vnet.ibm.com> References: <56FD0D77.6090905@linux.vnet.ibm.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1258700318775612657==" Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: tpmdd-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Christophe Ricard Cc: "tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org" List-Id: tpmdd-devel@lists.sourceforge.net This is a multi-part message in MIME format. --===============1258700318775612657== Content-Type: multipart/alternative; boundary="------------090907010200010002020500" This is a multi-part message in MIME format. --------------090907010200010002020500 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit 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 >: > > 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 > > --------------090907010200010002020500 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit
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



--------------090907010200010002020500-- --===============1258700318775612657== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline ------------------------------------------------------------------------------ Transform Data into Opportunity. Accelerate data analysis in your applications with Intel Data Analytics Acceleration Library. Click to learn more. http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140 --===============1258700318775612657== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ tpmdd-devel mailing list tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/tpmdd-devel --===============1258700318775612657==--