On 4/11/2018 4:54 AM, Fabio Estevam wrote: > From: Fabio Estevam <fabio.estevam@xxxxxxx> > > The 'era' information can be retrieved from CAAM registers, so > introduce a caam_get_era_from_hw() function that gets it via register > reads in case the 'fsl,sec-era' property is not passed in the device > tree. > Indeed, "fsl,sec-era" property is marked as optional in DT bindings doc. This means the previous commit 883619a931e9 ("crypto: caam - fix ERA retrieval function") should have kept the detection based on registers as a fallback. Have you actually hit a case where the property was missing from DT? > This function is based on the U-Boot implementation from > drivers/crypto/fsl/sec.c > > Signed-off-by: Fabio Estevam <fabio.estevam@xxxxxxx> > --- > Changes since v1: > - None. I previously asked to put the linux-crypto list on Cc > > drivers/crypto/caam/ctrl.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- > drivers/crypto/caam/regs.h | 6 ++++++ > 2 files changed, 48 insertions(+), 3 deletions(-) > > diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > index bee690a..3f10791 100644 > --- a/drivers/crypto/caam/ctrl.c > +++ b/drivers/crypto/caam/ctrl.c > @@ -396,11 +396,47 @@ static void kick_trng(struct platform_device *pdev, int ent_delay) > clrsetbits_32(&r4tst->rtmctl, RTMCTL_PRGM, RTMCTL_SAMP_MODE_RAW_ES_SC); > } > > +static u8 caam_get_era_from_hw(struct caam_ctrl __iomem *ctrl) > +{ > + const struct sec_vid id[] = { Make the struct static. sec_vid should not be used since: -it defines the layout of SECVID_MS register (ip_id, maj_rev, min_rev) -while the array below contains (ip_id, maj_rev, era) tuples You could instead use an anonymous struct, just like in kernel commit 82c2f9607b8a4 or as in u-boot. > + {0x0A10, 1, 1}, > + {0x0A10, 2, 2}, > + {0x0A12, 1, 3}, > + {0x0A14, 1, 3}, > + {0x0A14, 2, 4}, > + {0x0A16, 1, 4}, > + {0x0A10, 3, 4}, > + {0x0A11, 1, 4}, > + {0x0A18, 1, 4}, > + {0x0A11, 2, 5}, > + {0x0A12, 2, 5}, > + {0x0A13, 1, 5}, > + {0x0A1C, 1, 5}, > + }; > + int i; > + > + u32 secvid_ms = rd_reg32(&ctrl->perfmon.caam_id_ms); Reading caam_id_ms should be done only if ccbvid does not provide the era, i.e. should be moved just before the for loop. > + u32 ccbvid = rd_reg32(&ctrl->perfmon.ccb_id); > + u16 ip_id = (secvid_ms & SECVID_MS_IPID_MASK) >> SECVID_MS_IPID_SHIFT; > + u8 maj_rev = (secvid_ms & SECVID_MS_MAJ_REV_MASK) >> > + SECVID_MS_MAJ_REV_SHIFT; > + u8 era = (ccbvid & CCBVID_ERA_MASK) >> CCBVID_ERA_SHIFT; > + > + if (era) /* This is '0' prior to CAAM ERA-6 */ > + return era; > + > + for (i = 0; i < ARRAY_SIZE(id); i++) > + if (id[i].ip_id == ip_id && id[i].maj_rev == maj_rev) > + return id[i].min_rev; > + > + return 0; Should return -ENOTSUPP, to keep the semantics of caam_get_era(). > +} > + > /** > * caam_get_era() - Return the ERA of the SEC on SoC, based > * on "sec-era" propery in the DTS. This property is updated by u-boot. While here: _optional_ ^^^ s/propery/property Should also mention that ERA detection fallback relies on SEC registers (CCBVID or SECVID). > **/ > -static int caam_get_era(void) > +static int caam_get_era(struct caam_ctrl __iomem *ctrl) > { > struct device_node *caam_node; > int ret; > @@ -410,7 +446,10 @@ static int caam_get_era(void) > ret = of_property_read_u32(caam_node, "fsl,sec-era", &prop); > of_node_put(caam_node); > > - return ret ? -ENOTSUPP : prop; > + if (!ret) > + return prop; > + else > + return caam_get_era_from_hw(ctrl); > } > > static const struct of_device_id caam_match[] = { > @@ -622,7 +661,7 @@ static int caam_probe(struct platform_device *pdev) > goto iounmap_ctrl; > } > > - ctrlpriv->era = caam_get_era(); > + ctrlpriv->era = caam_get_era(ctrl); > > ret = of_platform_populate(nprop, caam_match, NULL, dev); > if (ret) { > diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h > index fee3638..6f96d7b 100644 > --- a/drivers/crypto/caam/regs.h > +++ b/drivers/crypto/caam/regs.h > @@ -311,6 +311,12 @@ struct caam_perfmon { > u64 rsvd3; > > /* Component Instantiation Parameters fe0-fff */ > +#define SECVID_MS_IPID_MASK 0xffff0000 > +#define SECVID_MS_IPID_SHIFT 16 > +#define SECVID_MS_MAJ_REV_MASK 0x0000ff00 > +#define SECVID_MS_MAJ_REV_SHIFT 8 > +#define CCBVID_ERA_MASK 0xff000000 > +#define CCBVID_ERA_SHIFT 24 Please add the defines in front of each register: -SECVID_* before caam_id_ms -CCBVID_* before ccb_id > u32 rtic_id; /* RVID - RTIC Version ID */ > u32 ccb_id; /* CCBVID - CCB Version ID */ > u32 cha_id_ms; /* CHAVID - CHA Version ID Most Significant*/ > Thanks, Horia