Re: [PATCH 01/12] staging: ccree: correct coding style violations

2017-05-29 Thread Gilad Ben-Yossef 
On Mon, May 29, 2017 at 8:36 PM, Joe Perches  wrote:
> On Mon, 2017-05-29 at 20:11 +0300, Gilad Ben-Yossef wrote:
>> On Mon, May 29, 2017 at 7:57 PM, Joe Perches  wrote:
>> > On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
>> > > On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
>> > > > cc_crypto_ctx.h had multiple coding style violations reported by
>> > > > checkpatch. Fix them all.
>> > >
>> > > Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
>> > > style issues is not "one thing".  I wouldn't take this kind of patch
>> > > from anyone else, so why should I take it from you?  :)
>> >
>> > Because he's the named MAINTAINER of the subsystem
>> > and you are acting as an upstream conduit.
>> >
>>
>> LOL. Thank you Joe, but I have opted to upstream via staging to get the 
>> guidance
>> and review of Greg and other developers kind enough to offer it, and I'd be a
>> fool not to listen to them.
>
> For reviews of technical merit, true.
>
> For reviews of commit log wording of whitespace
> changes, where git diff -w shows no difference,
> less true.
>
> This patch seems almost entirely whitespace except
> one bit reformatting a comment block.
>
> Breaking those down into changes like:
> added space after commas
> added spaces around bit shifts
> added spaces around arithmetic
> is simply excessive.

I'll admit that this was my line of reasoning as well for including it
in a single patch but I if Greg finds it easier to review broken
down I'm fine with that. Something tells me he sees a lot of patches... :-)

> The only comment I would have given would be to
> change the patch context that added line comment
> initiators to use the /** kernel-doc style.
>
> And maybe a style change of
>
> #define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE (CC_MULTI2_SYSTEM_KEY_SIZE + \
>   CC_MULTI2_DATA_KEY_SIZE)
>
> to
>
> #define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE \
> (CC_MULTI2_SYSTEM_KEY_SIZE + CC_MULTI2_DATA_KEY_SIZE)
>
> as it's sometimes easier to scan arithmetic on a single line.
>

Thanks for the feedback. I will include it in the revised series.

> btw: this #define seems misleading as it's used in both .min_keysize
> and .max_keysize with "+ 1" and key[CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE]
> is also used.
>

 I'll look into that. There are still areas in this driver I've
inherited that I find... intriguing :-)

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH 01/12] staging: ccree: correct coding style violations

2017-05-29 Thread Joe Perches 
On Mon, 2017-05-29 at 20:11 +0300, Gilad Ben-Yossef wrote:
> On Mon, May 29, 2017 at 7:57 PM, Joe Perches  wrote:
> > On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
> > > On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
> > > > cc_crypto_ctx.h had multiple coding style violations reported by
> > > > checkpatch. Fix them all.
> > > 
> > > Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
> > > style issues is not "one thing".  I wouldn't take this kind of patch
> > > from anyone else, so why should I take it from you?  :)
> > 
> > Because he's the named MAINTAINER of the subsystem
> > and you are acting as an upstream conduit.
> > 
> 
> LOL. Thank you Joe, but I have opted to upstream via staging to get the 
> guidance
> and review of Greg and other developers kind enough to offer it, and I'd be a
> fool not to listen to them.

For reviews of technical merit, true.

For reviews of commit log wording of whitespace
changes, where git diff -w shows no difference,
less true.

This patch seems almost entirely whitespace except
one bit reformatting a comment block.

Breaking those down into changes like:
added space after commas
added spaces around bit shifts
added spaces around arithmetic
is simply excessive.

The only comment I would have given would be to
change the patch context that added line comment
initiators to use the /** kernel-doc style.

And maybe a style change of

#define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE (CC_MULTI2_SYSTEM_KEY_SIZE + \
  CC_MULTI2_DATA_KEY_SIZE)

to 

#define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE \
(CC_MULTI2_SYSTEM_KEY_SIZE + CC_MULTI2_DATA_KEY_SIZE)

as it's sometimes easier to scan arithmetic on a single line.

btw: this #define seems misleading as it's used in both .min_keysize
and .max_keysize with "+ 1" and key[CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE]
is also used.

Re: [PATCH 01/12] staging: ccree: correct coding style violations

2017-05-29 Thread Gilad Ben-Yossef 
On Mon, May 29, 2017 at 7:57 PM, Joe Perches  wrote:
> On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
>> On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
>> > cc_crypto_ctx.h had multiple coding style violations reported by
>> > checkpatch. Fix them all.
>>
>> Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
>> style issues is not "one thing".  I wouldn't take this kind of patch
>> from anyone else, so why should I take it from you?  :)
>
> Because he's the named MAINTAINER of the subsystem
> and you are acting as an upstream conduit.
>

LOL. Thank you Joe, but I have opted to upstream via staging to get the guidance
and review of Greg and other developers kind enough to offer it, and I'd be a
fool not to listen to them.

Thanks,
Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker

"If you take a class in large-scale robotics, can you end up in a
situation where the homework eats your dog?"
 -- Jean-Baptiste Queru

Re: [PATCH 01/12] staging: ccree: correct coding style violations

2017-05-29 Thread Joe Perches 
On Mon, 2017-05-29 at 16:37 +0200, Greg Kroah-Hartman wrote:
> On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
> > cc_crypto_ctx.h had multiple coding style violations reported by
> > checkpatch. Fix them all.
> 
> Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
> style issues is not "one thing".  I wouldn't take this kind of patch
> from anyone else, so why should I take it from you?  :)

Because he's the named MAINTAINER of the subsystem
and you are acting as an upstream conduit.

Re: [PATCH 01/12] staging: ccree: correct coding style violations

2017-05-29 Thread Greg Kroah-Hartman 
On Sun, May 28, 2017 at 05:40:26PM +0300, Gilad Ben-Yossef wrote:
> cc_crypto_ctx.h had multiple coding style violations reported by
> checkpatch. Fix them all.

Sorry, no.  You need to do only one-thing-per-patch, and "fix all coding
style issues is not "one thing".  I wouldn't take this kind of patch
from anyone else, so why should I take it from you?  :)

thnaks,

greg k-h

[PATCH 01/12] staging: ccree: correct coding style violations

2017-05-28 Thread Gilad Ben-Yossef 
cc_crypto_ctx.h had multiple coding style violations reported by
checkpatch. Fix them all.

Signed-off-by: Gilad Ben-Yossef 
---
 drivers/staging/ccree/cc_crypto_ctx.h | 66 +--
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/ccree/cc_crypto_ctx.h 
b/drivers/staging/ccree/cc_crypto_ctx.h
index ac39d34..0823b0f 100644
--- a/drivers/staging/ccree/cc_crypto_ctx.h
+++ b/drivers/staging/ccree/cc_crypto_ctx.h
@@ -14,7 +14,6 @@
  * along with this program; if not, see .
  */
 
