On Wed, 2023-07-12 at 18:05 +0300, Andy Shevchenko wrote: > On Tue, Jul 11, 2023 at 03:09:47PM -0700, Srinivas Pandruvada wrote: > > Some of the PM features can be locked or disabled. In that case, > > write > > interface can be locked. > > > > This status is read via a mailbox. There is one TPMI ID which > > provides > > base address for interface and data register for mail box > > operation. > > The mailbox operations is defined in the TPMI specification. Refer > > to > > https://github.com/intel/tpmi_power_management/ for TPMI > > specifications. > > > > An API is exposed to feature drivers to read feature control > > status. > > ... > > > +/* > > + * TPMI PFS and per feature memory size can't exceed 4K. > > + * Also PFS start and feature memory is 4K aligned. > > + */ > > +#define TPMI_MAX_BUFFER_SIZE (4 * 1024) > > SZ_4K from sizes.h? > > ... I added a macro for size and uses sizes.h define. > > > +#define TPMI_CONTROL_TIMEOUT_MAX_US USEC_PER_SEC > > > +#define TPMI_RB_TIMEOUT_MAX_US USEC_PER_SEC > > I think it's easier to get in a form (1 * ..._SEC) > OK > ... > > > +static int tpmi_wait_for_owner(struct intel_tpmi_info *tpmi_info, > > u8 owner) > > +{ > > + u64 control; > > + > > + return read_poll_timeout(readq, control, > > + owner == > > FIELD_GET(TPMI_CONTROL_STATUS_OWNER, control), > > + TPMI_CONTROL_TIMEOUT_US, > > TPMI_CONTROL_TIMEOUT_MAX_US, false, > > + tpmi_info->tpmi_control_mem + > > TPMI_CONTROL_STATUS_OFFSET); > > Since you have "false" why not use readq_poll_timeout()? > Changed in new version > > +} > > ... > > > + /* Wait for Run Busy clear */ > > + ret = read_poll_timeout(readq, control, !(control & > > TPMI_CONTROL_STATUS_RB), > > + TPMI_RB_TIMEOUT_US, > > TPMI_RB_TIMEOUT_MAX_US, false, > > + tpmi_info->tpmi_control_mem + > > TPMI_CONTROL_STATUS_OFFSET); > > Ditto. Done. > > > + if (ret) > > + goto done_proc; > > ... > > > + size = pfs->pfs_header.num_entries * pfs- > > >pfs_header.entry_size * sizeof(u32); > > + /* This size is coming from trusted hardware, but verify > > anyway */ > > I would move this comment before size assignment that we already know > that it's > from the trusted hw. Created a macro. Thanks, Srinivas > > > + if (size > TPMI_MAX_BUFFER_SIZE) > > + return; >
