Re: [PATCH v2 1/2] tpm: devicetree: document properties for cr50

[Andrey Pronin <apronin-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>]

On Thu, Jul 21, 2016 at 04:03:12PM -0500, Rob Herring wrote:
> On Wed, Jul 20, 2016 at 12:49:12PM -0700, Andrey Pronin wrote:
> > On Wed, Jul 20, 2016 at 02:03:03PM -0500, Rob Herring wrote:
> > > On Tue, Jul 19, 2016 at 08:41:24PM -0700, Andrey Pronin wrote:
> > 
> > Hi Rob,
> > 
> > > As I mentioned, there may be common properties. It doesn't seem you 
> > > looked, so I did:
> > > 
> > > - spi-rx-delay-us  - (optional) Microsecond delay after a read transfer.
> > > - spi-tx-delay-us  - (optional) Microsecond delay after a write transfer.
> > > 
> > > Seems to me setting one or both of these should work for you.
> > >
> > 
> > Yes, good catch, my fault I didn't see those.
> > But they are not exactly what I mean and need. I don't need delay after
> > each read or write transfer. What is needed is a guaranteed time
> > between transfers.
> > 
> > So, if the next transaction doesn't come withing the next X ms (or us),
> > we don't waste time on inserting a delays after this transaction at all.
> > Following the description and always inserting a delay must work well
> > for short microseconds-long delays. For longer milliseconds-long delays
> > a different strategy of checking the time when the previous transaction
> > was and only delaying if it was not too long ago is better.
> 
> I'd guess that the intent is the same for all. A simple delay is 
> just much easier to implement. I would think implementing the more 
> sophisticated algorithm would work for all users. Perhaps with some 
> threshold for a simple delay.
> 
> > Thus, I won't be able to re-use these properties anyways based on their
> > current description in bindings/spi/spi-bus.txt.
> > 
> > > > +- sleep-delay-ms: Time after the last SPI activity, after which the chip
> > > > +  may go to sleep.
> > > > +- wake-start-delay-ms: Time after initiating wake up before the chip is
> > > > +  ready to accept commands over SPI.
> > > 
> > > I also asked why these 2 can't be hard-coded in the driver?
> > >
> > 
> > Sorry, I just updated this patch description in v2 to indicate why they are not
> > hard-coded, but didn't answer explicitly. As the firmware changes, a different
> > revision of it can have a different time before it sleeps in its configuration,
> > or the time it takes it to startup may be different. Thus, there's a way to
> > set it here w/o changing the driver.
> 
> The firmware and DT may not be updated in sync especially if you are 
> loading the firmware from the rootfs. Are you doing DT and firmware 
> updates without changing the kernel?
> 
> Rob

Hi Rob,

Thanks for the feedback. I will hard-code those parameters in the
driver instead of reading from DT.

Thanks,
Andrey

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