[PATCH 3.12 42/56] crypto: ux500 - make interrupt mode plausible
From: Arnd Bergmann a...@arndb.de
3.12-stable review patch. If anyone has any objections, please let me know.
===
commit e1f8859ee265fc89bd21b4dca79e8e983a044892 upstream.
The interrupt handler in the ux500 crypto driver has an obviously
incorrect way to access the data buffer, which for a while has
caused this build warning:
../ux500/cryp/cryp_core.c: In function 'cryp_interrupt_handler':
../ux500/cryp/cryp_core.c:234:5: warning: passing argument 1 of '__fswab32'
makes integer from pointer without a cast [enabled by default]
writel_relaxed(ctx-indata,
^
In file included from ../include/linux/swab.h:4:0,
from ../include/uapi/linux/byteorder/big_endian.h:12,
from ../include/linux/byteorder/big_endian.h:4,
from ../arch/arm/include/uapi/asm/byteorder.h:19,
from ../include/asm-generic/bitops/le.h:5,
from ../arch/arm/include/asm/bitops.h:340,
from ../include/linux/bitops.h:33,
from ../include/linux/kernel.h:10,
from ../include/linux/clk.h:16,
from ../drivers/crypto/ux500/cryp/cryp_core.c:12:
../include/uapi/linux/swab.h:57:119: note: expected '__u32' but argument is of
type 'const u8 *'
static inline __attribute_const__ __u32 __fswab32(__u32 val)
There are at least two, possibly three problems here:
a) when writing into the FIFO, we copy the pointer rather than the
actual data we want to give to the hardware
b) the data pointer is an array of 8-bit values, while the FIFO
is 32-bit wide, so both the read and write access fail to do
a proper type conversion
c) This seems incorrect for big-endian kernels, on which we need to
byte-swap any register access, but not normally FIFO accesses,
at least the DMA case doesn't do it either.
This converts the bogus loop to use the same readsl/writesl pair
that we use for the two other modes (DMA and polling). This is
more efficient and consistent, and probably correct for endianess.
The bug has existed since the driver was first merged, and was
probably never detected because nobody tried to use interrupt mode.
It might make sense to backport this fix to stable kernels, depending
on how the crypto maintainers feel about that.
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: linux-crypto@vger.kernel.org
Cc: Fabio Baltieri fabio.balti...@linaro.org
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
Signed-off-by: Jiri Slaby jsl...@suse.cz
---
drivers/crypto/ux500/cryp/cryp_core.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/crypto/ux500/cryp/cryp_core.c
b/drivers/crypto/ux500/cryp/cryp_core.c
index a999f537228f..92105f3dc8e0 100644
--- a/drivers/crypto/ux500/cryp/cryp_core.c
+++ b/drivers/crypto/ux500/cryp/cryp_core.c
@@ -190,7 +190,7 @@ static void add_session_id(struct cryp_ctx *ctx)
static irqreturn_t cryp_interrupt_handler(int irq, void *param)
{
struct cryp_ctx *ctx;
- int i;
+ int count;
struct cryp_device_data *device_data;
if (param == NULL) {
@@ -215,12 +215,11 @@ static irqreturn_t cryp_interrupt_handler(int irq, void
*param)
if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_OUTPUT_FIFO)) {
if (ctx-outlen / ctx-blocksize 0) {
- for (i = 0; i ctx-blocksize / 4; i++) {
- *(ctx-outdata) = readl_relaxed(
- device_data-base-dout);
- ctx-outdata += 4;
- ctx-outlen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ readsl(device_data-base-dout, ctx-outdata, count);
+ ctx-outdata += count;
+ ctx-outlen -= count;
if (ctx-outlen == 0) {
cryp_disable_irq_src(device_data,
@@ -230,12 +229,12 @@ static irqreturn_t cryp_interrupt_handler(int irq, void
*param)
} else if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_INPUT_FIFO)) {
if (ctx-datalen / ctx-blocksize 0) {
- for (i = 0 ; i ctx-blocksize / 4; i++) {
- writel_relaxed(ctx-indata,
- device_data-base-din);
- ctx-indata += 4;
- ctx-datalen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ writesl(device_data-base-din, ctx-indata, count);
+
+ ctx-indata += count;
+ ctx-datalen -= count;
[patch added to the 3.12 stable tree] crypto: ux500 - make interrupt mode plausible
From: Arnd Bergmann a...@arndb.de
This patch has been added to the 3.12 stable tree. If you have any
objections, please let us know.
===
commit e1f8859ee265fc89bd21b4dca79e8e983a044892 upstream.
The interrupt handler in the ux500 crypto driver has an obviously
incorrect way to access the data buffer, which for a while has
caused this build warning:
../ux500/cryp/cryp_core.c: In function 'cryp_interrupt_handler':
../ux500/cryp/cryp_core.c:234:5: warning: passing argument 1 of '__fswab32'
makes integer from pointer without a cast [enabled by default]
writel_relaxed(ctx-indata,
^
In file included from ../include/linux/swab.h:4:0,
from ../include/uapi/linux/byteorder/big_endian.h:12,
from ../include/linux/byteorder/big_endian.h:4,
from ../arch/arm/include/uapi/asm/byteorder.h:19,
from ../include/asm-generic/bitops/le.h:5,
from ../arch/arm/include/asm/bitops.h:340,
from ../include/linux/bitops.h:33,
from ../include/linux/kernel.h:10,
from ../include/linux/clk.h:16,
from ../drivers/crypto/ux500/cryp/cryp_core.c:12:
../include/uapi/linux/swab.h:57:119: note: expected '__u32' but argument is of
type 'const u8 *'
static inline __attribute_const__ __u32 __fswab32(__u32 val)
There are at least two, possibly three problems here:
a) when writing into the FIFO, we copy the pointer rather than the
actual data we want to give to the hardware
b) the data pointer is an array of 8-bit values, while the FIFO
is 32-bit wide, so both the read and write access fail to do
a proper type conversion
c) This seems incorrect for big-endian kernels, on which we need to
byte-swap any register access, but not normally FIFO accesses,
at least the DMA case doesn't do it either.
This converts the bogus loop to use the same readsl/writesl pair
that we use for the two other modes (DMA and polling). This is
more efficient and consistent, and probably correct for endianess.
The bug has existed since the driver was first merged, and was
probably never detected because nobody tried to use interrupt mode.
It might make sense to backport this fix to stable kernels, depending
on how the crypto maintainers feel about that.
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: linux-crypto@vger.kernel.org
Cc: Fabio Baltieri fabio.balti...@linaro.org
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
Signed-off-by: Jiri Slaby jsl...@suse.cz
---
drivers/crypto/ux500/cryp/cryp_core.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/drivers/crypto/ux500/cryp/cryp_core.c
b/drivers/crypto/ux500/cryp/cryp_core.c
index a999f537228f..92105f3dc8e0 100644
--- a/drivers/crypto/ux500/cryp/cryp_core.c
+++ b/drivers/crypto/ux500/cryp/cryp_core.c
@@ -190,7 +190,7 @@ static void add_session_id(struct cryp_ctx *ctx)
static irqreturn_t cryp_interrupt_handler(int irq, void *param)
{
struct cryp_ctx *ctx;
- int i;
+ int count;
struct cryp_device_data *device_data;
if (param == NULL) {
@@ -215,12 +215,11 @@ static irqreturn_t cryp_interrupt_handler(int irq, void
*param)
if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_OUTPUT_FIFO)) {
if (ctx-outlen / ctx-blocksize 0) {
- for (i = 0; i ctx-blocksize / 4; i++) {
- *(ctx-outdata) = readl_relaxed(
- device_data-base-dout);
- ctx-outdata += 4;
- ctx-outlen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ readsl(device_data-base-dout, ctx-outdata, count);
+ ctx-outdata += count;
+ ctx-outlen -= count;
if (ctx-outlen == 0) {
cryp_disable_irq_src(device_data,
@@ -230,12 +229,12 @@ static irqreturn_t cryp_interrupt_handler(int irq, void
*param)
} else if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_INPUT_FIFO)) {
if (ctx-datalen / ctx-blocksize 0) {
- for (i = 0 ; i ctx-blocksize / 4; i++) {
- writel_relaxed(ctx-indata,
- device_data-base-din);
- ctx-indata += 4;
- ctx-datalen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ writesl(device_data-base-din, ctx-indata, count);
+
+ ctx-indata += count;
+ ctx-datalen -= count;
Re: [PATCH v2 00/12] crypto: caam - Add RTA descriptor creation library
On 8/16/2014 2:16 PM, Kim Phillips wrote: On Thu, 14 Aug 2014 15:54:22 +0300 Horia Geanta horia.gea...@freescale.com wrote: This patch set adds Run Time Assembler (RTA) SEC descriptor library. RTA is a replacement for incumbent inline append. The library is intended to be a single code base for SEC descriptors creation for all Freescale products. This comes with a series of advantages, such as library being maintained / kept up-to-date with latest platforms, i.e. SEC functionalities (for e.g. SEC incarnations present in Layerscape LS1 and LS2). RTA detects options in SEC descriptors that are not supported by a SEC HW revision (Era) and reports this back. Say a descriptor uses Sequence Out Pointer (SOP) option for the SEQINPTR command, which is supported starting from SEC Era 5. If the descriptor would be built on a P4080R3 platform (which has SEC Era 4), RTA would report SEQ IN PTR: Flag(s) not supported by SEC Era 4. This is extremely useful and saves a lot of time wasted on debugging. SEC HW detects only *some* of these problems, leaving user wonder what causes a DECO Watchdog Timeout. And when it prints something more useful, sometimes it does not point to the exact opcode. again, RTA just adds bloat to the kernel driver - the kernel driver is supposed to generate the appropriate descriptor for its target running SEC version no matter what, not report back what is/is not supported. This is a flaw at the RTA design level, as far as the kernel driver is concerned. What is your understanding of developing a descriptor? First it needs to be written, then tested - within the kernel driver. Having no error checking in the code that generates descriptors increases testing / debugging time significantly. Again, SEC HW provides some error reporting, but in many cases this is a clueless Watchdog Timeout. SEC descriptors development is complex enough to deserve a few indications along the way. Regards, Horia -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3.16 054/125] crypto: ux500 - make interrupt mode plausible
3.16-stable review patch. If anyone has any objections, please let me know.
--
From: Arnd Bergmann a...@arndb.de
commit e1f8859ee265fc89bd21b4dca79e8e983a044892 upstream.
The interrupt handler in the ux500 crypto driver has an obviously
incorrect way to access the data buffer, which for a while has
caused this build warning:
../ux500/cryp/cryp_core.c: In function 'cryp_interrupt_handler':
../ux500/cryp/cryp_core.c:234:5: warning: passing argument 1 of '__fswab32'
makes integer from pointer without a cast [enabled by default]
writel_relaxed(ctx-indata,
^
In file included from ../include/linux/swab.h:4:0,
from ../include/uapi/linux/byteorder/big_endian.h:12,
from ../include/linux/byteorder/big_endian.h:4,
from ../arch/arm/include/uapi/asm/byteorder.h:19,
from ../include/asm-generic/bitops/le.h:5,
from ../arch/arm/include/asm/bitops.h:340,
from ../include/linux/bitops.h:33,
from ../include/linux/kernel.h:10,
from ../include/linux/clk.h:16,
from ../drivers/crypto/ux500/cryp/cryp_core.c:12:
../include/uapi/linux/swab.h:57:119: note: expected '__u32' but argument is of
type 'const u8 *'
static inline __attribute_const__ __u32 __fswab32(__u32 val)
There are at least two, possibly three problems here:
a) when writing into the FIFO, we copy the pointer rather than the
actual data we want to give to the hardware
b) the data pointer is an array of 8-bit values, while the FIFO
is 32-bit wide, so both the read and write access fail to do
a proper type conversion
c) This seems incorrect for big-endian kernels, on which we need to
byte-swap any register access, but not normally FIFO accesses,
at least the DMA case doesn't do it either.
This converts the bogus loop to use the same readsl/writesl pair
that we use for the two other modes (DMA and polling). This is
more efficient and consistent, and probably correct for endianess.
The bug has existed since the driver was first merged, and was
probably never detected because nobody tried to use interrupt mode.
It might make sense to backport this fix to stable kernels, depending
on how the crypto maintainers feel about that.
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: linux-crypto@vger.kernel.org
Cc: Fabio Baltieri fabio.balti...@linaro.org
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/crypto/ux500/cryp/cryp_core.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
--- a/drivers/crypto/ux500/cryp/cryp_core.c
+++ b/drivers/crypto/ux500/cryp/cryp_core.c
@@ -190,7 +190,7 @@ static void add_session_id(struct cryp_c
static irqreturn_t cryp_interrupt_handler(int irq, void *param)
{
struct cryp_ctx *ctx;
- int i;
+ int count;
struct cryp_device_data *device_data;
if (param == NULL) {
@@ -215,12 +215,11 @@ static irqreturn_t cryp_interrupt_handle
if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_OUTPUT_FIFO)) {
if (ctx-outlen / ctx-blocksize 0) {
- for (i = 0; i ctx-blocksize / 4; i++) {
- *(ctx-outdata) = readl_relaxed(
- device_data-base-dout);
- ctx-outdata += 4;
- ctx-outlen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ readsl(device_data-base-dout, ctx-outdata, count);
+ ctx-outdata += count;
+ ctx-outlen -= count;
if (ctx-outlen == 0) {
cryp_disable_irq_src(device_data,
@@ -230,12 +229,12 @@ static irqreturn_t cryp_interrupt_handle
} else if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_INPUT_FIFO)) {
if (ctx-datalen / ctx-blocksize 0) {
- for (i = 0 ; i ctx-blocksize / 4; i++) {
- writel_relaxed(ctx-indata,
- device_data-base-din);
- ctx-indata += 4;
- ctx-datalen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ writesl(device_data-base-din, ctx-indata, count);
+
+ ctx-indata += count;
+ ctx-datalen -= count;
if (ctx-datalen == 0)
cryp_disable_irq_src(device_data,
--
To unsubscribe from this list: send the line unsubscribe
[PATCH 3.14 38/88] crypto: ux500 - make interrupt mode plausible
3.14-stable review patch. If anyone has any objections, please let me know.
--
From: Arnd Bergmann a...@arndb.de
commit e1f8859ee265fc89bd21b4dca79e8e983a044892 upstream.
The interrupt handler in the ux500 crypto driver has an obviously
incorrect way to access the data buffer, which for a while has
caused this build warning:
../ux500/cryp/cryp_core.c: In function 'cryp_interrupt_handler':
../ux500/cryp/cryp_core.c:234:5: warning: passing argument 1 of '__fswab32'
makes integer from pointer without a cast [enabled by default]
writel_relaxed(ctx-indata,
^
In file included from ../include/linux/swab.h:4:0,
from ../include/uapi/linux/byteorder/big_endian.h:12,
from ../include/linux/byteorder/big_endian.h:4,
from ../arch/arm/include/uapi/asm/byteorder.h:19,
from ../include/asm-generic/bitops/le.h:5,
from ../arch/arm/include/asm/bitops.h:340,
from ../include/linux/bitops.h:33,
from ../include/linux/kernel.h:10,
from ../include/linux/clk.h:16,
from ../drivers/crypto/ux500/cryp/cryp_core.c:12:
../include/uapi/linux/swab.h:57:119: note: expected '__u32' but argument is of
type 'const u8 *'
static inline __attribute_const__ __u32 __fswab32(__u32 val)
There are at least two, possibly three problems here:
a) when writing into the FIFO, we copy the pointer rather than the
actual data we want to give to the hardware
b) the data pointer is an array of 8-bit values, while the FIFO
is 32-bit wide, so both the read and write access fail to do
a proper type conversion
c) This seems incorrect for big-endian kernels, on which we need to
byte-swap any register access, but not normally FIFO accesses,
at least the DMA case doesn't do it either.
This converts the bogus loop to use the same readsl/writesl pair
that we use for the two other modes (DMA and polling). This is
more efficient and consistent, and probably correct for endianess.
The bug has existed since the driver was first merged, and was
probably never detected because nobody tried to use interrupt mode.
It might make sense to backport this fix to stable kernels, depending
on how the crypto maintainers feel about that.
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: linux-crypto@vger.kernel.org
Cc: Fabio Baltieri fabio.balti...@linaro.org
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/crypto/ux500/cryp/cryp_core.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
--- a/drivers/crypto/ux500/cryp/cryp_core.c
+++ b/drivers/crypto/ux500/cryp/cryp_core.c
@@ -190,7 +190,7 @@ static void add_session_id(struct cryp_c
static irqreturn_t cryp_interrupt_handler(int irq, void *param)
{
struct cryp_ctx *ctx;
- int i;
+ int count;
struct cryp_device_data *device_data;
if (param == NULL) {
@@ -215,12 +215,11 @@ static irqreturn_t cryp_interrupt_handle
if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_OUTPUT_FIFO)) {
if (ctx-outlen / ctx-blocksize 0) {
- for (i = 0; i ctx-blocksize / 4; i++) {
- *(ctx-outdata) = readl_relaxed(
- device_data-base-dout);
- ctx-outdata += 4;
- ctx-outlen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ readsl(device_data-base-dout, ctx-outdata, count);
+ ctx-outdata += count;
+ ctx-outlen -= count;
if (ctx-outlen == 0) {
cryp_disable_irq_src(device_data,
@@ -230,12 +229,12 @@ static irqreturn_t cryp_interrupt_handle
} else if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_INPUT_FIFO)) {
if (ctx-datalen / ctx-blocksize 0) {
- for (i = 0 ; i ctx-blocksize / 4; i++) {
- writel_relaxed(ctx-indata,
- device_data-base-din);
- ctx-indata += 4;
- ctx-datalen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ writesl(device_data-base-din, ctx-indata, count);
+
+ ctx-indata += count;
+ ctx-datalen -= count;
if (ctx-datalen == 0)
cryp_disable_irq_src(device_data,
--
To unsubscribe from this list: send the line unsubscribe
[PATCH 3.10 27/55] crypto: ux500 - make interrupt mode plausible
3.10-stable review patch. If anyone has any objections, please let me know.
--
From: Arnd Bergmann a...@arndb.de
commit e1f8859ee265fc89bd21b4dca79e8e983a044892 upstream.
The interrupt handler in the ux500 crypto driver has an obviously
incorrect way to access the data buffer, which for a while has
caused this build warning:
../ux500/cryp/cryp_core.c: In function 'cryp_interrupt_handler':
../ux500/cryp/cryp_core.c:234:5: warning: passing argument 1 of '__fswab32'
makes integer from pointer without a cast [enabled by default]
writel_relaxed(ctx-indata,
^
In file included from ../include/linux/swab.h:4:0,
from ../include/uapi/linux/byteorder/big_endian.h:12,
from ../include/linux/byteorder/big_endian.h:4,
from ../arch/arm/include/uapi/asm/byteorder.h:19,
from ../include/asm-generic/bitops/le.h:5,
from ../arch/arm/include/asm/bitops.h:340,
from ../include/linux/bitops.h:33,
from ../include/linux/kernel.h:10,
from ../include/linux/clk.h:16,
from ../drivers/crypto/ux500/cryp/cryp_core.c:12:
../include/uapi/linux/swab.h:57:119: note: expected '__u32' but argument is of
type 'const u8 *'
static inline __attribute_const__ __u32 __fswab32(__u32 val)
There are at least two, possibly three problems here:
a) when writing into the FIFO, we copy the pointer rather than the
actual data we want to give to the hardware
b) the data pointer is an array of 8-bit values, while the FIFO
is 32-bit wide, so both the read and write access fail to do
a proper type conversion
c) This seems incorrect for big-endian kernels, on which we need to
byte-swap any register access, but not normally FIFO accesses,
at least the DMA case doesn't do it either.
This converts the bogus loop to use the same readsl/writesl pair
that we use for the two other modes (DMA and polling). This is
more efficient and consistent, and probably correct for endianess.
The bug has existed since the driver was first merged, and was
probably never detected because nobody tried to use interrupt mode.
It might make sense to backport this fix to stable kernels, depending
on how the crypto maintainers feel about that.
Signed-off-by: Arnd Bergmann a...@arndb.de
Cc: linux-crypto@vger.kernel.org
Cc: Fabio Baltieri fabio.balti...@linaro.org
Cc: Linus Walleij linus.wall...@linaro.org
Cc: Herbert Xu herb...@gondor.apana.org.au
Cc: David S. Miller da...@davemloft.net
Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/crypto/ux500/cryp/cryp_core.c | 25 -
1 file changed, 12 insertions(+), 13 deletions(-)
--- a/drivers/crypto/ux500/cryp/cryp_core.c
+++ b/drivers/crypto/ux500/cryp/cryp_core.c
@@ -190,7 +190,7 @@ static void add_session_id(struct cryp_c
static irqreturn_t cryp_interrupt_handler(int irq, void *param)
{
struct cryp_ctx *ctx;
- int i;
+ int count;
struct cryp_device_data *device_data;
if (param == NULL) {
@@ -215,12 +215,11 @@ static irqreturn_t cryp_interrupt_handle
if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_OUTPUT_FIFO)) {
if (ctx-outlen / ctx-blocksize 0) {
- for (i = 0; i ctx-blocksize / 4; i++) {
- *(ctx-outdata) = readl_relaxed(
- device_data-base-dout);
- ctx-outdata += 4;
- ctx-outlen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ readsl(device_data-base-dout, ctx-outdata, count);
+ ctx-outdata += count;
+ ctx-outlen -= count;
if (ctx-outlen == 0) {
cryp_disable_irq_src(device_data,
@@ -230,12 +229,12 @@ static irqreturn_t cryp_interrupt_handle
} else if (cryp_pending_irq_src(device_data,
CRYP_IRQ_SRC_INPUT_FIFO)) {
if (ctx-datalen / ctx-blocksize 0) {
- for (i = 0 ; i ctx-blocksize / 4; i++) {
- writel_relaxed(ctx-indata,
- device_data-base-din);
- ctx-indata += 4;
- ctx-datalen -= 4;
- }
+ count = ctx-blocksize / 4;
+
+ writesl(device_data-base-din, ctx-indata, count);
+
+ ctx-indata += count;
+ ctx-datalen -= count;
if (ctx-datalen == 0)
cryp_disable_irq_src(device_data,
--
To unsubscribe from this list: send the line unsubscribe
Re: [PATCH v2 00/12] crypto: caam - Add RTA descriptor creation library
On Wed, 3 Sep 2014 12:59:34 +0300 Horia Geantă horia.gea...@freescale.com wrote: On 8/16/2014 2:16 PM, Kim Phillips wrote: On Thu, 14 Aug 2014 15:54:22 +0300 Horia Geanta horia.gea...@freescale.com wrote: This patch set adds Run Time Assembler (RTA) SEC descriptor library. RTA is a replacement for incumbent inline append. The library is intended to be a single code base for SEC descriptors creation for all Freescale products. This comes with a series of advantages, such as library being maintained / kept up-to-date with latest platforms, i.e. SEC functionalities (for e.g. SEC incarnations present in Layerscape LS1 and LS2). RTA detects options in SEC descriptors that are not supported by a SEC HW revision (Era) and reports this back. Say a descriptor uses Sequence Out Pointer (SOP) option for the SEQINPTR command, which is supported starting from SEC Era 5. If the descriptor would be built on a P4080R3 platform (which has SEC Era 4), RTA would report SEQ IN PTR: Flag(s) not supported by SEC Era 4. This is extremely useful and saves a lot of time wasted on debugging. SEC HW detects only *some* of these problems, leaving user wonder what causes a DECO Watchdog Timeout. And when it prints something more useful, sometimes it does not point to the exact opcode. again, RTA just adds bloat to the kernel driver - the kernel driver is supposed to generate the appropriate descriptor for its target running SEC version no matter what, not report back what is/is not supported. This is a flaw at the RTA design level, as far as the kernel driver is concerned. What is your understanding of developing a descriptor? First it needs to be written, then tested - within the kernel driver. Having no error checking in the code that generates descriptors increases testing / debugging time significantly. Again, SEC HW provides some error reporting, but in many cases this is a clueless Watchdog Timeout. SEC descriptors development is complex enough to deserve a few indications along the way. AFAICT, RTA doesn't address the Watchdog Timeout issue (which pretty much always means some part of the SEC has timed out waiting for more input data). This is something learnt pretty quickly by SEC developers, and something best to leave to the h/w anyway, since it would be too cumbersome to add to descriptor construction. So instead of using RTA, we can discuss enhancing error.c messages to address cluing in users wrt what the error means, but that change should probably start with the SEC documentation itself (which is what error.c messages are based on, and what developers should be referencing in the first place :). So RTA tells you what command flags are supported in which SEC versions. I'm pretty sure h/w behaviour covers that case, too, but the premise doesn't apply to the kernel driver, since adding support to the existing descriptors for a new feature flag implies knowing which SEC version it was introduced in, because the kernel must work on all versions of the SEC. This promotes a more constructive implementation design, i.e., the upper levels of the descriptor construction code shouldn't use the flag if the h/w doesn't support it in the first place: none of this 'reporting back' business. FWIW, I doubt that out-of-kernel users would want this feature either - it's bloat to them too, assuming they are working under the same constraints as the kernel. If a user experiences the 'flag unsupported' error, hypothetically they'd go back to the upper level descriptor construction code and adjust it accordingly, rendering the original check a waste of runtime after that point. This reinforces my notion that RTA was not well thought-out from the beginning. Probably a better place to do these basic checks is in the form of an external disassembler. In any case, most of the kernel's crypto API algorithm descriptors are already fixed, written and constantly get backward-compatibility tested on newer h/w via the kernel's testing infrastructure: RTA adds nothing but bloat, and for that reason, I consider it a regression, and therefore unacceptable for upstream inclusion. Sorry. Kim -- To unsubscribe from this list: send the line unsubscribe linux-crypto in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
