Re: [PATCH 02/12] staging: ccree: move to kernel bitfields/bitops

From: Greg Kroah-Hartman
Date: Mon May 29 2017 - 10:43:07 EST


On Sun, May 28, 2017 at 05:40:27PM +0300, Gilad Ben-Yossef wrote:
> ccree had a lot of boilerplate code for dealing with bitops
> and bitfield register access. Move it over to the generic kernel
> infrastructure used for doing the same.
>
> Signed-off-by: Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx>
> ---
> drivers/staging/ccree/cc_bitops.h | 11 +-
> drivers/staging/ccree/cc_hw_queue_defs.h | 679 ++++++++++++-----------
> drivers/staging/ccree/ssi_aead.c | 901 +++++++++++++++----------------
> drivers/staging/ccree/ssi_cipher.c | 224 ++++----
> drivers/staging/ccree/ssi_driver.h | 4 +-
> drivers/staging/ccree/ssi_fips_ll.c | 704 ++++++++++++------------
> drivers/staging/ccree/ssi_hash.c | 832 ++++++++++++++--------------
> drivers/staging/ccree/ssi_ivgen.c | 77 ++-
> drivers/staging/ccree/ssi_request_mgr.c | 25 +-
> drivers/staging/ccree/ssi_sram_mgr.c | 8 +-
> 10 files changed, 1765 insertions(+), 1700 deletions(-)
>
> diff --git a/drivers/staging/ccree/cc_bitops.h b/drivers/staging/ccree/cc_bitops.h
> index ee5238a..a12a65c 100644
> --- a/drivers/staging/ccree/cc_bitops.h
> +++ b/drivers/staging/ccree/cc_bitops.h
> @@ -21,9 +21,14 @@
> #ifndef _CC_BITOPS_H_
> #define _CC_BITOPS_H_
>
> -#define BITMASK(mask_size) (((mask_size) < 32) ? \
> - ((1UL << (mask_size)) - 1) : 0xFFFFFFFFUL)
> -#define BITMASK_AT(mask_size, mask_offset) (BITMASK(mask_size) << (mask_offset))
> +#include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +
> +#define BITMASK(mask_size) (((mask_size) < 32) ? \
> + ((1UL << (mask_size)) - 1) : 0xFFFFFFFFUL)
> +
> +#define BITMASK_AT(mask_size, mask_offset) \
> + (BITMASK(mask_size) << (mask_offset))

Why do you still needed these macros with this change?

>
> #define BITFIELD_GET(word, bit_offset, bit_size) \
> (((word) >> (bit_offset)) & BITMASK(bit_size))
> diff --git a/drivers/staging/ccree/cc_hw_queue_defs.h b/drivers/staging/ccree/cc_hw_queue_defs.h
> index 7138176..af10850 100644
> --- a/drivers/staging/ccree/cc_hw_queue_defs.h
> +++ b/drivers/staging/ccree/cc_hw_queue_defs.h
> @@ -22,25 +22,69 @@
> #include "cc_regs.h"
> #include "dx_crys_kernel.h"
>
> -/******************************************************************************
> -* DEFINITIONS
> -******************************************************************************/
> -
> -/* Dma AXI Secure bit */
> -#define AXI_SECURE 0
> -#define AXI_NOT_SECURE 1
> +/*****************************************************************************
> + * DEFINITIONS
> + *****************************************************************************/

That has nothing to do with changing bitops :(

>
> #define HW_DESC_SIZE_WORDS 6
> -#define HW_QUEUE_SLOTS_MAX 15 /* Max. available slots in HW queue */
> +#define HW_QUEUE_SLOTS_MAX 15 /* Max. available HW queue slots */

Neither does this :(

I'm stopping reviewing this here, please be more careful...

greg k-h