Getting the ccree driver out of staging

Getting the ccree driver out of staging

[Gilad Ben-Yossef]

Hi folks,

With the enormous help of people in the to and CCed lists I've gotten
the ccree driver to a point I believe it is ready to graduate out of
the staging tree:

- The code base has been reduced by something by 30% and is *much*
more  readable and manageable.
- The very few checkpatch warnings are all false positives (one due to
dt bindings going through the crypto tree, the others due to harmless
macro argument reuse)
and the few sparse output are equally benign.
- The driver follows the crypto API expected behavior as much as I can tell.
- Fixed all the bugs I could fine as a result of tcrypt tests.
- It works :-)

There are obviously more things todo, but I believe they are out of
scope for staging.

So, if you see something that still needs work, kindly let me know.
Alternatively, will you accept a patch moving it to drivers/crypto/?

Many 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: Getting the ccree driver out of staging

[Greg Kroah-Hartman]

On Wed, Jan 10, 2018 at 08:54:55AM +0200, Gilad Ben-Yossef wrote:
> Hi folks,
> 
> With the enormous help of people in the to and CCed lists I've gotten
> the ccree driver to a point I believe it is ready to graduate out of
> the staging tree:
> 
> - The code base has been reduced by something by 30% and is *much*
> more  readable and manageable.
> - The very few checkpatch warnings are all false positives (one due to
> dt bindings going through the crypto tree, the others due to harmless
> macro argument reuse)
> and the few sparse output are equally benign.
> - The driver follows the crypto API expected behavior as much as I can tell.
> - Fixed all the bugs I could fine as a result of tcrypt tests.
> - It works :-)
> 
> There are obviously more things todo, but I believe they are out of
> scope for staging.
> 
> So, if you see something that still needs work, kindly let me know.
> Alternatively, will you accept a patch moving it to drivers/crypto/?

It might make more sense to just send a "new" patch adding the driver to
drivers/crypto/ and if that is accepted, we can drop it from staging.
That way it's easier for the crypto developers to review it.

But it's up to them, whatever they want is fine with me.

thanks,

greg k-h

Re: Getting the ccree driver out of staging

[Gilad Ben-Yossef]

On Wed, Jan 10, 2018 at 6:39 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Wed, Jan 10, 2018 at 08:54:55AM +0200, Gilad Ben-Yossef wrote:
>> Hi folks,
>>
>> With the enormous help of people in the to and CCed lists I've gotten
>> the ccree driver to a point I believe it is ready to graduate out of
>> the staging tree:
>>
>> - The code base has been reduced by something by 30% and is *much*
>> more  readable and manageable.
>> - The very few checkpatch warnings are all false positives (one due to
>> dt bindings going through the crypto tree, the others due to harmless
>> macro argument reuse)
>> and the few sparse output are equally benign.
>> - The driver follows the crypto API expected behavior as much as I can tell.
>> - Fixed all the bugs I could fine as a result of tcrypt tests.
>> - It works :-)
>>
>> There are obviously more things todo, but I believe they are out of
>> scope for staging.
>>
>> So, if you see something that still needs work, kindly let me know.
>> Alternatively, will you accept a patch moving it to drivers/crypto/?
>
> It might make more sense to just send a "new" patch adding the driver to
> drivers/crypto/ and if that is accepted, we can drop it from staging.
> That way it's easier for the crypto developers to review it.
>
> But it's up to them, whatever they want is fine with me.

Thanks, Greg.

Herbert, it's up to you - shall I send a new patch set or do you want to just
move what's in the staging tree?

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: Getting the ccree driver out of staging

[Herbert Xu]

On Thu, Jan 11, 2018 at 08:33:00AM +0200, Gilad Ben-Yossef wrote:
>
> Herbert, it's up to you - shall I send a new patch set or do you want to just
> move what's in the staging tree?

Please send a new patch.

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

Re: Getting the ccree driver out of staging

[Dan Carpenter]

Here are my remaining Smatch warnings:

    drivers/staging/ccree/cc_driver.c:219 init_cc_resources()
    error: '%pa' can only be followed by one of [dp]

drivers/staging/ccree/cc_driver.c
   217  
   218          if (rc) {
   219                  dev_err(dev, "Failed in dma_set_mask, mask=%par\n",
                                                                      ^
This 'r' is is treated as a 'p'.  Not sure what was intended.

   220                          &dma_mask);
   221                  return rc;
   222          }
   223  


    drivers/staging/ccree/cc_buffer_mgr.c:1067 cc_aead_chain_data()
    warn: inconsistent indenting

drivers/staging/ccree/cc_buffer_mgr.c
  1064          if (src_mapped_nents > LLI_MAX_NUM_OF_DATA_ENTRIES) {
  1065                  dev_err(dev, "Too many fragments. current %d max %d\n",
  1066                          src_mapped_nents, LLI_MAX_NUM_OF_DATA_ENTRIES);
  1067                          return -ENOMEM;
                                ^^^^^^^^^^^^^^
  1068          }

    drivers/staging/ccree/cc_cipher.c:373 cc_cipher_setkey()
    warn: inconsistent indenting

drivers/staging/ccree/cc_cipher.c
   369          dma_sync_single_for_device(dev, ctx_p->user.key_dma_addr,
   370                                     max_key_buf_size, DMA_TO_DEVICE);
   371          ctx_p->keylen = keylen;
   372  
   373           dev_dbg(dev, "return safely");
                ^
One extra space.

   374          return 0;
   375  }

regards,
dan carpenter

Re: Getting the ccree driver out of staging

[Gilad Ben-Yossef]

On Thu, Jan 11, 2018 at 10:42 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> Here are my remaining Smatch warnings:
>
>     drivers/staging/ccree/cc_driver.c:219 init_cc_resources()
>     error: '%pa' can only be followed by one of [dp]
>
> drivers/staging/ccree/cc_driver.c
>    217
>    218          if (rc) {
>    219                  dev_err(dev, "Failed in dma_set_mask, mask=%par\n",
>                                                                       ^
> This 'r' is is treated as a 'p'.  Not sure what was intended.
>
>    220                          &dma_mask);
>    221                  return rc;
>    222          }
>    223
>
>
>     drivers/staging/ccree/cc_buffer_mgr.c:1067 cc_aead_chain_data()
>     warn: inconsistent indenting
>
> drivers/staging/ccree/cc_buffer_mgr.c
>   1064          if (src_mapped_nents > LLI_MAX_NUM_OF_DATA_ENTRIES) {
>   1065                  dev_err(dev, "Too many fragments. current %d max %d\n",
>   1066                          src_mapped_nents, LLI_MAX_NUM_OF_DATA_ENTRIES);
>   1067                          return -ENOMEM;
>                                 ^^^^^^^^^^^^^^
>   1068          }
>
>     drivers/staging/ccree/cc_cipher.c:373 cc_cipher_setkey()
>     warn: inconsistent indenting
>
> drivers/staging/ccree/cc_cipher.c
>    369          dma_sync_single_for_device(dev, ctx_p->user.key_dma_addr,
>    370                                     max_key_buf_size, DMA_TO_DEVICE);
>    371          ctx_p->keylen = keylen;
>    372
>    373           dev_dbg(dev, "return safely");
>                 ^
> One extra space.
>
>    374          return 0;
>    375  }
>

Will fix in v2.

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