Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Nayna]

Hi

I might not be aware of some background related to TPM device driver 
design and so have got few questions regarding recently added support in 
Nuvoton i2c driver for TPM2.0 
(http://git.infradead.org/users/jjs/linux-tpmdd.git/commit/82cc1a49b6358394938e759dc4c22b2be773bbad). 
I got these questions while testing some TPM2.0 stuff using the kernel 
code from repo having this patch and am using Nuvoton TPM.

#1. It seems that support is added only for following device-ids.
         {.compatible = "nuvoton,npct501"},
         {.compatible = "winbond,wpct301"},
         {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},

So, was wondering about why device id nuvoton,npct650  wasn't added for 
the support ?

Was it expected to work with some wild-card type device-id as specified 
in the first line of description comment of file i.e. npct6XX. ?
/**
* Nuvoton TPM I2C Device Driver Interface for WPCT301/NPCT501/NPCT6XX,
........
**/

Also, the patch log message says "tpm: Add TPM 2.0 support to the 
Nuvoton i2c driver (NPCT6xx family)" i.e. NPCT6xx family, so my 
assumption was that it will work with any NPCT6xx device id.

#2 Was also trying to understand why the .data is hardcoded for 
nuvoton,npct601, but not for other device ids ?

And I see that this device-id .data info is then used in 
i2c_nuvoton_probe() method to set the tpm_chip flag as TPM2 device
        if (dev->of_node) {
                 const struct of_device_id *of_id;
                 printk("dev->of_node if\n");
                 of_id = of_match_device(dev->driver->of_match_table, dev);
                 if (of_id && of_id->data == OF_IS_TPM2)
                 {
                         printk("of_id->data == OF_IS_TPM2\n");
                         chip->flags |= TPM_CHIP_FLAG_TPM2;
                 }
         } else
         {
                 printk("dev->of_node else\n");
                 if (id->driver_data == I2C_IS_TPM2)
                 {

                         printk("id->driver_data == I2C_IS_TPM2\n");
                         chip->flags |= TPM_CHIP_FLAG_TPM2;
                 }
         }


So, why is there hard-coded checking and not using tpm2_probe() method 
which is itself based on direct TPM hardware response for setting the 
TPM2 flag. ? Is there something I am missing in the design which 
mandates to have .data set as OF_IS_TPM2.

Please let me know.


Thanks & Regards,
    - Nayna


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Jason Gunthorpe]

