| Message ID | 1455885728-10315-9-git-send-email-stefanb@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Feb 19, 2016 at 07:42:05AM -0500, Stefan Berger wrote: > cdev_init(&chip->cdev, &tpm_fops); > - chip->cdev.owner = dev->driver->owner; Shouldn't this be: chip->cdev.owner = THIS_MODULE; ? Please drop this hunk into the 'tpm: Get rid of module locking' patch > + > + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) > + err = sysfs_create_group(&chip->dev.parent->kobj, > + &tpm_dev_group); > + else > + chip->groups[chip->groups_cnt++] = &tpm_dev_group; For other reviewers: The intent here is to make another patch that moves the main sysfs to only use groups with symlinks link ppi does now. Does sysfs work with vtpm? I am worried about dev_get_drvdata here (and other places): static ssize_t durations_show(struct device *dev, struct device_attribute *attr, char *buf) { struct tpm_chip *chip = dev_get_drvdata(dev); There is only one dev_set_drvdata and it does not apply to chip->dev. Ultimately this should change to container_of like ppi does. Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 02:19:22 PM: > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > To: Stefan Berger <stefanb@linux.vnet.ibm.com> > Cc: tpmdd-devel@lists.sourceforge.net > Date: 02/22/2016 02:19 PM > Subject: Re: [tpmdd-devel] [PATCH v3 08/11] tpm: Introduce > TPM_CHIP_FLAG_VIRTUAL > > On Fri, Feb 19, 2016 at 07:42:05AM -0500, Stefan Berger wrote: > > > cdev_init(&chip->cdev, &tpm_fops); > > - chip->cdev.owner = dev->driver->owner; > > Shouldn't this be: > > chip->cdev.owner = THIS_MODULE; > > ? > > Please drop this hunk into the 'tpm: Get rid of module locking' patch Done. > > > + > > + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) > > + err = sysfs_create_group(&chip->dev.parent->kobj, > > + &tpm_dev_group); > > + else > > + chip->groups[chip->groups_cnt++] = &tpm_dev_group; > > For other reviewers: The intent here is to make another patch that > moves the main sysfs to only use groups with symlinks link ppi does > now. > > Does sysfs work with vtpm? I am worried about dev_get_drvdata here > (and other places): > > static ssize_t durations_show(struct device *dev, struct > device_attribute *attr, > char *buf) > { > struct tpm_chip *chip = dev_get_drvdata(dev); > > There is only one dev_set_drvdata and it does not apply to chip->dev. > > Ultimately this should change to container_of like ppi does. Fix in this patch or yet another one ? Stefan > > Jason > > ------------------------------------------------------------------------------ > Site24x7 APM Insight: Get Deep Visibility into Application Performance > APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month > Monitor end-to-end web transactions and take corrective actions now > Troubleshoot faster and improve end-user experience. Signup Now! > http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140 > _______________________________________________ > tpmdd-devel mailing list > tpmdd-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tpmdd-devel > ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 02:19:22 PM: > > + > > + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) > > + err = sysfs_create_group(&chip->dev.parent->kobj, > > + &tpm_dev_group); > > + else > > + chip->groups[chip->groups_cnt++] = &tpm_dev_group; > > For other reviewers: The intent here is to make another patch that > moves the main sysfs to only use groups with symlinks link ppi does > now. > > Does sysfs work with vtpm? I am worried about dev_get_drvdata here > (and other places): Yes, works. I have a set_drv_data in the vtpm driver. Stefan ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 08:21:09PM -0500, Stefan Berger wrote: > > Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 02:19:22 > PM: > > > > > + > > > + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) > > > + err = sysfs_create_group(&chip->dev.parent->kobj, > > > + &tpm_dev_group); > > > + else > > > + chip->groups[chip->groups_cnt++] = &tpm_dev_group; > > > > For other reviewers: The intent here is to make another patch that > > moves the main sysfs to only use groups with symlinks link ppi does > > now. > > > > Does sysfs work with vtpm? I am worried about dev_get_drvdata here > > (and other places): > > Yes, works. I have a set_drv_data in the vtpm driver. ?? I don't see that in the vtpm patch posted to the list? Can you point me to it? In any event, the core code should be self contained and not rely on things in the drivers like this. Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
On Mon, Feb 22, 2016 at 08:20:03PM -0500, Stefan Berger wrote: > > Ultimately this should change to container_of like ppi does. > > Fix in this patch or yet another one ? It would have to go with a future patch to change always use compat symlinks for the parent. Jason ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/2016 09:05:15 PM: > > On Mon, Feb 22, 2016 at 08:21:09PM -0500, Stefan Berger wrote: > > > > Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote on 02/22/ > 2016 02:19:22 > > PM: > > > > > > > > + > > > > + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) > > > > + err = sysfs_create_group(&chip->dev.parent->kobj, > > > > + &tpm_dev_group); > > > > + else > > > > + chip->groups[chip->groups_cnt++] = &tpm_dev_group; > > > > > > For other reviewers: The intent here is to make another patch that > > > moves the main sysfs to only use groups with symlinks link ppi does > > > now. > > > > > > Does sysfs work with vtpm? I am worried about dev_get_drvdata here > > > (and other places): > > > > Yes, works. I have a set_drv_data in the vtpm driver. > > ?? I don't see that in the vtpm patch posted to the list? Can you > point me to it? That was in the past that I had that call ... when there still was a device and when sysfs was still working. I now converted the dev_get_drvdata in tpm-interface.c and tpm-sysfs.c to container_of to make sysfs work for vtpm. It has to be part of this patch. Stefan > > In any event, the core code should be self contained and not rely on > things in the drivers like this. > > Jason > ------------------------------------------------------------------------------ Site24x7 APM Insight: Get Deep Visibility into Application Performance APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month Monitor end-to-end web transactions and take corrective actions now Troubleshoot faster and improve end-user experience. Signup Now! http://pubads.g.doubleclick.net/gampad/clk?id=272487151&iu=/4140
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 4fd36ba..2585f6a 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -168,9 +168,7 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev, chip->dev.class = tpm_class; chip->dev.release = tpm_dev_release; chip->dev.parent = dev; -#ifdef CONFIG_ACPI chip->dev.groups = chip->groups; -#endif if (chip->dev_num == 0) chip->dev.devt = MKDEV(MISC_MAJOR, TPM_MINOR); @@ -181,8 +179,10 @@ struct tpm_chip *tpm_chip_alloc(struct device *dev, if (err) goto out; + if (!dev) + chip->flags |= TPM_CHIP_FLAG_VIRTUAL; + cdev_init(&chip->cdev, &tpm_fops); - chip->cdev.owner = dev->driver->owner; chip->cdev.kobj.parent = &chip->dev.kobj; return chip; @@ -318,7 +318,7 @@ int tpm_chip_register(struct tpm_chip *chip) chip->flags |= TPM_CHIP_FLAG_REGISTERED; - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { + if (!(chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))) { rc = __compat_only_sysfs_link_entry_to_kobj( &chip->dev.parent->kobj, &chip->dev.kobj, "ppi"); if (rc && rc != -ENOENT) { @@ -359,7 +359,7 @@ void tpm_chip_unregister(struct tpm_chip *chip) idr_replace(&dev_nums_idr, NULL, chip->dev_num); mutex_unlock(&idr_lock); - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) + if (!(chip->flags & (TPM_CHIP_FLAG_TPM2 | TPM_CHIP_FLAG_VIRTUAL))) sysfs_remove_link(&chip->dev.parent->kobj, "ppi"); tpm1_chip_unregister(chip); diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c index 34e7fc7..127b0e5 100644 --- a/drivers/char/tpm/tpm-sysfs.c +++ b/drivers/char/tpm/tpm-sysfs.c @@ -283,9 +283,13 @@ static const struct attribute_group tpm_dev_group = { 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 + chip->groups[chip->groups_cnt++] = &tpm_dev_group; if (err) dev_err(&chip->dev, @@ -300,5 +304,6 @@ void tpm_sysfs_del_device(struct tpm_chip *chip) * synchronizes this removal so that no callbacks are running or can * run again */ - sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group); + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) + sysfs_remove_group(&chip->dev.parent->kobj, &tpm_dev_group); } diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 2ca5fb4..dc50ca6 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -168,6 +168,7 @@ struct tpm_vendor_specific { enum tpm_chip_flags { TPM_CHIP_FLAG_REGISTERED = BIT(0), TPM_CHIP_FLAG_TPM2 = BIT(1), + TPM_CHIP_FLAG_VIRTUAL = BIT(2), }; struct tpm_chip { @@ -193,9 +194,9 @@ struct tpm_chip { struct dentry **bios_dir; -#ifdef CONFIG_ACPI - const struct attribute_group *groups[2]; + const struct attribute_group *groups[3]; unsigned int groups_cnt; +#ifdef CONFIG_ACPI acpi_handle acpi_dev_handle; char ppi_version[TPM_PPI_VERSION_LEN + 1]; #endif /* CONFIG_ACPI */
Introduce TPM_CHIP_FLAG_VIRTUAL to be used when the chip device has no parent device. Also adapt tpm_chip_alloc so that it can be called with parent device being NULL. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- drivers/char/tpm/tpm-chip.c | 10 +++++----- drivers/char/tpm/tpm-sysfs.c | 13 +++++++++---- drivers/char/tpm/tpm.h | 5 +++-- 3 files changed, 17 insertions(+), 11 deletions(-)