[02/10] crypto: omap-aes: Fix configuring of AES mode
diff mbox

Message ID 1435814320-30347-3-git-send-email-lokeshvutla@ti.com
State New
Headers show

Commit Message

Lokesh Vutla July 2, 2015, 5:18 a.m. UTC
AES_CTRL_REG is used to configure AES mode. Before configuring
any mode we need to make sure all other modes are reset or else
driver will misbehave. So mask all modes before configuring
any AES mode.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/crypto/omap-aes.c |   13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

Comments

Felipe Balbi July 2, 2015, 7:57 a.m. UTC | #1
On Thu, Jul 02, 2015 at 10:48:32AM +0530, Lokesh Vutla wrote:
> AES_CTRL_REG is used to configure AES mode. Before configuring
> any mode we need to make sure all other modes are reset or else
> driver will misbehave. So mask all modes before configuring
> any AES mode.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
>  drivers/crypto/omap-aes.c |   13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
> index a923101..96fc7f7 100644
> --- a/drivers/crypto/omap-aes.c
> +++ b/drivers/crypto/omap-aes.c
> @@ -63,6 +63,7 @@
>  #define AES_REG_CTRL_DIRECTION		(1 << 2)
>  #define AES_REG_CTRL_INPUT_READY	(1 << 1)
>  #define AES_REG_CTRL_OUTPUT_READY	(1 << 0)
> +#define AES_REG_CTRL_MASK		FLD_MASK(24, 2)

you end up masking bits which aren't even defined in this driver. What
are those bits ? Perhaps add macros for them and define
AES_REG_CTRL_MASK by explicitly ORing those macros ? That would, at
least, be clearer
Lokesh Vutla July 2, 2015, 9:43 a.m. UTC | #2
On Thursday 02 July 2015 01:27 PM, Felipe Balbi wrote:
> On Thu, Jul 02, 2015 at 10:48:32AM +0530, Lokesh Vutla wrote:
>> AES_CTRL_REG is used to configure AES mode. Before configuring
>> any mode we need to make sure all other modes are reset or else
>> driver will misbehave. So mask all modes before configuring
>> any AES mode.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>>  drivers/crypto/omap-aes.c |   13 +++++--------
>>  1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
>> index a923101..96fc7f7 100644
>> --- a/drivers/crypto/omap-aes.c
>> +++ b/drivers/crypto/omap-aes.c
>> @@ -63,6 +63,7 @@
>>  #define AES_REG_CTRL_DIRECTION		(1 << 2)
>>  #define AES_REG_CTRL_INPUT_READY	(1 << 1)
>>  #define AES_REG_CTRL_OUTPUT_READY	(1 << 0)
>> +#define AES_REG_CTRL_MASK		FLD_MASK(24, 2)
> 
> you end up masking bits which aren't even defined in this driver. What
> are those bits ? Perhaps add macros for them and define
> AES_REG_CTRL_MASK by explicitly ORing those macros ? That would, at
> least, be clearer
Hardware supports ECB, CBC, CTR, CFB, F8, CBC_MAC, F9, GCM, CCM, XTS modes.
But current driver has only ECB, CBC, CTR modes support.
That is why the other fields are not yet defined.
So, defining these is fine, but ORing all these will be very big and looks a bit ugly.
So I kept it as mask of all these bits.
Ill move it to GEN_MASK here only.

Thanks and regards,
Lokesh

> 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index a923101..96fc7f7 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -63,6 +63,7 @@ 
 #define AES_REG_CTRL_DIRECTION		(1 << 2)
 #define AES_REG_CTRL_INPUT_READY	(1 << 1)
 #define AES_REG_CTRL_OUTPUT_READY	(1 << 0)
+#define AES_REG_CTRL_MASK		FLD_MASK(24, 2)
 
 #define AES_REG_DATA_N(dd, x)		((dd)->pdata->data_ofs + ((x) * 0x04))
 
@@ -254,7 +255,7 @@  static int omap_aes_write_ctrl(struct omap_aes_dev *dd)
 {
 	unsigned int key32;
 	int i, err;
-	u32 val, mask = 0;
+	u32 val;
 
 	err = omap_aes_hw_init(dd);
 	if (err)
@@ -274,17 +275,13 @@  static int omap_aes_write_ctrl(struct omap_aes_dev *dd)
 	val = FLD_VAL(((dd->ctx->keylen >> 3) - 1), 4, 3);
 	if (dd->flags & FLAGS_CBC)
 		val |= AES_REG_CTRL_CBC;
-	if (dd->flags & FLAGS_CTR) {
+	if (dd->flags & FLAGS_CTR)
 		val |= AES_REG_CTRL_CTR | AES_REG_CTRL_CTR_WIDTH_128;
-		mask = AES_REG_CTRL_CTR | AES_REG_CTRL_CTR_WIDTH_MASK;
-	}
+
 	if (dd->flags & FLAGS_ENCRYPT)
 		val |= AES_REG_CTRL_DIRECTION;
 
-	mask |= AES_REG_CTRL_CBC | AES_REG_CTRL_DIRECTION |
-			AES_REG_CTRL_KEY_SIZE;
-
-	omap_aes_write_mask(dd, AES_REG_CTRL(dd), val, mask);
+	omap_aes_write_mask(dd, AES_REG_CTRL(dd), val, AES_REG_CTRL_MASK);
 
 	return 0;
 }