On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
> I got these questions while testing some TPM2.0 stuff using the kernel 
> code from repo having this patch and am using Nuvoton TPM.
> 
> #1. It seems that support is added only for following device-ids.
>          {.compatible = "nuvoton,npct501"},
>          {.compatible = "winbond,wpct301"},
>          {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
> 
> So, was wondering about why device id nuvoton,npct650  wasn't added for 
> the support ?

Yes, that was the device ID list for Nuvoton.

It is convention in device tree to include older device IDs if the
device is compatible.

So you might do

 compatible = "nuvoton,npct650", "nuvoton,npct601"

Andrew, is 601 even the right name?

> Was it expected to work with some wild-card type device-id as specified 
> in the first line of description comment of file i.e. npct6XX. ?

No.

> So, why is there hard-coded checking and not using tpm2_probe() method 
> which is itself based on direct TPM hardware response for setting the 
> TPM2 flag. ? Is there something I am missing in the design which 
> mandates to have .data set as OF_IS_TPM2.

Generally speaking probing is somewhat discouraged, currently we only
probe for PC platform tis (and even that might be a mistake), all
other drivers are designed to be explicit.

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[George Wilson]

On Tue, Jul 26, 2016 at 02:17:11PM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
> > I got these questions while testing some TPM2.0 stuff using the kernel 
> > code from repo having this patch and am using Nuvoton TPM.
> > 
> > #1. It seems that support is added only for following device-ids.
> >          {.compatible = "nuvoton,npct501"},
> >          {.compatible = "winbond,wpct301"},
> >          {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
> > 
> > So, was wondering about why device id nuvoton,npct650  wasn't added for 
> > the support ?
> 
> Yes, that was the device ID list for Nuvoton.
> 
> It is convention in device tree to include older device IDs if the
> device is compatible.
> 
> So you might do
> 
>  compatible = "nuvoton,npct650", "nuvoton,npct601"
> 
> Andrew, is 601 even the right name?
> 
> > Was it expected to work with some wild-card type device-id as specified 
> > in the first line of description comment of file i.e. npct6XX. ?
> 
> No.
> 
> > So, why is there hard-coded checking and not using tpm2_probe() method 
> > which is itself based on direct TPM hardware response for setting the 
> > TPM2 flag. ? Is there something I am missing in the design which 
> > mandates to have .data set as OF_IS_TPM2.
> 
> Generally speaking probing is somewhat discouraged, currently we only
> probe for PC platform tis (and even that might be a mistake), all
> other drivers are designed to be explicit.

How should field upgradable/downgradable TPMs be handled since hardcoding
the version in the device tree might give the wrong answer?  Would early
firmware be expected to probe nonetheless and set the right device tree
property?

> 
> Jason
> 
> ------------------------------------------------------------------------------
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are 
> consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
> J-Flow, sFlow and other flows. Make informed decisions using capacity planning
> reports.http://sdm.link/zohodev2dev
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
> 

-- 
George Wilson
IBM Linux Technology Center
Security Development


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Jason Gunthorpe]

On Tue, Jul 26, 2016 at 03:39:02PM -0500, George Wilson wrote:
> > Generally speaking probing is somewhat discouraged, currently we only
> > probe for PC platform tis (and even that might be a mistake), all
> > other drivers are designed to be explicit.
> 
> How should field upgradable/downgradable TPMs be handled since hardcoding
> the version in the device tree might give the wrong answer?  Would early
> firmware be expected to probe nonetheless and set the right device tree
> property?

Is that a real thing?

Yes, generally Linux expects DT to be set correctly by the boot
firmware. Early firmware needs to know the TPM type anyhow to do the
TPM setup, so this doesn't seem like a realistic scenario.

For TPM we made a somewhat arbitary choice that TPM2 has to be
explicit. If there are real systems that benefit from auto-probing it
could be revisited..

But, to be honest, I'm not certain how robust our probe technique is,
and I think we should avoid probing, since TCG didn't design an
approved detection sequence (??).

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[George Wilson]

On Tue, Jul 26, 2016 at 03:03:44PM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 03:39:02PM -0500, George Wilson wrote:
> > > Generally speaking probing is somewhat discouraged, currently we only
> > > probe for PC platform tis (and even that might be a mistake), all
> > > other drivers are designed to be explicit.
> > 
> > How should field upgradable/downgradable TPMs be handled since hardcoding
> > the version in the device tree might give the wrong answer?  Would early
> > firmware be expected to probe nonetheless and set the right device tree
> > property?
> 
> Is that a real thing?

Yep, it's a thing.  I know of at least 2 parts from 2 different
suppliers that are field upgradable/downgradable.

> 
> Yes, generally Linux expects DT to be set correctly by the boot
> firmware. Early firmware needs to know the TPM type anyhow to do the
> TPM setup, so this doesn't seem like a realistic scenario.

A reset is required after upgrade/downgrade.  But the version still
needs to be detected by the firmware somehow.  It could be configured
manually in firmware state after the upgrade/downgrade to properly set
the property, which seems much less desirable than a probe.

> 
> For TPM we made a somewhat arbitary choice that TPM2 has to be
> explicit. If there are real systems that benefit from auto-probing it
> could be revisited..

I think it's as necessary - at least at the firmware level - as for
x86_64.

> 
> But, to be honest, I'm not certain how robust our probe technique is,
> and I think we should avoid probing, since TCG didn't design an
> approved detection sequence (??).

We did see issues with some older 1.2 TPMs that hung when probed by the
kernel with the wrong device driver but that hasn't been an issue for
some time.  It looks like UEFI ultimately does probe and likely that's
required for other platforms.  I don't think it's safe to assume a
TPM is always loaded with a particular firmware version across hard
resets.

> 
> Jason
> 

-- 
George Wilson
IBM Linux Technology Center
Security Development


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Jason Gunthorpe]

On Wed, Jul 27, 2016 at 11:05:14AM -0500, George Wilson wrote:

> > Yes, generally Linux expects DT to be set correctly by the boot
> > firmware. Early firmware needs to know the TPM type anyhow to do the
> > TPM setup, so this doesn't seem like a realistic scenario.
> 
> A reset is required after upgrade/downgrade.  But the version still
> needs to be detected by the firmware somehow.  It could be configured
> manually in firmware state after the upgrade/downgrade to properly set
> the property, which seems much less desirable than a probe.

Well, the firmware has to take care of this. If the firmware wants to
support switchable firmware it needs to be able to do probe that works
with all the firmware versions.

ACPI and DT both expect the TPM version to be passed to the OS, it is
up to the firmware to do that correctly..

