Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753585AbdCMPAT (ORCPT ); Mon, 13 Mar 2017 11:00:19 -0400 Received: from mga14.intel.com ([192.55.52.115]:60171 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753317AbdCMPAJ (ORCPT ); Mon, 13 Mar 2017 11:00:09 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.36,159,1486454400"; d="scan'208";a="74833317" Date: Mon, 13 Mar 2017 16:59:58 +0200 From: Jarkko Sakkinen 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 Message-ID: <20170313145958.zoh2fdxnh3ytyr2z@intel.com> References: <1489396817-24855-1-git-send-email-Meng.Li@windriver.com> <20170313115415.2czyr4woaq4h3ncr@intel.com> <529F9A9100AE8045A7A5B5A00A39FBB83DE9325F@ALA-MBC.corp.ad.wrs.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <529F9A9100AE8045A7A5B5A00A39FBB83DE9325F@ALA-MBC.corp.ad.wrs.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2606 Lines: 67 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 > > > > > > 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 > > > --- > > > > 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