From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751923AbcGRTMJ (ORCPT ); Mon, 18 Jul 2016 15:12:09 -0400 Received: from mga14.intel.com ([192.55.52.115]:55248 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751254AbcGRTMG (ORCPT ); Mon, 18 Jul 2016 15:12:06 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,385,1464678000"; d="scan'208";a="141417200" Date: Mon, 18 Jul 2016 22:11:41 +0300 From: Jarkko Sakkinen To: Andrey Pronin Cc: Peter Huewe , Marcel Selhorst , Jason Gunthorpe , tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, groeck@chromium.org, smbarber@chromium.org, dianders@chromium.org Subject: Re: [PATCH 2/2] tpm: support driver-specific sysfs attrs in tpm_tis_core Message-ID: <20160718191141.GO31463@intel.com> References: <1468547496-16215-1-git-send-email-apronin@chromium.org> <1468547496-16215-3-git-send-email-apronin@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1468547496-16215-3-git-send-email-apronin@chromium.org> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote: > Add attr_group to phy_ops that a driver relying on tpm_tis_core_init > can set to have its specific attributes registered in sysfs. > > Signed-off-by: Andrey Pronin > --- > drivers/char/tpm/tpm-sysfs.c | 1 - > drivers/char/tpm/tpm.h | 8 +++++++- > drivers/char/tpm/tpm_tis_core.c | 3 +++ > drivers/char/tpm/tpm_tis_core.h | 1 + > 4 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c > index 95ce90d..22c9874 100644 > --- a/drivers/char/tpm/tpm-sysfs.c > +++ b/drivers/char/tpm/tpm-sysfs.c > @@ -304,7 +304,6 @@ void tpm_sysfs_add_device(struct tpm_chip *chip) > * is called before ops is null'd and the sysfs core synchronizes this > * removal so that no callbacks are running or can run again > */ > - WARN_ON(chip->groups_cnt != 0); You should explain this in a commit message if you want to remove it. In general, this make user space API vendor specific, which is unacceptable. /Jarkko > chip->groups[chip->groups_cnt++] = &tpm_dev_group; > if (chip->flags & TPM_CHIP_FLAG_TPM2) > chip->groups[chip->groups_cnt++] = &tpm2_dev_group; > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index 8890df2..8c69649 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -173,7 +173,13 @@ struct tpm_chip { > > struct dentry **bios_dir; > > - const struct attribute_group *groups[3]; > + /* up to 4 attribute groups: > + * - driver-specific > + * - common TPM1.2 and TPM2.0 > + * - TPM1.2/2.0-specific > + * - ppi > + */ > + const struct attribute_group *groups[5]; > unsigned int groups_cnt; > #ifdef CONFIG_ACPI > acpi_handle acpi_dev_handle; > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c > index 8110b52..6d5d8f4 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -796,6 +796,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq, > } > } > > + if (priv->phy_ops->attr_group) > + chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group; > + > return tpm_chip_register(chip); > out_err: > tpm_tis_remove(chip); > diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h > index 9191aab..4417ed9 100644 > --- a/drivers/char/tpm/tpm_tis_core.h > +++ b/drivers/char/tpm/tpm_tis_core.h > @@ -95,6 +95,7 @@ struct tpm_tis_data { > }; > > struct tpm_tis_phy_ops { > + const struct attribute_group *attr_group; > int (*read_bytes)(struct tpm_tis_data *data, u32 addr, u16 len, > u8 *result); > int (*write_bytes)(struct tpm_tis_data *data, u32 addr, u16 len, > -- > 2.6.6 >