Re: [PATCH v2 01/21] staging: wilc1000: replace crc7_byte() with inline macro CRC7_BYTE

[Greg KH <gregkh@linuxfoundation.org>]

On Wed, Apr 25, 2018 at 10:48:06PM +0530, Ajay Singh wrote:
> Replace the function call for crc7_byte() with macro CRC7_BYTE.
> crc7_byte() was called in close while(), so replaced it with macro to
> avoid extra functional call depth.
> 
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  drivers/staging/wilc1000/wilc_spi.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/wilc_spi.c b/drivers/staging/wilc1000/wilc_spi.c
> index 2cb9f4e..3bb8fec 100644
> --- a/drivers/staging/wilc1000/wilc_spi.c
> +++ b/drivers/staging/wilc1000/wilc_spi.c
> @@ -75,15 +75,12 @@ static const u8 crc7_syndrome_table[256] = {
>  	0x46, 0x4f, 0x54, 0x5d, 0x62, 0x6b, 0x70, 0x79
>  };
>  
> -static u8 crc7_byte(u8 crc, u8 data)
> -{
> -	return crc7_syndrome_table[(crc << 1) ^ data];
> -}
> +#define CRC7_BYTE(crc, data) crc7_syndrome_table[(crc << 1) ^ data]

Ick, no.  That's not needed at all, a function is always much better.

And a good compiler will just inline this, are you _sure_ you are
actually changing anything here?

But most importantly, why not just use the build-in crc7 functionality
that the kernel already provides?  Don't have a duplicate version here.

That would be a much better cleanup, right?

thanks,

greg k-h