The fact you've already seen TPM2 probing failures makes me very
reluctant to turn it on more broadly, and maybe even think we should
get rid of it from tis too..

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[George Wilson]

On Wed, Jul 27, 2016 at 10:31:52AM -0600, Jason Gunthorpe wrote:
> On Wed, Jul 27, 2016 at 11:05:14AM -0500, George Wilson wrote:
> 
> > > Yes, generally Linux expects DT to be set correctly by the boot
> > > firmware. Early firmware needs to know the TPM type anyhow to do the
> > > TPM setup, so this doesn't seem like a realistic scenario.
> > 
> > A reset is required after upgrade/downgrade.  But the version still
> > needs to be detected by the firmware somehow.  It could be configured
> > manually in firmware state after the upgrade/downgrade to properly set
> > the property, which seems much less desirable than a probe.
> 
> Well, the firmware has to take care of this. If the firmware wants to
> support switchable firmware it needs to be able to do probe that works
> with all the firmware versions.
> 
> ACPI and DT both expect the TPM version to be passed to the OS, it is
> up to the firmware to do that correctly..
> 
> The fact you've already seen TPM2 probing failures makes me very
> reluctant to turn it on more broadly, and maybe even think we should
> get rid of it from tis too..

Cool, that clarifies it precisely:  the expectation is that firmware will
do any necessary probing and pass the property up in the device tree.  All
drivers supporting OF must use the data property as a flag for 2.0 for
models capable of 2.0 function.

> 
> Jason
> 

-- 
George Wilson
IBM Linux Technology Center
Security Development


------------------------------------------------------------------------------

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Dave Heller]

On 07/26/2016 04:17 PM, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
>> I got these questions while testing some TPM2.0 stuff using the kernel
>> code from repo having this patch and am using Nuvoton TPM.
>>
>> #1. It seems that support is added only for following device-ids.
>>           {.compatible = "nuvoton,npct501"},
>>           {.compatible = "winbond,wpct301"},
>>           {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
>>
>> So, was wondering about why device id nuvoton,npct650  wasn't added for
>> the support ?
>
> Yes, that was the device ID list for Nuvoton.
>
> It is convention in device tree to include older device IDs if the
> device is compatible.
>
> So you might do
>
>   compatible = "nuvoton,npct650", "nuvoton,npct601"
>
> Andrew, is 601 even the right name?

Hi Jason, I'm not sure I'm understanding your answer here.  Looks like 
we will have to add a couple of lines to specifically list the npct650, 
why not just using the existing syntax, one device id per line?  Or you 
are saying, since the npct601 and npct650 are similar models, it's 
better to list them on the same line?

>> Was it expected to work with some wild-card type device-id as specified
>> in the first line of description comment of file i.e. npct6XX. ?
>
> No.
>
>> So, why is there hard-coded checking and not using tpm2_probe() method
>> which is itself based on direct TPM hardware response for setting the
>> TPM2 flag. ? Is there something I am missing in the design which
>> mandates to have .data set as OF_IS_TPM2.
>
> Generally speaking probing is somewhat discouraged, currently we only
> probe for PC platform tis (and even that might be a mistake), all
> other drivers are designed to be explicit.
>
> Jason
>
> ------------------------------------------------------------------------------
> What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
> patterns at an interface-level. Reveals which users, apps, and protocols are
> consuming the most bandwidth. Provides multi-vendor support for NetFlow,
> J-Flow, sFlow and other flows. Make informed decisions using capacity planning
> reports.http://sdm.link/zohodev2dev
> _______________________________________________
> tpmdd-devel mailing list
> tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Jason Gunthorpe]

On Wed, Jul 27, 2016 at 10:30:09AM -0400, Dave Heller wrote:
> > So you might do
> >
> >   compatible = "nuvoton,npct650", "nuvoton,npct601"
> >
> > Andrew, is 601 even the right name?
> 
> Hi Jason, I'm not sure I'm understanding your answer here.  Looks like 
> we will have to add a couple of lines to specifically list the npct650, 
> why not just using the existing syntax, one device id per line?  Or you 
> are saying, since the npct601 and npct650 are similar models, it's 
> better to list them on the same line?

The above example would go in your device tree, are you using device tree?

Jason

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Dave Heller]

