[v6,05/14] crytpo: caam - make use of iowrite64*_hi_lo in wr_reg64
diff mbox series

Message ID 20190717152458.22337-6-andrew.smirnov@gmail.com
State Changes Requested
Delegated to: Herbert Xu
Headers show
Series
  • crypto: caam - Add i.MX8MQ support
Related show

Commit Message

Andrey Smirnov July 17, 2019, 3:24 p.m. UTC
In order to be able to unify 64 and 32 bit implementations of
wr_reg64, let's convert it to use helpers from
<linux/io-64-nonatomic-hi-lo.h> first. Here are the steps of the
transformation:

1. Inline wr_reg32 helpers:

	if (!caam_imx && caam_little_end) {
		if (caam_little_end) {
			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32(data, (u32 __iomem *)(reg));
		} else {
			iowrite32be(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32be(data, (u32 __iomem *)(reg));
		}
	} else {
		if (caam_little_end) {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		} else {
			iowrite32be(data >> 32, (u32 __iomem *)(reg));
			iowrite32be(data, (u32 __iomem *)(reg) + 1);
		}
	}

2. Transfrom the conditionals such that the check for
'caam_little_end' is at the top level:

	if (caam_little_end) {
		if (!caam_imx) {
			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32(data, (u32 __iomem *)(reg));
		} else {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		}
	} else {
		iowrite32be(data >> 32, (u32 __iomem *)(reg));
		iowrite32be(data, (u32 __iomem *)(reg) + 1);
	}

3. Invert the check for !caam_imx:

	if (caam_little_end) {
		if (caam_imx) {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		} else {
			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
			iowrite32(data, (u32 __iomem *)(reg));
		}
	} else {
		iowrite32be(data >> 32, (u32 __iomem *)(reg));
		iowrite32be(data, (u32 __iomem *)(reg) + 1);
	}

4. Make use of iowrite64* helpers from <linux/io-64-nonatomic-hi-lo.h>

	if (caam_little_end) {
		if (caam_imx) {
			iowrite32(data >> 32, (u32 __iomem *)(reg));
			iowrite32(data, (u32 __iomem *)(reg) + 1);
		} else {
			iowrite64(data, reg);
		}
	} else {
		iowrite64be(data, reg);
	}

No functional change intended.

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Chris Spencer <christopher.spencer@sea.co.uk>
Cc: Cory Tusar <cory.tusar@zii.aero>
Cc: Chris Healy <cphealy@gmail.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: Horia Geantă <horia.geanta@nxp.com>
Cc: Aymen Sghaier <aymen.sghaier@nxp.com>
Cc: Leonard Crestez <leonard.crestez@nxp.com>
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/crypto/caam/regs.h | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Horia Geanta July 29, 2019, 3:29 p.m. UTC | #1
On 7/17/2019 6:25 PM, Andrey Smirnov wrote:
> In order to be able to unify 64 and 32 bit implementations of
> wr_reg64, let's convert it to use helpers from
> <linux/io-64-nonatomic-hi-lo.h> first. Here are the steps of the
> transformation:
> 
> 1. Inline wr_reg32 helpers:
> 
> 	if (!caam_imx && caam_little_end) {
> 		if (caam_little_end) {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
> 			iowrite32(data, (u32 __iomem *)(reg));
> 		} else {
> 			iowrite32be(data >> 32, (u32 __iomem *)(reg) + 1);
> 			iowrite32be(data, (u32 __iomem *)(reg));
> 		}
> 	} else {
> 		if (caam_little_end) {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg));
> 			iowrite32(data, (u32 __iomem *)(reg) + 1);
> 		} else {
> 			iowrite32be(data >> 32, (u32 __iomem *)(reg));
> 			iowrite32be(data, (u32 __iomem *)(reg) + 1);
> 		}
> 	}
> 
> 2. Transfrom the conditionals such that the check for
> 'caam_little_end' is at the top level:
> 
> 	if (caam_little_end) {
> 		if (!caam_imx) {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
> 			iowrite32(data, (u32 __iomem *)(reg));
> 		} else {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg));
> 			iowrite32(data, (u32 __iomem *)(reg) + 1);
> 		}
> 	} else {
> 		iowrite32be(data >> 32, (u32 __iomem *)(reg));
> 		iowrite32be(data, (u32 __iomem *)(reg) + 1);
> 	}
> 
> 3. Invert the check for !caam_imx:
> 
> 	if (caam_little_end) {
> 		if (caam_imx) {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg));
> 			iowrite32(data, (u32 __iomem *)(reg) + 1);
> 		} else {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg) + 1);
> 			iowrite32(data, (u32 __iomem *)(reg));
> 		}
> 	} else {
> 		iowrite32be(data >> 32, (u32 __iomem *)(reg));
> 		iowrite32be(data, (u32 __iomem *)(reg) + 1);
> 	}
> 
> 4. Make use of iowrite64* helpers from <linux/io-64-nonatomic-hi-lo.h>
> 
> 	if (caam_little_end) {
> 		if (caam_imx) {
> 			iowrite32(data >> 32, (u32 __iomem *)(reg));
> 			iowrite32(data, (u32 __iomem *)(reg) + 1);
> 		} else {
> 			iowrite64(data, reg);
> 		}
> 	} else {
> 		iowrite64be(data, reg);
> 	}
> 
> No functional change intended.
> 
> Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Reviewed-by: Horia Geantă <horia.geanta@nxp.com>

Just to clarify one thing.

For a previous patch I mentioned:
> To be consistent with CAAM engine HW spec: in case of 64-bit registers,
> irrespective of device endianness, the lower address should be read from
> / written to first, followed by the upper address.
https://lore.kernel.org/linux-crypto/VI1PR0401MB259145C2DFDB5E4084EA5DFC98D20@VI1PR0401MB2591.eurprd04.prod.outlook.com/

I've checked again and actually there is no limitation wrt. the order in which
the two 32-bit parts of 64-bit registers are read from / written to,
except for performance counters (only available on DN parts, not on i.MX).
However, performance counters do not user {rd,wr}_reg64 and should be fixed
separately.

In conclusion, it's ok to use either hi_lo or lo_hi semantics (which is _data_
semantics btw).
It makes more sense for this patch to include io-64-nonatomic-hi-lo.h since
that's what regs.h currently uses.

Horia

Patch
diff mbox series

diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 8591914d5c51..6e8352ac0d92 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -12,6 +12,7 @@ 
 #include <linux/types.h>
 #include <linux/bitops.h>
 #include <linux/io.h>
+#include <linux/io-64-nonatomic-hi-lo.h>
 
 /*
  * Architecture-specific register access methods
@@ -157,12 +158,15 @@  static inline u64 rd_reg64(void __iomem *reg)
 #else /* CONFIG_64BIT */
 static inline void wr_reg64(void __iomem *reg, u64 data)
 {
-	if (!caam_imx && caam_little_end) {
-		wr_reg32((u32 __iomem *)(reg) + 1, data >> 32);
-		wr_reg32((u32 __iomem *)(reg), data);
+	if (caam_little_end) {
+		if (caam_imx) {
+			iowrite32(data >> 32, (u32 __iomem *)(reg));
+			iowrite32(data, (u32 __iomem *)(reg) + 1);
+		} else {
+			iowrite64(data, reg);
+		}
 	} else {
-		wr_reg32((u32 __iomem *)(reg), data >> 32);
-		wr_reg32((u32 __iomem *)(reg) + 1, data);
+		iowrite64be(data, reg);
 	}
 }