Hi Jimmy,
On Tue, Oct 09, 2012 at 05:35:22PM +0800, gang....@intel.com wrote:
> From: Gang Wei <gang....@intel.com>
>
> This patch try to fix the S3 regression https://lkml.org/lkml/2012/10/5/433,
> which includes below line:
> [ 1554.684638] sysfs: cannot create duplicate filename
> '/devices/pnp0/00:0c/ppi'
>
> The root cause is that ppi sysfs teardown code is MIA, so while S3 resume,
> the ppi kobject will be created again upon existing one.
>
> To make the tear down code simple, change the ppi subfolder creation from
> using kobject_create_and_add to just using a named ppi attribute_group. Then
> ppi sysfs teardown could be done with a simple sysfs_remove_group call.
>
> Adjusted the name & return type for ppi sysfs init function.
>
> Reported-by: Ben Guthro <b...@guthro.net>
> Signed-off-by: Gang Wei <gang....@intel.com>
> ---
> drivers/char/tpm/tpm.c | 3 ++-
> drivers/char/tpm/tpm.h | 9 +++++++--
> drivers/char/tpm/tpm_ppi.c | 18 ++++++++++--------
> 3 files changed, 19 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index f26afdb..b429f1e 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -1259,6 +1259,7 @@ void tpm_remove_hardware(struct device *dev)
>
> misc_deregister(&chip->vendor.miscdev);
> sysfs_remove_group(&dev->kobj, chip->vendor.attr_group);
> + tpm_remove_ppi(&dev->kobj);
> tpm_bios_log_teardown(chip->bios_dir);
>
> /* write it this way to be explicit (chip->dev == dev) */
> @@ -1476,7 +1477,7 @@ struct tpm_chip *tpm_register_hardware(struct device
> *dev,
> goto put_device;
> }
>
> - if (sys_add_ppi(&dev->kobj)) {
> + if (tpm_add_ppi(&dev->kobj)) {
> misc_deregister(&chip->vendor.miscdev);
> goto put_device;
> }
Hmm, tpm_add_ppi is just sysfs_create_group, which only ever returns
0. Looks like we can remove this error path, but PPI is unusable in the
failure case.
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 02c266a..8ef7649 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -329,10 +329,15 @@ extern int wait_for_tpm_stat(struct tpm_chip *, u8,
> unsigned long,
> wait_queue_head_t *);
>
> #ifdef CONFIG_ACPI
> -extern ssize_t sys_add_ppi(struct kobject *parent);
> +extern int tpm_add_ppi(struct kobject *);
> +extern void tpm_remove_ppi(struct kobject *);
> #else
> -static inline ssize_t sys_add_ppi(struct kobject *parent)
> +static inline int tpm_add_ppi(struct kobject *parent)
> {
> return 0;
> }
> +
> +static inline void tpm_remove_ppi(struct kobject *parent)
> +{
> +}
> #endif
> diff --git a/drivers/char/tpm/tpm_ppi.c b/drivers/char/tpm/tpm_ppi.c
> index f27b58c..720ebcf 100644
> --- a/drivers/char/tpm/tpm_ppi.c
> +++ b/drivers/char/tpm/tpm_ppi.c
> @@ -444,18 +444,20 @@ static struct attribute *ppi_attrs[] = {
> &dev_attr_vs_operations.attr, NULL,
> };
> static struct attribute_group ppi_attr_grp = {
> + .name = "ppi",
> .attrs = ppi_attrs
> };
>
> -ssize_t sys_add_ppi(struct kobject *parent)
> +int tpm_add_ppi(struct kobject *parent)
> {
> - struct kobject *ppi;
> - ppi = kobject_create_and_add("ppi", parent);
> - if (sysfs_create_group(ppi, &ppi_attr_grp))
> - return -EFAULT;
> - else
> - return 0;
> + return sysfs_create_group(parent, &ppi_attr_grp);
> +}
> +EXPORT_SYMBOL_GPL(tpm_add_ppi);
> +
> +void tpm_remove_ppi(struct kobject *parent)
> +{
> + sysfs_remove_group(parent, &ppi_attr_grp);
> }
> -EXPORT_SYMBOL_GPL(sys_add_ppi);
> +EXPORT_SYMBOL_GPL(tpm_remove_ppi);
Do we need to export these symbols? These might have been left around
from when ppi was a standalone module.
Kent
> MODULE_LICENSE("GPL");
> --
> 1.7.7.6
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