On 07/27/2016 12:24 PM, Jason Gunthorpe wrote:
> On Wed, Jul 27, 2016 at 10:30:09AM -0400, Dave Heller wrote:
>>> So you might do
>>>
>>>    compatible = "nuvoton,npct650", "nuvoton,npct601"
>>>
>>> Andrew, is 601 even the right name?
>>
>> Hi Jason, I'm not sure I'm understanding your answer here.  Looks like
>> we will have to add a couple of lines to specifically list the npct650,
>> why not just using the existing syntax, one device id per line?  Or you
>> are saying, since the npct601 and npct650 are similar models, it's
>> better to list them on the same line?
>
> The above example would go in your device tree, are you using device tree?
>
> Jason

Thanks, I get in now: you are saying we should add that to our device 
tree and leave the driver code alone.

Here is were my understanding of the device tree entry is a bit lacking" 
maybe this is the convention that is normally followed.  And yes, the 
device tree property is named "compatible".  But this seems intuitively 
backward to me:

Should not the device tree tell you what is actually *there* and let the 
driver code decide what is compatible?  Rather than the firmware trying 
to make a guess as to which Nuvoton device id to try to match in the 
Nuvoton driver code?

Put another way, should the "smarts" as to which Nuvoton part numbers 
are considered compatible to the driver, be done by FW?  Or left to the 
driver?


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports.http://sdm.link/zohodev2dev

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Jason Gunthorpe]

On Wed, Jul 27, 2016 at 01:27:23PM -0400, Dave Heller wrote:
> 
> On 07/27/2016 12:24 PM, Jason Gunthorpe wrote:
> >On Wed, Jul 27, 2016 at 10:30:09AM -0400, Dave Heller wrote:
> >>>So you might do
> >>>
> >>>   compatible = "nuvoton,npct650", "nuvoton,npct601"
> >>>
> >>>Andrew, is 601 even the right name?
> >>
> >>Hi Jason, I'm not sure I'm understanding your answer here.  Looks like
> >>we will have to add a couple of lines to specifically list the npct650,
> >>why not just using the existing syntax, one device id per line?  Or you
> >>are saying, since the npct601 and npct650 are similar models, it's
> >>better to list them on the same line?
> >
> >The above example would go in your device tree, are you using device tree?
> >
> >Jason
> 
> Thanks, I get in now: you are saying we should add that to our device tree
> and leave the driver code alone.
> 
> Here is were my understanding of the device tree entry is a bit lacking"
> maybe this is the convention that is normally followed.  And yes, the device
> tree property is named "compatible".  But this seems intuitively backward to
> me:
> 
> Should not the device tree tell you what is actually *there* and let the
> driver code decide what is compatible?  Rather than the firmware trying to
> make a guess as to which Nuvoton device id to try to match in the Nuvoton
> driver code?

Yes, that is what I suggested:

> >>>   compatible = "nuvoton,npct650", "nuvoton,npct601"

Today's kernel has no idea what 650 is, but since the DT says it is
compatible with the 601 then today's kernel will bind to it. 601 is
the kernel documented compatible tag for that specific I2C
API.

If tomorrow's kernel needs a work around for the 650 then it can start
matching on 650. Generally speaking we don't add new compatible
strings to the kernel unless they actually do something..

The goal is to reduce tight coupling between the kernel and the
DT.

Perhaps we should add 6xx as the generic flavor? I'm not as clear on
what DT maintainers think about that, but there is precedent.

> Put another way, should the "smarts" as to which Nuvoton part numbers are
> considered compatible to the driver, be done by FW?  Or left to the driver?

Done in the firmware. List the most specific thing you can first, then
the generic thing second.

A clearer example would be for the tcg standard methods:

compatible = "st,st33htpm-spi", "tcg,tpm_tis-spi";

Indicating that the device is actually a st33, but implements the
standard TCG interface.

Future kernels might match to st33 to implement bug work arounds.

Jason

------------------------------------------------------------------------------

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[George Wilson]

On Wed, Jul 27, 2016 at 11:42:29AM -0600, Jason Gunthorpe wrote:
> > Should not the device tree tell you what is actually *there* and let the
> > driver code decide what is compatible?  Rather than the firmware trying to
> > make a guess as to which Nuvoton device id to try to match in the Nuvoton
> > driver code?
> 
> Yes, that is what I suggested:
> 
> > >>>   compatible = "nuvoton,npct650", "nuvoton,npct601"
> 
> Today's kernel has no idea what 650 is, but since the DT says it is
> compatible with the 601 then today's kernel will bind to it. 601 is
> the kernel documented compatible tag for that specific I2C
> API.

So in this case, early firmware would simply pass the additional
compatible string.

