Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On 04/22/2016 11:06 AM, Jarkko Sakkinen wrote: On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote: From: Jason Gunthorpe The final thing preventing this was the way the sysfs files were attached to the pdev. Follow the approach developed for ppi and move the sysfs files to the chip->dev with symlinks from the pdev for compatibility. Everything in the core now sanely uses container_of to get the chip. Signed-off-by: Jason Gunthorpe Signed-off-by: Stefan Berger Tested-by: Jarkko Sakkinen Three configurations: * Haswell NUC with PTT (tpm_crb) * Another NUC with dTPM 2.0 chip * Dell E6420, which has TPM 1.2 chip Things seem to be unbroken. Stefan, have you verified that sysfs attributes work through routes: 1. From char device sysfs directory 2. Through link Tested-by: Stefan Berger An also tried insmod/rmmod couple of rounds? Works as-is in polling mode but requires Jason's patch "tpm: Fix IRQ unwind ordering in TIS" for interrupt mode to not throw errors on 'rmmod'. Tested-by: Stefan Berger
Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> From: Jason Gunthorpe
>
> The final thing preventing this was the way the sysfs files were
> attached to the pdev. Follow the approach developed for ppi and move
> the sysfs files to the chip->dev with symlinks from the pdev
> for compatibility. Everything in the core now sanely uses container_of
> to get the chip.
>
> Signed-off-by: Jason Gunthorpe
> Signed-off-by: Stefan Berger
Tested-by: Jarkko Sakkinen
Three configurations:
* Haswell NUC with PTT (tpm_crb)
* Another NUC with dTPM 2.0 chip
* Dell E6420, which has TPM 1.2 chip
Things seem to be unbroken.
Stefan, have you verified that sysfs attributes work through routes:
1. From char device sysfs directory
2. Through link
An also tried insmod/rmmod couple of rounds?
If you have and things continue work could you give this Tested-by?
/Jarkko
> ---
> drivers/char/tpm/tpm-chip.c | 73
>
> drivers/char/tpm/tpm-interface.c | 7 ++--
> drivers/char/tpm/tpm-sysfs.c | 61 ++---
> drivers/char/tpm/tpm.h | 10 +++---
> 4 files changed, 84 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5bc530c..7e2c9cf 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -170,9 +170,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);
> @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>
> static int tpm1_chip_register(struct tpm_chip *chip)
> {
> - int rc;
> -
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> return 0;
>
> - rc = tpm_sysfs_add_device(chip);
> - if (rc)
> - return rc;
> + tpm_sysfs_add_device(chip);
>
> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>
> @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>
> if (chip->bios_dir)
> tpm_bios_log_teardown(chip->bios_dir);
> +}
> +
> +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return;
>
> - tpm_sysfs_del_device(chip);
> + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> +
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> }
>
> +/* For compatibility with legacy sysfs paths we provide symlinks from the
> + * parent dev directory to selected names within the tpm chip directory. Old
> + * kernel versions created these files directly under the parent.
> + */
> +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> + int rc;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return 0;
> +
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> + if (rc && rc != -ENOENT)
> + return rc;
> +
> + /* All the names from tpm-sysfs */
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> + if (rc) {
> + tpm_del_legacy_sysfs(chip);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> /*
> * tpm_chip_register() - create a character device for the TPM chip
> * @chip: TPM chip to use.
> @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> tpm_add_ppi(chip);
>
> rc = tpm_add_char_device(chip);
> - if (rc)
> - goto out_err;
> + if (rc) {
> + tpm1_chip_unregister(chip);
> + return rc;
> + }
>
> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> - rc = __compat_only_sysfs_link_entry_to_kobj(
> - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> - if (rc && rc != -ENOENT) {
> - tpm_chip_unregister(chip);
> - return rc;
> - }
> + rc = tpm_add_legacy_sysfs(chip);
> + if (rc) {
> + tpm_chip_unregister(chip);
> + return rc;
> }
>
> return 0;
> -out_err:
> - tpm1_chip_unregister(chip);
> - return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_chip_register);
>
> @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> return;
>
> - if (!
Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On Tue, Apr 19, 2016 at 11:06:45AM -0600, Jason Gunthorpe wrote: > On Tue, Apr 19, 2016 at 06:36:22AM -0400, Stefan Berger wrote: > > On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote: > > >On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote: > > >>From: Jason Gunthorpe > > >> > > >>The final thing preventing this was the way the sysfs files were > > >>attached to the pdev. Follow the approach developed for ppi and move > > >>the sysfs files to the chip->dev with symlinks from the pdev > > >>for compatibility. Everything in the core now sanely uses container_of > > >>to get the chip. > > >Can you give me a quick recap why this patch was mandatory to make the > > >patch set work? Which regression in the earlier versions of the patch > > >set this fixes? > > > > The below patch removes usage of dev_get_drvdata() for retrieving the chip. > > With Christophe's series dropping the priv field I now can use the drvdata > > to store proxy_dev rather than re-introducing the priv field in the chip > > structure. Besides that it doesn't seem necessary to use the drvdata field > > to get from the chip to the device if a simple container_of can do it. > > More specifically, since the vtpm patches use a NULL parent, the > approach of putting the sysfs files on the parent is no longer > workable. > > The early vtpm patches simply moved the sysfs files to the tpm_chip > when a parent is NULL, which is inconsistent for userspace. This also > created a problem where drvdata on the chip now had to point back to > the chip, meaning it became unusable for its new intended purpose. > > The fix is to make everything uniform and put the sysfs files in the > correct place for all drivers (under the chip) and use symlinks for > compat. OK, thanks for explaining this, make perfect sense now. I'll do a more detailed review (and testing) later on. > Jason /Jarkko
Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On Tue, Apr 19, 2016 at 06:36:22AM -0400, Stefan Berger wrote:
> On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote:
> >On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> >>From: Jason Gunthorpe
> >>
> >>The final thing preventing this was the way the sysfs files were
> >>attached to the pdev. Follow the approach developed for ppi and move
> >>the sysfs files to the chip->dev with symlinks from the pdev
> >>for compatibility. Everything in the core now sanely uses container_of
> >>to get the chip.
> >Can you give me a quick recap why this patch was mandatory to make the
> >patch set work? Which regression in the earlier versions of the patch
> >set this fixes?
>
> The below patch removes usage of dev_get_drvdata() for retrieving the chip.
> With Christophe's series dropping the priv field I now can use the drvdata
> to store proxy_dev rather than re-introducing the priv field in the chip
> structure. Besides that it doesn't seem necessary to use the drvdata field
> to get from the chip to the device if a simple container_of can do it.
Right, ok sounds justified.
> Stefan
>
> >
> >/Jarkko
> >
> >>Signed-off-by: Jason Gunthorpe
> >>Signed-off-by: Stefan Berger
> >>---
> >> drivers/char/tpm/tpm-chip.c | 73
> >>
> >> drivers/char/tpm/tpm-interface.c | 7 ++--
> >> drivers/char/tpm/tpm-sysfs.c | 61 ++---
> >> drivers/char/tpm/tpm.h | 10 +++---
> >> 4 files changed, 84 insertions(+), 67 deletions(-)
> >>
> >>diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> >>index 5bc530c..7e2c9cf 100644
> >>--- a/drivers/char/tpm/tpm-chip.c
> >>+++ b/drivers/char/tpm/tpm-chip.c
> >>@@ -170,9 +170,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);
> >>@@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
> >> static int tpm1_chip_register(struct tpm_chip *chip)
> >> {
> >>- int rc;
> >>-
> >>if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >>return 0;
> >>- rc = tpm_sysfs_add_device(chip);
> >>- if (rc)
> >>- return rc;
> >>+ tpm_sysfs_add_device(chip);
> >>chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
> >>@@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip
> >>*chip)
> >>if (chip->bios_dir)
> >>tpm_bios_log_teardown(chip->bios_dir);
> >>+}
> >>+
> >>+static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> >>+{
> >>+ struct attribute **i;
> >>+
> >>+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >>+ return;
> >>- tpm_sysfs_del_device(chip);
> >>+ sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> >>+
> >>+ for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> >>+ sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> >> }
> >>+/* For compatibility with legacy sysfs paths we provide symlinks from the
> >>+ * parent dev directory to selected names within the tpm chip directory.
> >>Old
> >>+ * kernel versions created these files directly under the parent.
> >>+ */
> >>+static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> >>+{
> >>+ struct attribute **i;
> >>+ int rc;
> >>+
> >>+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
> >>+ return 0;
> >>+
> >>+ rc = __compat_only_sysfs_link_entry_to_kobj(
> >>+ &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> >>+ if (rc && rc != -ENOENT)
> >>+ return rc;
> >>+
> >>+ /* All the names from tpm-sysfs */
> >>+ for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> >>+ rc = __compat_only_sysfs_link_entry_to_kobj(
> >>+ &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> >>+ if (rc) {
> >>+ tpm_del_legacy_sysfs(chip);
> >>+ return rc;
> >>+ }
> >>+ }
> >>+
> >>+ return 0;
> >>+}
> >> /*
> >> * tpm_chip_register() - create a character device for the TPM chip
> >> * @chip: TPM chip to use.
> >>@@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> >>tpm_add_ppi(chip);
> >>rc = tpm_add_char_device(chip);
> >>- if (rc)
> >>- goto out_err;
> >>+ if (rc) {
> >>+ tpm1_chip_unregister(chip);
> >>+ return rc;
> >>+ }
> >>chip->flags |= TPM_CHIP_FLAG_REGISTERED;
> >>- if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> >>- rc = __compat_only_sysfs_link_entry_to_kobj(
> >>- &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> >>- if (rc && rc != -ENOENT) {
> >>- tpm_chip_unregister(chip);
> >>- return rc;
> >>- }
> >>+ rc = tpm_add_legacy_sysfs(chip);
> >>+ if (rc) {
> >>+
Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On Tue, Apr 19, 2016 at 06:36:22AM -0400, Stefan Berger wrote: > On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote: > >On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote: > >>From: Jason Gunthorpe > >> > >>The final thing preventing this was the way the sysfs files were > >>attached to the pdev. Follow the approach developed for ppi and move > >>the sysfs files to the chip->dev with symlinks from the pdev > >>for compatibility. Everything in the core now sanely uses container_of > >>to get the chip. > >Can you give me a quick recap why this patch was mandatory to make the > >patch set work? Which regression in the earlier versions of the patch > >set this fixes? > > The below patch removes usage of dev_get_drvdata() for retrieving the chip. > With Christophe's series dropping the priv field I now can use the drvdata > to store proxy_dev rather than re-introducing the priv field in the chip > structure. Besides that it doesn't seem necessary to use the drvdata field > to get from the chip to the device if a simple container_of can do it. More specifically, since the vtpm patches use a NULL parent, the approach of putting the sysfs files on the parent is no longer workable. The early vtpm patches simply moved the sysfs files to the tpm_chip when a parent is NULL, which is inconsistent for userspace. This also created a problem where drvdata on the chip now had to point back to the chip, meaning it became unusable for its new intended purpose. The fix is to make everything uniform and put the sysfs files in the correct place for all drivers (under the chip) and use symlinks for compat. Jason
Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On 04/19/2016 06:12 AM, Jarkko Sakkinen wrote:
On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
From: Jason Gunthorpe
The final thing preventing this was the way the sysfs files were
attached to the pdev. Follow the approach developed for ppi and move
the sysfs files to the chip->dev with symlinks from the pdev
for compatibility. Everything in the core now sanely uses container_of
to get the chip.
Can you give me a quick recap why this patch was mandatory to make the
patch set work? Which regression in the earlier versions of the patch
set this fixes?
The below patch removes usage of dev_get_drvdata() for retrieving the
chip. With Christophe's series dropping the priv field I now can use the
drvdata to store proxy_dev rather than re-introducing the priv field in
the chip structure. Besides that it doesn't seem necessary to use the
drvdata field to get from the chip to the device if a simple
container_of can do it.
Stefan
/Jarkko
Signed-off-by: Jason Gunthorpe
Signed-off-by: Stefan Berger
---
drivers/char/tpm/tpm-chip.c | 73
drivers/char/tpm/tpm-interface.c | 7 ++--
drivers/char/tpm/tpm-sysfs.c | 61 ++---
drivers/char/tpm/tpm.h | 10 +++---
4 files changed, 84 insertions(+), 67 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 5bc530c..7e2c9cf 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -170,9 +170,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);
@@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
static int tpm1_chip_register(struct tpm_chip *chip)
{
- int rc;
-
if (chip->flags & TPM_CHIP_FLAG_TPM2)
return 0;
- rc = tpm_sysfs_add_device(chip);
- if (rc)
- return rc;
+ tpm_sysfs_add_device(chip);
chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
@@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
if (chip->bios_dir)
tpm_bios_log_teardown(chip->bios_dir);
+}
+
+static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
+{
+ struct attribute **i;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ return;
- tpm_sysfs_del_device(chip);
+ sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
+
+ for (i = chip->groups[0]->attrs; *i != NULL; ++i)
+ sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
}
+/* For compatibility with legacy sysfs paths we provide symlinks from the
+ * parent dev directory to selected names within the tpm chip directory. Old
+ * kernel versions created these files directly under the parent.
+ */
+static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
+{
+ struct attribute **i;
+ int rc;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ return 0;
+
+ rc = __compat_only_sysfs_link_entry_to_kobj(
+ &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
+ if (rc && rc != -ENOENT)
+ return rc;
+
+ /* All the names from tpm-sysfs */
+ for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
+ rc = __compat_only_sysfs_link_entry_to_kobj(
+ &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
+ if (rc) {
+ tpm_del_legacy_sysfs(chip);
+ return rc;
+ }
+ }
+
+ return 0;
+}
/*
* tpm_chip_register() - create a character device for the TPM chip
* @chip: TPM chip to use.
@@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
tpm_add_ppi(chip);
rc = tpm_add_char_device(chip);
- if (rc)
- goto out_err;
+ if (rc) {
+ tpm1_chip_unregister(chip);
+ return rc;
+ }
chip->flags |= TPM_CHIP_FLAG_REGISTERED;
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
- rc = __compat_only_sysfs_link_entry_to_kobj(
- &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
- if (rc && rc != -ENOENT) {
- tpm_chip_unregister(chip);
- return rc;
- }
+ rc = tpm_add_legacy_sysfs(chip);
+ if (rc) {
+ tpm_chip_unregister(chip);
+ return rc;
}
return 0;
-out_err:
- tpm1_chip_unregister(chip);
- return rc;
}
EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
Re: [PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
On Mon, Apr 18, 2016 at 01:26:13PM -0400, Stefan Berger wrote:
> From: Jason Gunthorpe
>
> The final thing preventing this was the way the sysfs files were
> attached to the pdev. Follow the approach developed for ppi and move
> the sysfs files to the chip->dev with symlinks from the pdev
> for compatibility. Everything in the core now sanely uses container_of
> to get the chip.
Can you give me a quick recap why this patch was mandatory to make the
patch set work? Which regression in the earlier versions of the patch
set this fixes?
/Jarkko
> Signed-off-by: Jason Gunthorpe
> Signed-off-by: Stefan Berger
> ---
> drivers/char/tpm/tpm-chip.c | 73
>
> drivers/char/tpm/tpm-interface.c | 7 ++--
> drivers/char/tpm/tpm-sysfs.c | 61 ++---
> drivers/char/tpm/tpm.h | 10 +++---
> 4 files changed, 84 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 5bc530c..7e2c9cf 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -170,9 +170,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);
> @@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
>
> static int tpm1_chip_register(struct tpm_chip *chip)
> {
> - int rc;
> -
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> return 0;
>
> - rc = tpm_sysfs_add_device(chip);
> - if (rc)
> - return rc;
> + tpm_sysfs_add_device(chip);
>
> chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
>
> @@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
>
> if (chip->bios_dir)
> tpm_bios_log_teardown(chip->bios_dir);
> +}
> +
> +static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return;
>
> - tpm_sysfs_del_device(chip);
> + sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> +
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i)
> + sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
> }
>
> +/* For compatibility with legacy sysfs paths we provide symlinks from the
> + * parent dev directory to selected names within the tpm chip directory. Old
> + * kernel versions created these files directly under the parent.
> + */
> +static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
> +{
> + struct attribute **i;
> + int rc;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2)
> + return 0;
> +
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> + if (rc && rc != -ENOENT)
> + return rc;
> +
> + /* All the names from tpm-sysfs */
> + for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
> + rc = __compat_only_sysfs_link_entry_to_kobj(
> + &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
> + if (rc) {
> + tpm_del_legacy_sysfs(chip);
> + return rc;
> + }
> + }
> +
> + return 0;
> +}
> /*
> * tpm_chip_register() - create a character device for the TPM chip
> * @chip: TPM chip to use.
> @@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
> tpm_add_ppi(chip);
>
> rc = tpm_add_char_device(chip);
> - if (rc)
> - goto out_err;
> + if (rc) {
> + tpm1_chip_unregister(chip);
> + return rc;
> + }
>
> chip->flags |= TPM_CHIP_FLAG_REGISTERED;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> - rc = __compat_only_sysfs_link_entry_to_kobj(
> - &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
> - if (rc && rc != -ENOENT) {
> - tpm_chip_unregister(chip);
> - return rc;
> - }
> + rc = tpm_add_legacy_sysfs(chip);
> + if (rc) {
> + tpm_chip_unregister(chip);
> + return rc;
> }
>
> return 0;
> -out_err:
> - tpm1_chip_unregister(chip);
> - return rc;
> }
> EXPORT_SYMBOL_GPL(tpm_chip_register);
>
> @@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
> if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
> return;
>
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> - sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
> + tpm_del_legacy_sysfs(chip);
>
> tpm1_chip_unregister(chip);
> tpm_del_char_device(chip);
> diff --git a/drivers/char/tpm/tpm-interface.c
> b/d
[PATCH v11 1/4] tpm: Remove all uses of drvdata from the TPM Core
From: Jason Gunthorpe
The final thing preventing this was the way the sysfs files were
attached to the pdev. Follow the approach developed for ppi and move
the sysfs files to the chip->dev with symlinks from the pdev
for compatibility. Everything in the core now sanely uses container_of
to get the chip.
Signed-off-by: Jason Gunthorpe
Signed-off-by: Stefan Berger
---
drivers/char/tpm/tpm-chip.c | 73
drivers/char/tpm/tpm-interface.c | 7 ++--
drivers/char/tpm/tpm-sysfs.c | 61 ++---
drivers/char/tpm/tpm.h | 10 +++---
4 files changed, 84 insertions(+), 67 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 5bc530c..7e2c9cf 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -170,9 +170,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);
@@ -276,14 +274,10 @@ static void tpm_del_char_device(struct tpm_chip *chip)
static int tpm1_chip_register(struct tpm_chip *chip)
{
- int rc;
-
if (chip->flags & TPM_CHIP_FLAG_TPM2)
return 0;
- rc = tpm_sysfs_add_device(chip);
- if (rc)
- return rc;
+ tpm_sysfs_add_device(chip);
chip->bios_dir = tpm_bios_log_setup(dev_name(&chip->dev));
@@ -297,10 +291,50 @@ static void tpm1_chip_unregister(struct tpm_chip *chip)
if (chip->bios_dir)
tpm_bios_log_teardown(chip->bios_dir);
+}
+
+static void tpm_del_legacy_sysfs(struct tpm_chip *chip)
+{
+ struct attribute **i;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ return;
- tpm_sysfs_del_device(chip);
+ sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
+
+ for (i = chip->groups[0]->attrs; *i != NULL; ++i)
+ sysfs_remove_link(&chip->dev.parent->kobj, (*i)->name);
}
+/* For compatibility with legacy sysfs paths we provide symlinks from the
+ * parent dev directory to selected names within the tpm chip directory. Old
+ * kernel versions created these files directly under the parent.
+ */
+static int tpm_add_legacy_sysfs(struct tpm_chip *chip)
+{
+ struct attribute **i;
+ int rc;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2)
+ return 0;
+
+ rc = __compat_only_sysfs_link_entry_to_kobj(
+ &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
+ if (rc && rc != -ENOENT)
+ return rc;
+
+ /* All the names from tpm-sysfs */
+ for (i = chip->groups[0]->attrs; *i != NULL; ++i) {
+ rc = __compat_only_sysfs_link_entry_to_kobj(
+ &chip->dev.parent->kobj, &chip->dev.kobj, (*i)->name);
+ if (rc) {
+ tpm_del_legacy_sysfs(chip);
+ return rc;
+ }
+ }
+
+ return 0;
+}
/*
* tpm_chip_register() - create a character device for the TPM chip
* @chip: TPM chip to use.
@@ -323,24 +357,20 @@ int tpm_chip_register(struct tpm_chip *chip)
tpm_add_ppi(chip);
rc = tpm_add_char_device(chip);
- if (rc)
- goto out_err;
+ if (rc) {
+ tpm1_chip_unregister(chip);
+ return rc;
+ }
chip->flags |= TPM_CHIP_FLAG_REGISTERED;
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
- rc = __compat_only_sysfs_link_entry_to_kobj(
- &chip->dev.parent->kobj, &chip->dev.kobj, "ppi");
- if (rc && rc != -ENOENT) {
- tpm_chip_unregister(chip);
- return rc;
- }
+ rc = tpm_add_legacy_sysfs(chip);
+ if (rc) {
+ tpm_chip_unregister(chip);
+ return rc;
}
return 0;
-out_err:
- tpm1_chip_unregister(chip);
- return rc;
}
EXPORT_SYMBOL_GPL(tpm_chip_register);
@@ -362,8 +392,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
if (!(chip->flags & TPM_CHIP_FLAG_REGISTERED))
return;
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
- sysfs_remove_link(&chip->dev.parent->kobj, "ppi");
+ tpm_del_legacy_sysfs(chip);
tpm1_chip_unregister(chip);
tpm_del_char_device(chip);
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 7cba092..080dade 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -432,12 +432,11 @@ static const struct tpm_input_header tpm_getcap_header = {
.ordinal = TPM_ORD_GET_CAP
};
-ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
+ssize_t tpm_getcap(struct tpm_chip *chip, _
