Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Fabio Estevam]

Hi Herbert,

On Tue, Apr 24, 2018 at 1:39 PM, Herbert Xu  wrote:

> As this is a new device support issue I'd prefer to delay this
> until the next merge window.

Understood.

I will send a patch that passes ''fsl,sec-era' property in the dts, so
that we can have CAAM working on 4.17 fon i.MX7.

Thanks

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Herbert Xu]

Hi Fabio:

On Fri, Apr 20, 2018 at 03:21:47PM -0300, Fabio Estevam wrote:
> 
> It is not a regression.
> 
> We haven't seen this problem before because dtsi files passed the
> 'fsl,sec-era' property.
> 
> Since 4.17-rc1, imx7 supports CAAM:
> 0eeabcad7da5  ("ARM: dts: imx7s: add CAAM device node")
> 
> ,but it does not pass the 'fsl,sec-era' property leading to the following 
> error:
> caam 3090.caam: device ID = 0x0a160300 (Era -524)
> 
> Documentation/devicetree/bindings/crypto/fsl-sec4.txt states that
> 'fsl,sec-era' property is optional, so 0eeabcad7da5 is not incorrect
> by not passing it.
> 
> As we can retrieve the era information by reading the CAAM registers
> we can fix the problem on imx7 running 4.17-rc with this patch.

As this is a new device support issue I'd prefer to delay this
until the next merge window.

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Fabio Estevam]

Hi Herbert,

On Fri, Apr 20, 2018 at 3:01 PM, Herbert Xu  wrote:

> Is this a regression or a preexisting bug?

It is not a regression.

We haven't seen this problem before because dtsi files passed the
'fsl,sec-era' property.

Since 4.17-rc1, imx7 supports CAAM:
0eeabcad7da5  ("ARM: dts: imx7s: add CAAM device node")

,but it does not pass the 'fsl,sec-era' property leading to the following error:
caam 3090.caam: device ID = 0x0a160300 (Era -524)

Documentation/devicetree/bindings/crypto/fsl-sec4.txt states that
'fsl,sec-era' property is optional, so 0eeabcad7da5 is not incorrect
by not passing it.

As we can retrieve the era information by reading the CAAM registers
we can fix the problem on imx7 running 4.17-rc with this patch.

Thanks

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Herbert Xu]

On Fri, Apr 20, 2018 at 02:11:07PM -0300, Fabio Estevam wrote:
>
> Would it be possible to consider applying this one for 4.17-rc?
> 
> Without this patch we get incorrect CAAM IP block version (era) on
> i.MX7 on 4.17-rc1:
> 
> caam 3090.caam: device ID = 0x0a160300 (Era -524)

Is this a regression or a preexisting bug?

Cheers,
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Fabio Estevam]

Hi Herbert,

On Fri, Apr 20, 2018 at 1:52 PM, Herbert Xu  wrote:
> On Wed, Apr 11, 2018 at 09:45:20AM -0300, Fabio Estevam wrote:
>> From: Fabio Estevam 
>>
>> 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.
>>
>> This function is based on the U-Boot implementation from
>> drivers/crypto/fsl/sec.c
>>
>> Signed-off-by: Fabio Estevam 
>
> Patch applied.  Thanks.

Would it be possible to consider applying this one for 4.17-rc?

Without this patch we get incorrect CAAM IP block version (era) on
i.MX7 on 4.17-rc1:

caam 3090.caam: device ID = 0x0a160300 (Era -524)

Thanks

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Herbert Xu]

On Wed, Apr 11, 2018 at 09:45:20AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> 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.
> 
> This function is based on the U-Boot implementation from
> drivers/crypto/fsl/sec.c
> 
> Signed-off-by: Fabio Estevam 

Patch applied.  Thanks.
-- 
Email: Herbert Xu 
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Breno Matheus Lima]

Hi Fabio,

2018-04-11 9:45 GMT-03:00 Fabio Estevam :
> From: Fabio Estevam 
>
> 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.
>
> This function is based on the U-Boot implementation from
> drivers/crypto/fsl/sec.c
>
> Signed-off-by: Fabio Estevam 

I have just tested on a mx7dsabresd board and confirmed that the ERA
is correctly retrieved:

