Jason,

For your information, the lpcpd pin is named like this as per 
PCClientTPMSpecification is also known as TPMSTB on this product.
The TPMSTB pin can be used in order to support D1 and D2 power management state.
I am actually sorry to refer to Microsoft website here ;) : 
http://msdn.microsoft.com/en-us/library/windows/hardware/ff543162%28v=vs.85%29.aspx.
As mention by our product datasheet:
"In order to reach D1/D2 state, the TPMSTB pin must be connected to a GPIO 
signal.
In state D1/D2, the contents of the volatile memory is *preserved*."

Also "... the implementation of the LPCPD# pin on a TPM is platform and chipset 
implementation specific." Cf PCClientTPMSpecification.

I am currently giving the opportunity to an integrator to use this feature. 

I don't mind removing the power_mgt module option but don't you think there 
might be interest for an integrator to update their TPM power management policy 
on their platform ?

Best Regards
Christophe
-----Original Message-----
From: Jason Gunthorpe [mailto:[email protected]] 
Sent: mercredi 8 octobre 2014 22:51
To: Christophe Henri RICARD
Cc: Christophe RICARD; [email protected]; [email protected]; 
[email protected]; [email protected]; 
[email protected]; Jean-Luc BLANC
Subject: Re: [tpmdd-devel] [PATCH 07/16] tpm/tpm_i2c_stm_st33: Add devicetree 
structure

On Wed, Oct 08, 2014 at 10:41:36PM +0200, Christophe Henri RICARD wrote:
> Hi Jason,
> 
> I have currently seen both. I agree on the principles as it is 
> simplifying the code a little bit.  I including this clean up in this 
> patch Add devicetree structure for a future v2 submission

I was just looking at what the remaining lpcpd GPIO does, and..

1) I think you can probably just make this optional, if it is not
   specified in the DT or through platform data, just don't fiddle with
   it. This makes the driver much friendlier - ie it can be manually
   attached via sysfs for instance.

2) This looks fishy:

static int tpm_st33_i2c_pm_suspend(struct device *dev)
        if (power_mgt) {
                gpio_set_value(pin_infos->io_lpcpd, 0);
        } else {
                ret = tpm_pm_suspend(dev);
        }

   I can't think of a reason why the driver wouldn't need to call
   tpm_pm_suspend to save the internal state? Ditto for restore?

   I'm also suspicious of the power_mgt module option, that should
   probably go away entirely?

Jason
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to