On Thu, Jun 22, 2017 at 12:04 PM, Dan Carpenter
<dan.carpenter@xxxxxxxxxx> wrote:
> On Thu, Jun 22, 2017 at 10:07:52AM +0300, Gilad Ben-Yossef wrote:
>> The ccree driver has build time configurable support
>> to work on top of coherent (e.g. ACP) vs. none coherent bus
>> connections. Turn it to run-time configurable option
>> based on device tree.
>>
>> Signed-off-by: Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx>
>> ---
>> drivers/staging/ccree/ssi_buffer_mgr.c | 37 ++++++++++++++++++----------------
>> drivers/staging/ccree/ssi_config.h | 20 ------------------
>> drivers/staging/ccree/ssi_driver.c | 14 +++++++++----
>> drivers/staging/ccree/ssi_driver.h | 3 +++
>> 4 files changed, 33 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/staging/ccree/ssi_buffer_mgr.c b/drivers/staging/ccree/ssi_buffer_mgr.c
>> index 88ebda8..75810dc 100644
>> --- a/drivers/staging/ccree/ssi_buffer_mgr.c
>> +++ b/drivers/staging/ccree/ssi_buffer_mgr.c
>> @@ -627,6 +627,9 @@ void ssi_buffer_mgr_unmap_aead_request(
>> struct aead_req_ctx *areq_ctx = aead_request_ctx(req);
>> unsigned int hw_iv_size = areq_ctx->hw_iv_size;
>> struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>> + struct ssi_drvdata *drvdata =
>> + (struct ssi_drvdata *)dev_get_drvdata(dev);
>
> The cast is not needed.
>
>> +
>
> Don't put a blank in the middle of the declaration block.
>
>> u32 dummy;
>> bool chained;
>> u32 size_to_unmap = 0;
Will fix.
>
> [ snip ]
>
>> @@ -981,20 +983,22 @@ static inline int ssi_buffer_mgr_prepare_aead_data_mlli(
>> * MAC verification upon request completion
>> */
>> if (direct == DRV_CRYPTO_DIRECTION_DECRYPT) {
>> -#if !DX_HAS_ACP
>> - /* In ACP platform we already copying ICV
>> - * for any INPLACE-DECRYPT operation, hence
>> + if (!drvdata->coherent) {
>> + /* In coherent platforms (e.g. ACP)
>> + * already copying ICV for any
>> + * INPLACE-DECRYPT operation, hence
>> * we must neglect this code.
>> */
>> - u32 size_to_skip = req->assoclen;
>> - if (areq_ctx->is_gcm4543) {
>> - size_to_skip += crypto_aead_ivsize(tfm);
>> + u32 skip = req->assoclen;
>> +
>> + if (areq_ctx->is_gcm4543)
>> + skip += crypto_aead_ivsize(tfm);
>> +
>> + ssi_buffer_mgr_copy_scatterlist_portion(
>> +areq_ctx->backup_mac, req->src,
>> +(skip + req->cryptlen - areq_ctx->req_authsize),
>> +skip + req->cryptlen, SSI_SG_TO_BUF);
>
>
> Indenting is messed up.
Sigh... having a function with a name as long as
ssi_buffer_mgr_copy_scatterlist_portion()
with such a deep indentation level is what is really messed up (but
that is for another patch
to fix).
I will indent it more sanely.
>
>> }
>> - ssi_buffer_mgr_copy_scatterlist_portion(
>> - areq_ctx->backup_mac, req->src,
>> - size_to_skip+ req->cryptlen - areq_ctx->req_authsize,
>> - size_to_skip+ req->cryptlen, SSI_SG_TO_BUF);
>> -#endif
>> areq_ctx->icv_virt_addr = areq_ctx->backup_mac;
>> } else {
>> areq_ctx->icv_virt_addr = areq_ctx->mac_buf;
>
> [ snip ]
>
>> @@ -533,7 +539,7 @@ int cc_clk_on(struct ssi_drvdata *drvdata)
>> struct clk *clk = drvdata->clk;
>>
>> if (IS_ERR(clk))
>> - /* No all devices have a clock associated with CCREE */
>> + /* Not all devices have a clock associated with CCREE */
>
> This is not related. Do unrelated things in different patches. This
> typo was introduced in an earlier patch. There are a couple ways in git
> to edit previous patches.
Yes, this was not intended.
>
>> goto out;
>>
>> rc = clk_prepare_enable(clk);
>
> regards,
> dan carpenter
Thanks for your time and great review comments!
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