Without your patch:

[2.182756] caam 3090.caam: device ID = 0x0a160300 (Era -524)

With your patch applied:

[2.183526] caam 3090.caam: device ID = 0x0a160300 (Era 8)

Tested-by: Breno Lima 

Thanks,
Breno Lima

Re: [PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Horia Geantă]

On 4/11/2018 3:45 PM, Fabio Estevam wrote:
> From: Fabio Estevam 
> 
> 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.
> 
> This function is based on the U-Boot implementation from
> drivers/crypto/fsl/sec.c
> 
> Signed-off-by: Fabio Estevam 
Reviewed-by: Horia Geantă 
for the series.

Thanks,
Horia

[PATCH v4 2/2] crypto: caam - allow retrieving 'era' from register

[Fabio Estevam]

From: Fabio Estevam 

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.

This function is based on the U-Boot implementation from
drivers/crypto/fsl/sec.c

Signed-off-by: Fabio Estevam 
---
Changes since v3:
- Make checkpatch happy

 drivers/crypto/caam/ctrl.c | 56 ++
 drivers/crypto/caam/regs.h |  6 +
 2 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index bee690a..ab67e97 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -396,11 +396,56 @@ static void kick_trng(struct platform_device *pdev, int 
ent_delay)
clrsetbits_32(>rtmctl, RTMCTL_PRGM, RTMCTL_SAMP_MODE_RAW_ES_SC);
 }
 
+static int caam_get_era_from_hw(struct caam_ctrl __iomem *ctrl)
+{
+   static const struct {
+   u16 ip_id;
+   u8 maj_rev;
+   u8 era;
+   } id[] = {
+   {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}
+   };
+   u32 ccbvid, id_ms;
+   u8 maj_rev, era;
+   u16 ip_id;
+   int i;
+
+   ccbvid = rd_reg32(>perfmon.ccb_id);
+   era = (ccbvid & CCBVID_ERA_MASK) >> CCBVID_ERA_SHIFT;
+   if (era)/* This is '0' prior to CAAM ERA-6 */
+   return era;
+
+   id_ms = rd_reg32(>perfmon.caam_id_ms);
+   ip_id = (id_ms & SECVID_MS_IPID_MASK) >> SECVID_MS_IPID_SHIFT;
+   maj_rev = (id_ms & SECVID_MS_MAJ_REV_MASK) >> SECVID_MS_MAJ_REV_SHIFT;
+
+   for (i = 0; i < ARRAY_SIZE(id); i++)
+   if (id[i].ip_id == ip_id && id[i].maj_rev == maj_rev)
+   return id[i].era;
+
+   return -ENOTSUPP;
+}
+
 /**
  * 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.
+ * on "sec-era" optional property in the DTS. This property is updated
+ * by u-boot.
+ * In case this property is not passed an attempt to retrieve the CAAM
+ * era via register reads will be made.
  **/
-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 +455,10 @@ static int caam_get_era(void)
ret = of_property_read_u32(caam_node, "fsl,sec-era", );
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 +670,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..4fb91ba 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -312,11 +312,17 @@ struct caam_perfmon {
 
/* Component Instantiation Parameters   fe0-fff */
u32 rtic_id;/* RVID - RTIC Version ID   */
+#define CCBVID_ERA_MASK0xff00
+#define CCBVID_ERA_SHIFT   24
u32 ccb_id; /* CCBVID - CCB Version ID  */
u32 cha_id_ms;  /* CHAVID - CHA Version ID Most Significant*/
u32 cha_id_ls;  /* CHAVID - CHA Version ID Least Significant*/
u32 cha_num_ms; /* CHANUM - CHA Number Most Significant */
u32 cha_num_ls; /* CHANUM - CHA Number Least Significant*/
+#define SECVID_MS_IPID_MASK0x
+#define SECVID_MS_IPID_SHIFT   16
+#define SECVID_MS_MAJ_REV_MASK 0xff00
+#define SECVID_MS_MAJ_REV_SHIFT8
u32 caam_id_ms; /* CAAMVID - CAAM Version ID MS */
u32 caam_id_ls; /* CAAMVID - CAAM Version ID LS */
 };
-- 
2.7.4