Re: [PATCH] Bluetooth: hci_bcsp: Use the shared CRC-CCITT byte helper

[David Laight]

On Mon,  7 Sep 2026 23:10:15 +0800
Xuhua Zhang <zhangxuhua@xxxxxxxxxxxxxxx> wrote:

> bcsp_crc_update() processes each byte as two nibbles, requiring two
> dependent table lookups for every header and payload byte when CRC is
> enabled.
> 
> The existing crc_ccitt_byte() helper implements the same reflected
> polynomial with one lookup per byte. Use it instead of the private
> nibble-based implementation and select CRC_CCITT for BCSP-only UART
> configurations as well. The initial CRC value and final bit reversal
> remain unchanged.
> 
> This replaces the private 16-entry table with the shared 256-entry table,
> trading table size for fewer dependent lookups. An exhaustive comparison
> of all 65536 CRC states and 256 input bytes matches both the old code and
> a bitwise reference implementation.

How about this version?
static inline u16 crc_ccitt_byte(u16 crc, u8 c)
{
    c ^= crc;
    c ^= c << 4;
    return crc >> 8 ^ c << 8 ^ c << 3 ^ c >> 4;
}
Does the standard crc used for hdlc (etc).
Avoids the data cache misses associated with the array lookup
(which can make the nibble version faster than the byte one for short buffers).
A modern cpu will execute some of the instructions in parallel,
but you lose a clock because gcc converts (a ^ b) ^ (c ^ d) into
a ^ ( b ^ (c ^ d)) lengthening the register dependency chain by one.

David


> 
> Signed-off-by: Xuhua Zhang <zhangxuhua@xxxxxxxxxxxxxxx>
> ---
>  drivers/bluetooth/Kconfig    |  1 +
>  drivers/bluetooth/hci_bcsp.c | 26 +++-----------------------
>  2 files changed, 4 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index 4e8c24d757e9..2d6a3117e387 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -151,6 +151,7 @@ config BT_HCIUART_BCSP
>  	bool "BCSP protocol support"
>  	depends on BT_HCIUART
>  	select BITREVERSE
> +	select CRC_CCITT
>  	help
>  	  BCSP (BlueCore Serial Protocol) is serial protocol for communication
>  	  between Bluetooth device and host. This protocol is required for non
> diff --git a/drivers/bluetooth/hci_bcsp.c b/drivers/bluetooth/hci_bcsp.c
> index 0323db21c428..ef71a349e777 100644
> --- a/drivers/bluetooth/hci_bcsp.c
> +++ b/drivers/bluetooth/hci_bcsp.c
> @@ -25,6 +25,7 @@
>  #include <linux/ioctl.h>
>  #include <linux/skbuff.h>
>  #include <linux/bitrev.h>
> +#include <linux/crc-ccitt.h>
>  #include <linux/unaligned.h>
>  
>  #include <net/bluetooth/bluetooth.h>
> @@ -75,34 +76,13 @@ struct bcsp_struct {
>  
>  /* ---- BCSP CRC calculation ---- */
>  
> -/* Table for calculating CRC for polynomial 0x1021, LSB processed first,
> - * initial value 0xffff, bits shifted in reverse order.
> - */
> -
> -static const u16 crc_table[] = {
> -	0x0000, 0x1081, 0x2102, 0x3183,
> -	0x4204, 0x5285, 0x6306, 0x7387,
> -	0x8408, 0x9489, 0xa50a, 0xb58b,
> -	0xc60c, 0xd68d, 0xe70e, 0xf78f
> -};
> -
>  /* Initialise the crc calculator */
>  #define BCSP_CRC_INIT(x) x = 0xffff
>  
> -/* Update crc with next data byte
> - *
> - * Implementation note
> - *     The data byte is treated as two nibbles.  The crc is generated
> - *     in reverse, i.e., bits are fed into the register from the top.
> - */
> +/* Update crc with next data byte */
>  static void bcsp_crc_update(u16 *crc, u8 d)
>  {
> -	u16 reg = *crc;
> -
> -	reg = (reg >> 4) ^ crc_table[(reg ^ d) & 0x000f];
> -	reg = (reg >> 4) ^ crc_table[(reg ^ (d >> 4)) & 0x000f];
> -
> -	*crc = reg;
> +	*crc = crc_ccitt_byte(*crc, d);
>  }
>  
>  /* ---- BCSP core ---- */