> Perhaps we should add 6xx as the generic flavor? I'm not as clear on
> what DT maintainers think about that, but there is precedent.

We thought about suggesting that too.  However,
http://elinux.org/Device_Tree_Usage advises:

  Warning: Don't use wildcard compatible values, like
  "fsl,mpc83xx-uart" or similar. Silicon vendors will invariably
  make a change that breaks your wildcard assumptions the moment
  it is too late to change it. Instead, choose a specific silicon
  implementations and make all subsequent silicon compatible with
  it.

It looks like the driver's compatible table is on the right track and
we need to make our platform's device tree conform to its expectations.

-- 
George Wilson
IBM Linux Technology Center
Security Development


------------------------------------------------------------------------------

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Nayna]

Thanks everyone for discussion and clarifying the design points. This 
helps and we can now test by changing our device-tree entries.

Also, we are assuming that just updating .compatible device tree entry 
should do the fix. We don't have to add any additional .data property. 
And driver will match .data value even for nuvoton,npct650 from device 
driver npct601 entry, where .data is explicitly specified.

Thanks & Regards,
    - Nayna

On 07/28/2016 04:04 AM, George Wilson wrote:
> On Wed, Jul 27, 2016 at 11:42:29AM -0600, Jason Gunthorpe wrote:
>>> Should not the device tree tell you what is actually *there* and let the
>>> driver code decide what is compatible?  Rather than the firmware trying to
>>> make a guess as to which Nuvoton device id to try to match in the Nuvoton
>>> driver code?
>>
>> Yes, that is what I suggested:
>>
>>>>>>    compatible = "nuvoton,npct650", "nuvoton,npct601"
>>
>> Today's kernel has no idea what 650 is, but since the DT says it is
>> compatible with the 601 then today's kernel will bind to it. 601 is
>> the kernel documented compatible tag for that specific I2C
>> API.
>
> So in this case, early firmware would simply pass the additional
> compatible string.
>
>> Perhaps we should add 6xx as the generic flavor? I'm not as clear on
>> what DT maintainers think about that, but there is precedent.
>
> We thought about suggesting that too.  However,
> http://elinux.org/Device_Tree_Usage advises:
>
>    Warning: Don't use wildcard compatible values, like
>    "fsl,mpc83xx-uart" or similar. Silicon vendors will invariably
>    make a change that breaks your wildcard assumptions the moment
>    it is too late to change it. Instead, choose a specific silicon
>    implementations and make all subsequent silicon compatible with
>    it.
>
> It looks like the driver's compatible table is on the right track and
> we need to make our platform's device tree conform to its expectations.
>


------------------------------------------------------------------------------

Re: Regarding recently Added TPM2.0 support to the Nuvoton i2c driver

[Jarkko Sakkinen]

On Tue, Jul 26, 2016 at 02:17:11PM -0600, Jason Gunthorpe wrote:
> On Tue, Jul 26, 2016 at 11:44:43PM +0530, Nayna wrote:
> > I got these questions while testing some TPM2.0 stuff using the kernel 
> > code from repo having this patch and am using Nuvoton TPM.
> > 
> > #1. It seems that support is added only for following device-ids.
> >          {.compatible = "nuvoton,npct501"},
> >          {.compatible = "winbond,wpct301"},
> >          {.compatible = "nuvoton,npct601", .data = OF_IS_TPM2},
> > 
> > So, was wondering about why device id nuvoton,npct650  wasn't added for 
> > the support ?
> 
> Yes, that was the device ID list for Nuvoton.
> 
> It is convention in device tree to include older device IDs if the
> device is compatible.
> 
> So you might do
> 
>  compatible = "nuvoton,npct650", "nuvoton,npct601"
> 
> Andrew, is 601 even the right name?
> 
> > Was it expected to work with some wild-card type device-id as specified 
> > in the first line of description comment of file i.e. npct6XX. ?
> 
> No.
> 
> > So, why is there hard-coded checking and not using tpm2_probe() method 
> > which is itself based on direct TPM hardware response for setting the 
> > TPM2 flag. ? Is there something I am missing in the design which 
> > mandates to have .data set as OF_IS_TPM2.
> 
> Generally speaking probing is somewhat discouraged, currently we only
> probe for PC platform tis (and even that might be a mistake), all
> other drivers are designed to be explicit.

I did probing that way originally because the dTPM I had in hand in
2014 did not give expected information from TIS registers in the force
path. I'm sorry but can't recall anymore more exact description what was
the problem but the dTPM was not a production version.

> Jason

/Jarkko

------------------------------------------------------------------------------