| Message ID | 1489396817-24855-1-git-send-email-Meng.Li@windriver.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote: > From: Limeng <Meng.Li@windriver.com> > > So far, there is not a sysfs interface for user space code to > check the TPM family version(TPM1.x or TPM2). So, add a > file named description in /sys/class/tpm/tpmX/ to show it. > > Signed-off-by: Meng Li <Meng.Li@windriver.com> > --- Is this the first or which version of the patch is this? Version number and changelog are missing :/ /Jarkko > drivers/char/tpm/tpm-chip.c | 70 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 70 insertions(+) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index c406343..b222421 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -36,6 +36,68 @@ > dev_t tpm_devt; > > /** > + * show_description - sysfs interface for checking current TPM hardware version. > + * @dev: pointer to tpm chip device > + * @attr: unused > + * @buf: char buffer to be filled with TPM hardware version info > + * > + * Provides sysfs interface for showing current TPM hardware version. > + */ > +static ssize_t show_description(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct tpm_chip *chip = (struct tpm_chip *)container_of(dev,struct tpm_chip,dev); > + int ret; > + > + if (chip->flags & TPM_CHIP_FLAG_TPM2) > + ret = sprintf(buf, "TPM 2.0"); > + else > + ret = sprintf(buf, "TPM 1.x"); > + > + return ret; > +} > + > +static struct device_attribute tpm_attrs[] = { > + __ATTR(description, S_IRUGO, show_description, NULL), > +}; > + > +/** > + * tpm_create_sysfs - Create tpm sysfs interface. > + * @dev: pointer to tpm chip device > + * > + * Create sysfs interface for checking current TPM hardware version. > + */ > +static int tpm_create_sysfs(struct device *dev) > +{ > + int r, t; > + > + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { > + r = device_create_file(dev, &tpm_attrs[t]); > + if (r) { > + dev_err(dev, "failed to create sysfs file\n"); > + return r; > + } > + } > + > + return 0; > +} > + > +/** > + * tpm_remove_sysfs - Remove tpm sysfs interface. > + * @dev: pointer to tpm chip device > + * > + * Remove sysfs interface for checking current TPM hardware version. > + */ > +static void tpm_remove_sysfs(struct device *dev) > +{ > + int t; > + > + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { > + device_remove_file(dev, &tpm_attrs[t]); > + } > +} > + > +/** > * tpm_try_get_ops() - Get a ref to the tpm_chip > * @chip: Chip to ref > * > @@ -363,6 +425,13 @@ int tpm_chip_register(struct tpm_chip *chip) > return rc; > } > > + rc = tpm_create_sysfs(&chip->dev); > + if (rc) { > + tpm_del_legacy_sysfs(chip); > + tpm_chip_unregister(chip); > + return rc; > + } > + > return 0; > } > EXPORT_SYMBOL_GPL(tpm_chip_register); > @@ -382,6 +451,7 @@ int tpm_chip_register(struct tpm_chip *chip) > */ > void tpm_chip_unregister(struct tpm_chip *chip) > { > + tpm_remove_sysfs(&chip->dev); > tpm_del_legacy_sysfs(chip); > tpm_bios_log_teardown(chip); > tpm_del_char_device(chip); > -- > 1.7.9.5 > ------------------------------------------------------------------------------ Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford
> -----Original Message----- > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com] > Sent: Monday, March 13, 2017 7:54 PM > To: Li, Meng > Cc: linux-kernel@vger.kernel.org; peterhuewe@gmx.de; > tpmdd@selhorst.net; jgunthorpe@obsidianresearch.com; tpmdd- > devel@lists.sourceforge.net > Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM family version > > On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote: > > From: Limeng <Meng.Li@windriver.com> > > > > So far, there is not a sysfs interface for user space code to check > > the TPM family version(TPM1.x or TPM2). So, add a file named > > description in /sys/class/tpm/tpmX/ to show it. > > > > Signed-off-by: Meng Li <Meng.Li@windriver.com> > > --- > > Is this the first or which version of the patch is this? Version number and > changelog are missing :/ Hi Jarkko, This is the second version of this patch. The first one is reviewed by Peter who give out some good advices. It is my first time to submit patch to upstream(main line), and I am not very clear with the submitting rule. So, could you please give me a template to record the version and changing log? In my previous experience, I will attach a patch-0000 to record version and describe the background of this patch. But how to show the changing log when submit patch into mainline, I am not very clear. Regards, Limeng > > /Jarkko > > > drivers/char/tpm/tpm-chip.c | 70 > +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 70 insertions(+) > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > > index c406343..b222421 100644 > > --- a/drivers/char/tpm/tpm-chip.c > > +++ b/drivers/char/tpm/tpm-chip.c > > @@ -36,6 +36,68 @@ > > dev_t tpm_devt; > > > > /** > > + * show_description - sysfs interface for checking current TPM hardware > version. > > + * @dev: pointer to tpm chip device > > + * @attr: unused > > + * @buf: char buffer to be filled with TPM hardware version info > > + * > > + * Provides sysfs interface for showing current TPM hardware version. > > + */ > > +static ssize_t show_description(struct device *dev, > > + struct device_attribute *attr, char *buf) { > > + struct tpm_chip *chip = (struct tpm_chip *)container_of(dev,struct > tpm_chip,dev); > > + int ret; > > + > > + if (chip->flags & TPM_CHIP_FLAG_TPM2) > > + ret = sprintf(buf, "TPM 2.0"); > > + else > > + ret = sprintf(buf, "TPM 1.x"); > > + > > + return ret; > > +} > > + > > +static struct device_attribute tpm_attrs[] = { > > + __ATTR(description, S_IRUGO, show_description, NULL), }; > > + > > +/** > > + * tpm_create_sysfs - Create tpm sysfs interface. > > + * @dev: pointer to tpm chip device > > + * > > + * Create sysfs interface for checking current TPM hardware version. > > + */ > > +static int tpm_create_sysfs(struct device *dev) { > > + int r, t; > > + > > + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { > > + r = device_create_file(dev, &tpm_attrs[t]); > > + if (r) { > > + dev_err(dev, "failed to create sysfs file\n"); > > + return r; > > + } > > + } > > + > > + return 0; > > +} > > + > > +/** > > + * tpm_remove_sysfs - Remove tpm sysfs interface. > > + * @dev: pointer to tpm chip device > > + * > > + * Remove sysfs interface for checking current TPM hardware version. > > + */ > > +static void tpm_remove_sysfs(struct device *dev) { > > + int t; > > + > > + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { > > + device_remove_file(dev, &tpm_attrs[t]); > > + } > > +} > > + > > +/** > > * tpm_try_get_ops() - Get a ref to the tpm_chip > > * @chip: Chip to ref > > * > > @@ -363,6 +425,13 @@ int tpm_chip_register(struct tpm_chip *chip) > > return rc; > > } > > > > + rc = tpm_create_sysfs(&chip->dev); > > + if (rc) { > > + tpm_del_legacy_sysfs(chip); > > + tpm_chip_unregister(chip); > > + return rc; > > + } > > + > > return 0; > > } > > EXPORT_SYMBOL_GPL(tpm_chip_register); > > @@ -382,6 +451,7 @@ int tpm_chip_register(struct tpm_chip *chip) > > */ > > void tpm_chip_unregister(struct tpm_chip *chip) { > > + tpm_remove_sysfs(&chip->dev); > > tpm_del_legacy_sysfs(chip); > > tpm_bios_log_teardown(chip); > > tpm_del_char_device(chip); > > -- > > 1.7.9.5 > > ------------------------------------------------------------------------------ Announcing the Oxford Dictionaries API! The API offers world-renowned dictionary content that is easy and intuitive to access. Sign up for an account today to start using our lexical data to power your apps and projects. Get started today and enter our developer competition. http://sdm.link/oxford
On Mon, Mar 13, 2017 at 12:54:24PM +0000, Li, Meng wrote: > > > > -----Original Message----- > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com] > > Sent: Monday, March 13, 2017 7:54 PM > > To: Li, Meng > > Cc: linux-kernel@vger.kernel.org; peterhuewe@gmx.de; > > tpmdd@selhorst.net; jgunthorpe@obsidianresearch.com; tpmdd- > > devel@lists.sourceforge.net > > Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM family version > > > > On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote: > > > From: Limeng <Meng.Li@windriver.com> > > > > > > So far, there is not a sysfs interface for user space code to check > > > the TPM family version(TPM1.x or TPM2). So, add a file named > > > description in /sys/class/tpm/tpmX/ to show it. > > > > > > Signed-off-by: Meng Li <Meng.Li@windriver.com> > > > --- > > > > Is this the first or which version of the patch is this? Version number and > > changelog are missing :/ > > Hi Jarkko, > > This is the second version of this patch. The first one is reviewed by Peter who give out some good advices. > > It is my first time to submit patch to upstream(main line), A tip for you then. Wait at least one day between patch versions. That gives people time to comment on your patches, and you can fix many different issues at once. > and I am not very clear with the submitting rule. A good start is: Documentation/process/submitting-patches.rst > So, could you please give me a template to record the version and changing log? After the --- write one line per change you made from the previous version. Also, v3 should also contain the v2 change history etc... Picking a random example from the web: http://patchwork.ozlabs.org/patch/710615/ Andrew ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On Mon, Mar 13, 2017 at 12:54:24PM +0000, Li, Meng wrote: > > > > -----Original Message----- > > From: Jarkko Sakkinen [mailto:jarkko.sakkinen@linux.intel.com] > > Sent: Monday, March 13, 2017 7:54 PM > > To: Li, Meng > > Cc: linux-kernel@vger.kernel.org; peterhuewe@gmx.de; > > tpmdd@selhorst.net; jgunthorpe@obsidianresearch.com; tpmdd- > > devel@lists.sourceforge.net > > Subject: Re: [PATCH] tpm: Add sysfs interface to show TPM family version > > > > On Mon, Mar 13, 2017 at 05:20:17PM +0800, Meng.Li@windriver.com wrote: > > > From: Limeng <Meng.Li@windriver.com> > > > > > > So far, there is not a sysfs interface for user space code to check > > > the TPM family version(TPM1.x or TPM2). So, add a file named > > > description in /sys/class/tpm/tpmX/ to show it. > > > > > > Signed-off-by: Meng Li <Meng.Li@windriver.com> > > > --- > > > > Is this the first or which version of the patch is this? Version number and > > changelog are missing :/ > > Hi Jarkko, > > This is the second version of this patch. The first one is reviewed by Peter who give out some good advices. > > It is my first time to submit patch to upstream(main line), and I am not very clear with the submitting rule. > So, could you please give me a template to record the version and changing log? > In my previous experience, I will attach a patch-0000 to record version and describe the background of this patch. > But how to show the changing log when submit patch into mainline, I am not very clear. > > Regards, > Limeng Maybe this will help for the moment: https://kernelnewbies.org/FirstKernelPatch#head-5c81b3c517a1d0bbc24f92594cb734e155fcbbcb In the long run you probably should check this too: https://www.kernel.org/doc/html/latest/process/submitting-patches.html You probably should document in the commit message why this new sysfs attribute is needed. Saying that it is needed to detect the TPM version does not give any insight of any actual workload. User space facing things are something where I rather not apply something unless it is absolutely essetial because they have to be maintained forever. In addition this looks odd: if (chip->flags & TPM_CHIP_FLAG_TPM2) ret = sprintf(buf, "TPM 2.0"); else ret = sprintf(buf, "TPM 1.x"); I do not undertand for what specific purpose is the string "TPM " before the family number is needed. And I'm not sure whether the attribute should have both major and minor number, or should they be separate attributes. For this the commit message does not give any insight on the design choice. And please remove the dangling store function... /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On Mon, Mar 13, 2017 at 04:59:58PM +0200, Jarkko Sakkinen wrote: > if (chip->flags & TPM_CHIP_FLAG_TPM2) > ret = sprintf(buf, "TPM 2.0"); > else > ret = sprintf(buf, "TPM 1.x"); And 1.x should be 1.2 or 1.1 Jason ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On Mon, Mar 13, 2017 at 10:55:47AM -0600, Jason Gunthorpe wrote: > On Mon, Mar 13, 2017 at 04:59:58PM +0200, Jarkko Sakkinen wrote: > > if (chip->flags & TPM_CHIP_FLAG_TPM2) > > ret = sprintf(buf, "TPM 2.0"); > > else > > ret = sprintf(buf, "TPM 1.x"); > > And 1.x should be 1.2 or 1.1 > > Jason Or maybe just create a patch that adds "family_major", which prints 1 or 2. That is sufficient to pick the right daemon in the user space for example. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
On Mon, 2017-03-13 at 17:20 +0800, Meng.Li@windriver.com wrote: [...] > +/** > + * tpm_create_sysfs - Create tpm sysfs interface. > + * @dev: pointer to tpm chip device > + * > + * Create sysfs interface for checking current TPM hardware version. > + */ > +static int tpm_create_sysfs(struct device *dev) > +{ > + int r, t; > + > + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { > + r = device_create_file(dev, &tpm_attrs[t]); > + if (r) { > + dev_err(dev, "failed to create sysfs > file\n"); > + return r; > + } > + } > + > + return 0; > +} > + > +/** > + * tpm_remove_sysfs - Remove tpm sysfs interface. > + * @dev: pointer to tpm chip device > + * > + * Remove sysfs interface for checking current TPM hardware version. > + */ > +static void tpm_remove_sysfs(struct device *dev) > +{ > + int t; > + > + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { > + device_remove_file(dev, &tpm_attrs[t]); > + } > +} > + For non-dynamic attributes, use the attribute_group structure in the device instead. It will take care of creation and removal automatically. James ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index c406343..b222421 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -36,6 +36,68 @@ dev_t tpm_devt; /** + * show_description - sysfs interface for checking current TPM hardware version. + * @dev: pointer to tpm chip device + * @attr: unused + * @buf: char buffer to be filled with TPM hardware version info + * + * Provides sysfs interface for showing current TPM hardware version. + */ +static ssize_t show_description(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct tpm_chip *chip = (struct tpm_chip *)container_of(dev,struct tpm_chip,dev); + int ret; + + if (chip->flags & TPM_CHIP_FLAG_TPM2) + ret = sprintf(buf, "TPM 2.0"); + else + ret = sprintf(buf, "TPM 1.x"); + + return ret; +} + +static struct device_attribute tpm_attrs[] = { + __ATTR(description, S_IRUGO, show_description, NULL), +}; + +/** + * tpm_create_sysfs - Create tpm sysfs interface. + * @dev: pointer to tpm chip device + * + * Create sysfs interface for checking current TPM hardware version. + */ +static int tpm_create_sysfs(struct device *dev) +{ + int r, t; + + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { + r = device_create_file(dev, &tpm_attrs[t]); + if (r) { + dev_err(dev, "failed to create sysfs file\n"); + return r; + } + } + + return 0; +} + +/** + * tpm_remove_sysfs - Remove tpm sysfs interface. + * @dev: pointer to tpm chip device + * + * Remove sysfs interface for checking current TPM hardware version. + */ +static void tpm_remove_sysfs(struct device *dev) +{ + int t; + + for (t = 0; t < ARRAY_SIZE(tpm_attrs); t++) { + device_remove_file(dev, &tpm_attrs[t]); + } +} + +/** * tpm_try_get_ops() - Get a ref to the tpm_chip * @chip: Chip to ref * @@ -363,6 +425,13 @@ int tpm_chip_register(struct tpm_chip *chip) return rc; } + rc = tpm_create_sysfs(&chip->dev); + if (rc) { + tpm_del_legacy_sysfs(chip); + tpm_chip_unregister(chip); + return rc; + } + return 0; } EXPORT_SYMBOL_GPL(tpm_chip_register); @@ -382,6 +451,7 @@ int tpm_chip_register(struct tpm_chip *chip) */ void tpm_chip_unregister(struct tpm_chip *chip) { + tpm_remove_sysfs(&chip->dev); tpm_del_legacy_sysfs(chip); tpm_bios_log_teardown(chip); tpm_del_char_device(chip);