> > On Mon, Dec 12, 2016 at 02:25:32PM +0000, Winkler, Tomas wrote: > > > > > > > > In order to provide access to locality registers, this commits > > > > adds mapping of the head of the CRB registers, which are located right > before the control area. > > > > > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> > > > > --- > > > > drivers/char/tpm/tpm_crb.c | 89 > > > > +++++++++++++++++++++++++++++------------ > > > > ----- > > > > 1 file changed, 57 insertions(+), 32 deletions(-) > > > > > > > > diff --git a/drivers/char/tpm/tpm_crb.c > > > > b/drivers/char/tpm/tpm_crb.c index > > > > 717b6b4..80b9759 100644 > > > > --- a/drivers/char/tpm/tpm_crb.c > > > > +++ b/drivers/char/tpm/tpm_crb.c > > > > @@ -52,18 +52,28 @@ enum crb_cancel { > > > > CRB_CANCEL_INVOKE = BIT(0), > > > > }; > > > > > > > > -struct crb_control_area { > > > > - u32 req; > > > > - u32 sts; > > > > - u32 cancel; > > > > - u32 start; > > > > - u32 int_enable; > > > > - u32 int_sts; > > > > - u32 cmd_size; > > > > - u32 cmd_pa_low; > > > > - u32 cmd_pa_high; > > > > - u32 rsp_size; > > > > - u64 rsp_pa; > > > > +struct crb_regs_head { > > > > + u32 loc_state; > > > > + u32 reserved1; > > > > + u32 loc_ctrl; > > > > + u32 loc_sts; > > > > + u8 reserved2[32]; > > > > + u64 intf_id; > > > > + u64 ctrl_ext; > > > > +} __packed; > > > > + > > > > > > > +struct crb_regs_tail { > > > Why to change the name this is still control_area > > And how would you name struct crb_regs_h then?
Just crb_regs > In my opinion PC it makes a lot of sense to speak about registers here rather > than control area now that it is extended to the full range. The PC Client > Specification also speaks about registers. Right so crb_regs is to be and the nonstandard implementation of the legacy platforms should be even factored out. > > > > > + u32 ctrl_req; > > > > + u32 ctrl_sts; > > > > + u32 ctrl_cancel; > > > > + u32 ctrl_start; > > > > + u32 ctrl_int_enable; > > > > + u32 ctrl_int_sts; > > > > + u32 ctrl_cmd_size; > > > > + u32 ctrl_cmd_pa_low; > > > > + u32 ctrl_cmd_pa_high; > > > > + u32 ctrl_rsp_size; > > > > + u64 ctrl_rsp_pa; > > > > } __packed; > > > > > > > > enum crb_status { > > > > @@ -78,7 +88,8 @@ enum crb_flags { struct crb_priv { > > > > unsigned int flags; > > > > void __iomem *iobase; > > > > - struct crb_control_area __iomem *cca; > > > > + struct crb_regs_head __iomem *regs_h; > > > > + struct crb_regs_tail __iomem *regs_t; > > > > > > Same here, why to change the name, let's keep it cca it will reduce > > > the size of patch and make it more back portable. > > Backportability is a always a secondary priority for new features albeit > something that can be considered if it doesn't get in the way. I think of it as a staged steps. First do the minimal fix then do the overhaul. Anyway you are kind of counterproductive to yourself as you already know this need a back port. Thanks Tomas