From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751865AbcGRTUp (ORCPT ); Mon, 18 Jul 2016 15:20:45 -0400 Received: from mga01.intel.com ([192.55.52.88]:54427 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751013AbcGRTUn (ORCPT ); Mon, 18 Jul 2016 15:20:43 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,385,1464678000"; d="scan'208";a="997733581" Date: Mon, 18 Jul 2016 22:20:35 +0300 From: Jarkko Sakkinen To: Andrey Pronin Cc: Jason Gunthorpe , Peter Huewe , Marcel Selhorst , 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: <20160718192035.GQ31463@intel.com> References: <1468547496-16215-1-git-send-email-apronin@chromium.org> <1468547496-16215-3-git-send-email-apronin@chromium.org> <20160715032327.GF9347@obsidianresearch.com> <20160715033530.GB27104@apronin> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20160715033530.GB27104@apronin> 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 08:35:30PM -0700, Andrey Pronin wrote: > On Thu, Jul 14, 2016 at 09:23:27PM -0600, Jason Gunthorpe wrote: > > On Thu, Jul 14, 2016 at 06:51:36PM -0700, Andrey Pronin wrote: > > > - WARN_ON(chip->groups_cnt != 0); > > > > Nope. > > > > > - 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]; > > > > The prior patch needed to have groups[4], every patch much work. > > > > > + if (priv->phy_ops->attr_group) > > > + chip->groups[chip->groups_cnt++] = priv->phy_ops->attr_group; > > > > I am really not excited about having driver specific sysfs > > files. > > > > What is the justification for this? > > > > Jason > > Justification: give access to vendor-specific properties that are > specific to a particular chip and its registers. Please come with a vendor specific property or have this part of a series where the need becomes somehow obvious so that we can talk about a real problem and not in an abstract level. Making user API vendor wobbling is almost over my dead body type of thing but given the context there might be alternatives to consider. I honestly don't understand why this was even bundled with TPM2 patch. /Jarkko