On Mon, Nov 20, 2017 at 06:52:13PM +0000, Shaikh, Azhar wrote: > > > >-----Original Message----- > >From: Jason Gunthorpe [mailto:jgg@xxxxxxxx] > >Sent: Monday, November 20, 2017 10:35 AM > >To: Shaikh, Azhar <azhar.shaikh@xxxxxxxxx> > >Cc: jarkko.sakkinen@xxxxxxxxxxxxxxx; peterhuewe@xxxxxx; linux- > >integrity@xxxxxxxxxxxxxxx > >Subject: Re: [PATCH RFC v3 1/2] tpm: Keep CLKRUN enabled throughout the > >duration of transmit_cmd() > > > >On Wed, Nov 15, 2017 at 02:07:11PM -0800, Azhar Shaikh wrote: > > > >> -static void tpm_platform_begin_xfer(void) > >> +static void tpm_platform_begin_xfer(struct tpm_tis_data *data) > >> { > >> u32 clkrun_val; > >> + struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data); > >> > >> - if (!is_bsw()) > >> + if (!is_bsw() || ((data->flags & TPM_TIS_CLK_ENABLE) && > >> + phy->begin_xfer_done)) > >> return; > > > >I think everything looks OK now, but I was reading over the series again, and I > >admit I don't quite get it.. > > > >Why do we continue to have tpm_platform_begin_xfer after you added > >clk_toggle? > > > > We want to have the CLKRUN disabled for any/all TPM transactions. > The clk_toggle handles only the case while a TPM command is being sent and received. > We have to take into consideration other places too where TPM access is happening outside the TPM command flow. For eg: request_locality, check_locality, release_locality, wait_startup which might be called outside the flow of a TPM command. > > >Why not just directly enable CLK_RUN in clk_toggle and get rid of > >tpm_platform_begin_xfer/etc ?? > > > >Is there some reason we still need to to per transfer stuff??? > > > >clk_enable or device_enable would also be a better name than clock_toggle. > > > > Will change it to clk_enable. > Should I then upload the next patch for review and remove the "RFC" tag now? And if so, should I retain the change history of the patch versions? Please do and keep my reviewed-by. /Jarkko