-
 #ifndef _CC_CRYPTO_CTX_H_
 #define _CC_CRYPTO_CTX_H_
 
@@ -28,7 +27,7 @@
 #define CC_CTX_SIZE_LOG2 7
 #endif
 #endif
-#define CC_CTX_SIZE (1<> 2)
 
 #define CC_DRV_DES_IV_SIZE 8
@@ -54,13 +53,13 @@
 #define CC_AES_KEY_SIZE_MAXCC_AES_256_BIT_KEY_SIZE
 #define CC_AES_KEY_SIZE_WORDS_MAX  (CC_AES_KEY_SIZE_MAX >> 2)
 
-#define CC_MD5_DIGEST_SIZE 16
-#define CC_SHA1_DIGEST_SIZE20
-#define CC_SHA224_DIGEST_SIZE  28
-#define CC_SHA256_DIGEST_SIZE  32
+#define CC_MD5_DIGEST_SIZE 16
+#define CC_SHA1_DIGEST_SIZE 20
+#define CC_SHA224_DIGEST_SIZE 28
+#define CC_SHA256_DIGEST_SIZE 32
 #define CC_SHA256_DIGEST_SIZE_IN_WORDS 8
-#define CC_SHA384_DIGEST_SIZE  48
-#define CC_SHA512_DIGEST_SIZE  64
+#define CC_SHA384_DIGEST_SIZE 48
+#define CC_SHA512_DIGEST_SIZE 64
 
 #define CC_SHA1_BLOCK_SIZE 64
 #define CC_SHA1_BLOCK_SIZE_IN_WORDS 16
@@ -83,18 +82,17 @@
 
 #define CC_HMAC_BLOCK_SIZE_MAX CC_HASH_BLOCK_SIZE_MAX
 
