All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
To: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Peter Huewe <peterhuewe@gmx.de>,
	Ashley Lai <ashley@ashleylai.com>,
	Marcel Selhorst <tpmdd@selhorst.net>,
	tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
	josh.triplett@intel.com, christophe.ricard@gmail.com,
	jason.gunthorpe@obsidianresearch.com
Subject: Re: [PATCH v5 7/7] tpm: create TPM 2.0 devices using own device class
Date: Thu, 6 Nov 2014 10:58:06 +0200	[thread overview]
Message-ID: <20141106085805.GA4988@intel.com> (raw)
In-Reply-To: <20141105174836.GA5296@obsidianresearch.com>

On Wed, Nov 05, 2014 at 10:48:36AM -0700, Jason Gunthorpe wrote:
> On Wed, Nov 05, 2014 at 09:40:29AM +0200, Jarkko Sakkinen wrote:
> 
> > > I mean, if we have a patch that does:
> > > 
> > > struct tpm_chip {
> > >    struct device cdev;  // the class device
> > >    struct device *pdev; // the 'platform' device chip is bound too
> > > 
> > >    struct device *dev = pdev; // Temporary Compatability
> > > [+ device_register/etc/etc]
> > > 
> > > Then both cdev and pdev should always be valid. We should not have cdev
> > > be valid for TPM2 and invalid for TPM1, that is just a big mess.
> 
> > As a first patch I'll do a patch that does dev -> pdev rename and
> > nothing else. IMHO it's clean and easy to review if no other changes
> > are contained. One reason for this is obviously that I want to use
> > cdev for struct cdev not for the class device.
> 
> Well, once you add cdev, pdev and dev, you want most uses of dev to
> become cdev and some uses to become pdev.

struct cdev uses variable name 'cdev' in my previous patch set so 
this naming scheme doesn't work.

I do not understand why dev -> pdev rename as the very first commit
would be such a bad thing. It's anyway better variable name for it.
After that variable name 'dev' is free to be used for class device.

> Just bulk renaming dev -> pdev and then bulk renaming pdev -> cdev
> seems like lots of churn...

I'm not planning to do such thing. In the end there is:

   struct device dev;
   struct device *pdev;
   struct cdev cdev;

This will require only one "rename commit".

What I'm going to take from your feedback is to make /dev/tpm0 special
case when considering major:minor just like you proposed. It's very good
idea.

Other thing I'm going to do is to leave PPI sysfs interface to 
tpmX/device/ppi at least for TPM1 devices.

I'll bake the next patch set hopefully before the end of this week.
Lets see after that if it stil need refinement or breaking up to 
smaller patches.

> Jason

/Jarkko

  reply	other threads:[~2014-11-06  8:58 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-01  9:01 [PATCH v5 0/7] TPM 2.0 support Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 1/7] tpm: merge duplicate transmit_cmd() functions Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 2/7] tpm: two-phase chip management functions Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 3/7] tpm: fix multiple race conditions in tpm_ppi.c Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 4/7] tpm: TPM 2.0 baseline support Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 5/7] tpm: TPM 2.0 CRB Interface Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 6/7] tpm: TPM 2.0 FIFO Interface Jarkko Sakkinen
2014-11-01  9:01 ` [PATCH v5 7/7] tpm: create TPM 2.0 devices using own device class Jarkko Sakkinen
2014-11-02 21:33   ` Jason Gunthorpe
2014-11-03  5:41     ` Jarkko Sakkinen
2014-11-03 21:38       ` Jason Gunthorpe
2014-11-04 11:47         ` Jarkko Sakkinen
2014-11-04 12:05           ` Jarkko Sakkinen
2014-11-04 18:14           ` Jason Gunthorpe
2014-11-04 20:38             ` Jarkko Sakkinen
2014-11-04 20:49               ` Jason Gunthorpe
2014-11-04 23:00                 ` Jarkko Sakkinen
2014-11-05  7:40             ` Jarkko Sakkinen
2014-11-05 17:48               ` Jason Gunthorpe
2014-11-06  8:58                 ` Jarkko Sakkinen [this message]
2014-11-01  9:33 ` [PATCH v5 0/7] TPM 2.0 support Jarkko Sakkinen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141106085805.GA4988@intel.com \
    --to=jarkko.sakkinen@linux.intel.com \
    --cc=ashley@ashleylai.com \
    --cc=christophe.ricard@gmail.com \
    --cc=jason.gunthorpe@obsidianresearch.com \
    --cc=jgunthorpe@obsidianresearch.com \
    --cc=josh.triplett@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    --cc=tpmdd-devel@lists.sourceforge.net \
    --cc=tpmdd@selhorst.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.