From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752994AbcERJ7b (ORCPT ); Wed, 18 May 2016 05:59:31 -0400 Received: from mga09.intel.com ([134.134.136.24]:48008 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751950AbcERJ72 (ORCPT ); Wed, 18 May 2016 05:59:28 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,328,1459839600"; d="scan'208";a="704999204" Date: Wed, 18 May 2016 12:59:24 +0300 From: Jarkko Sakkinen To: Jason Gunthorpe Cc: andrew.zamansky@nuvoton.com, tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org Subject: Re: [PATCH v2] tpm: Factor out common startup code Message-ID: <20160518095924.GA7177@intel.com> References: <1463423147-9874-1-git-send-email-jgunthorpe@obsidianresearch.com> <20160517041557.GA29299@intel.com> <20160517165304.GA19976@obsidianresearch.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160517165304.GA19976@obsidianresearch.com> 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 Tue, May 17, 2016 at 10:53:04AM -0600, Jason Gunthorpe wrote: > On Tue, May 17, 2016 at 07:15:57AM +0300, Jarkko Sakkinen wrote: > > On Mon, May 16, 2016 at 12:25:47PM -0600, Jason Gunthorpe wrote: > > > Provide some flags in tpm_class_ops to allow drivers to opt-in to the > > > common startup sequence. This is the sequence used by tpm_tis and > > > tpm_crb. > > > > > > All drivers should set this flag. > > > > The commit message should be a much much more verbose I cannot include > > this without a better explanation. Please update this for the next > > revision. > > What more description do you want to see? It is lacking a lot of relevant information: * It should explain what you mean by startup sequence". * It should describe the constant TPM_OPS_AUTO_STARTUP * It should explain what drivers are doing at the moment (before this feature). * It should explain what is the benefit for different HW drivers after applying this patch. * It should explain why you call the executed sequence "automatic" and also use the word "standard". Yeah, I didn't understand this, this not me being picky. Maybe it should be DEFAULT_STARTUP?? Your commit message is as good as no commit message at all. I only got picture what the patch does by reading the code. I see the commit message as or sometimes more important than the code change itself. /Jarkko