-#define CC_MULTI2_SYSTEM_KEY_SIZE  32
-#define CC_MULTI2_DATA_KEY_SIZE8
-#define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE   (CC_MULTI2_SYSTEM_KEY_SIZE + 
CC_MULTI2_DATA_KEY_SIZE)
-#defineCC_MULTI2_BLOCK_SIZE8
-#defineCC_MULTI2_IV_SIZE   8
-#defineCC_MULTI2_MIN_NUM_ROUNDS8
-#defineCC_MULTI2_MAX_NUM_ROUNDS128
-
+#define CC_MULTI2_SYSTEM_KEY_SIZE 32
+#define CC_MULTI2_DATA_KEY_SIZE8
+#define CC_MULTI2_SYSTEM_N_DATA_KEY_SIZE (CC_MULTI2_SYSTEM_KEY_SIZE + \
+ CC_MULTI2_DATA_KEY_SIZE)
+#defineCC_MULTI2_BLOCK_SIZE 8
+#defineCC_MULTI2_IV_SIZE 8
+#defineCC_MULTI2_MIN_NUM_ROUND 8
+#defineCC_MULTI2_MAX_NUM_ROUND 128
 
 #define CC_DRV_ALG_MAX_BLOCK_SIZE CC_HASH_BLOCK_SIZE_MAX
 
-
 enum drv_engine_type {
DRV_ENGINE_NULL = 0,
DRV_ENGINE_AES = 1,
@@ -178,7 +176,6 @@ enum drv_multi2_mode {
DRV_MULTI2_RESERVE32B = S32_MAX
 };
 
-
 /* drv_crypto_key_type[1:0] is mapped to cipher_do[1:0] */
 /* drv_crypto_key_type[2] is mapped to cipher_config2 */
 enum drv_crypto_key_type {
@@ -208,7 +205,6 @@ struct drv_ctx_generic {
enum drv_crypto_alg alg;
 } __attribute__((__may_alias__));
 
-
 struct drv_ctx_hash {
enum drv_crypto_alg alg; /* DRV_CRYPTO_ALG_HASH */
enum drv_hash_mode mode;
@@ -218,13 +214,14 @@ struct drv_ctx_hash {
CC_DIGEST_SIZE_MAX];
 };
 
-/*  drv_ctx_hmac should have the same structure as drv_ctx_hash except
-   k0, k0_size fields */
+/* NOTE! drv_ctx_hmac should have the same structure as drv_ctx_hash except
+ * k0, k0_size fields
+ */
 struct drv_ctx_hmac {
enum drv_crypto_alg alg; /* DRV_CRYPTO_ALG_HMAC */
enum drv_hash_mode mode;
u8 digest[CC_DIGEST_SIZE_MAX];
-   u32 k0[CC_HMAC_BLOCK_SIZE_MAX/sizeof(u32)];
+   u32 k0[CC_HMAC_BLOCK_SIZE_MAX / sizeof(u32)];
u32 k0_size;
/* reserve to end of allocated context size */
u8 reserved[CC_CTX_SIZE - 3 * sizeof(u32) -
@@ -240,16 +237,17 @@ struct drv_ctx_cipher {
u32 key_size; /* numeric value in bytes   */
u32 data_unit_size; /* required for XTS */
/* block_state is the AES engine block state.
-   *  It is used by the host to pass IV or counter at initialization.
-   *  It is used by SeP for intermediate block chaining state and for
-   *  returning MAC algorithms results.   */
+*  It is used by the host to pass IV or counter at initialization.
+*  It is used by SeP for intermediate block chaining state and for
+*  returning MAC algorithms results.
+*/
u8 block_state[CC_AES_BLOCK_SIZE];
u8 key[CC_AES_KEY_SIZE_MAX];
u8 xex_key[CC_AES_KEY_SIZE_MAX];
/* reserve to end of allocated context size */
u32 reserved[CC_DRV_CTX_SIZE_WORDS - 7 -
-   CC_AES_BLOCK_SIZE/sizeof(u32) - 2 *
-   (CC_AES_KEY_SIZE_MAX/sizeof(u32))];
+   CC_AES_BLOCK_SIZE / sizeof(u32) - 2 *
+   (CC_AES_KEY_SIZE_MAX / sizeof(u32))];
 };
 
 /* authentication and encryption with associated data class */
@@ -269,20 +267,20 @@ struct