On Mon, Nov 20, 2017 at 07:34:07PM +0000, Shaikh, Azhar wrote: > >However, why not have check_locality, release_locality, wait_startup use > >clk_toggle instead? That seems better to me?? > > > > I think, better to have the clk disable/enable at one place instead > of adding it all locations where TPM access is done. The clk_toggle > is kind of a special case where, for the complete duration of the > TPM command processing, the CLKRUN protocol was supposed to be > disabled. For rest of the places we can disable and re-enable the > clkrun as soon as possible. > Also since it is part of the read/write APIs we won't miss any other > location in the code where TPM access is done. But I wonder if you will hit the same bug that motivated this change in the other places? It seems clearer to me to have a strong guard of the clock across the entire high level action, since this hardware is so broken. You could add a debug to ensure that read/write is never without clk_enable being held. Jason