>-----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? >Jason Regards, Azhar Shaikh
