[PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[George Spelvin]

The code in lib/ is the desired polynomial, and even includes
the 1-bit left shift in the table rather than needing to code
it explicitly.

While I'm in Kconfig, add a description of what a WILC1000 is.
Kconfig questions that require me to look up a data sheet to
find out that I probably don't have one are a pet peeve.

Signed-off-by: George Spelvin <lkml@sdf.org>
Reviewed-by: Ajay Singh <ajay.kathat@microchip.com>
Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
Cc: linux-wireless@vger.kernel.org
---
v2: Rebase on staging-next tree
v2: Resend to Greg.  I sent it to Ajay, maintainer of the driver, for
    him to forward.  Should I have bypassed him?

 drivers/staging/wilc1000/Kconfig |  5 +++
 drivers/staging/wilc1000/spi.c   | 64 +++-----------------------------
 2 files changed, 11 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/wilc1000/Kconfig b/drivers/staging/wilc1000/Kconfig
index 59e58550d1397..80c92e8bf8a59 100644
--- a/drivers/staging/wilc1000/Kconfig
+++ b/drivers/staging/wilc1000/Kconfig
@@ -2,6 +2,10 @@
 config WILC1000
 	tristate
 	help
+	  Add support for the Atmel WILC1000 802.11 b/g/n SoC.
+	  This provides Wi-FI over an SDIO or SPI interface, and
+	  is usually found in IoT devices.
+
 	  This module only support IEEE 802.11n WiFi.
 
 config WILC1000_SDIO
@@ -22,6 +26,7 @@ config WILC1000_SPI
 	tristate "Atmel WILC1000 SPI (WiFi only)"
 	depends on CFG80211 && INET && SPI
 	select WILC1000
+	select CRC7
 	help
 	  This module adds support for the SPI interface of adapters using
 	  WILC1000 chipset. The Atmel WILC1000 has a Serial Peripheral
diff --git a/drivers/staging/wilc1000/spi.c b/drivers/staging/wilc1000/spi.c
index 8d4b8c219c2fc..3f19e3f38a397 100644
--- a/drivers/staging/wilc1000/spi.c
+++ b/drivers/staging/wilc1000/spi.c
@@ -6,6 +6,7 @@
 
 #include <linux/clk.h>
 #include <linux/spi/spi.h>
+#include <linux/crc7.h>
 
 #include "netdev.h"
 #include "cfg80211.h"
@@ -16,64 +17,6 @@ struct wilc_spi {
 
 static const struct wilc_hif_func wilc_hif_spi;
 
-/********************************************
- *
- *      Crc7
- *
- ********************************************/
-
-static const u8 crc7_syndrome_table[256] = {
-	0x00, 0x09, 0x12, 0x1b, 0x24, 0x2d, 0x36, 0x3f,
-	0x48, 0x41, 0x5a, 0x53, 0x6c, 0x65, 0x7e, 0x77,
-	0x19, 0x10, 0x0b, 0x02, 0x3d, 0x34, 0x2f, 0x26,
-	0x51, 0x58, 0x43, 0x4a, 0x75, 0x7c, 0x67, 0x6e,
-	0x32, 0x3b, 0x20, 0x29, 0x16, 0x1f, 0x04, 0x0d,
-	0x7a, 0x73, 0x68, 0x61, 0x5e, 0x57, 0x4c, 0x45,
-	0x2b, 0x22, 0x39, 0x30, 0x0f, 0x06, 0x1d, 0x14,
-	0x63, 0x6a, 0x71, 0x78, 0x47, 0x4e, 0x55, 0x5c,
-	0x64, 0x6d, 0x76, 0x7f, 0x40, 0x49, 0x52, 0x5b,
-	0x2c, 0x25, 0x3e, 0x37, 0x08, 0x01, 0x1a, 0x13,
-	0x7d, 0x74, 0x6f, 0x66, 0x59, 0x50, 0x4b, 0x42,
-	0x35, 0x3c, 0x27, 0x2e, 0x11, 0x18, 0x03, 0x0a,
-	0x56, 0x5f, 0x44, 0x4d, 0x72, 0x7b, 0x60, 0x69,
-	0x1e, 0x17, 0x0c, 0x05, 0x3a, 0x33, 0x28, 0x21,
-	0x4f, 0x46, 0x5d, 0x54, 0x6b, 0x62, 0x79, 0x70,
-	0x07, 0x0e, 0x15, 0x1c, 0x23, 0x2a, 0x31, 0x38,
-	0x41, 0x48, 0x53, 0x5a, 0x65, 0x6c, 0x77, 0x7e,
-	0x09, 0x00, 0x1b, 0x12, 0x2d, 0x24, 0x3f, 0x36,
-	0x58, 0x51, 0x4a, 0x43, 0x7c, 0x75, 0x6e, 0x67,
-	0x10, 0x19, 0x02, 0x0b, 0x34, 0x3d, 0x26, 0x2f,
-	0x73, 0x7a, 0x61, 0x68, 0x57, 0x5e, 0x45, 0x4c,
-	0x3b, 0x32, 0x29, 0x20, 0x1f, 0x16, 0x0d, 0x04,
-	0x6a, 0x63, 0x78, 0x71, 0x4e, 0x47, 0x5c, 0x55,
-	0x22, 0x2b, 0x30, 0x39, 0x06, 0x0f, 0x14, 0x1d,
-	0x25, 0x2c, 0x37, 0x3e, 0x01, 0x08, 0x13, 0x1a,
-	0x6d, 0x64, 0x7f, 0x76, 0x49, 0x40, 0x5b, 0x52,
-	0x3c, 0x35, 0x2e, 0x27, 0x18, 0x11, 0x0a, 0x03,
-	0x74, 0x7d, 0x66, 0x6f, 0x50, 0x59, 0x42, 0x4b,
-	0x17, 0x1e, 0x05, 0x0c, 0x33, 0x3a, 0x21, 0x28,
-	0x5f, 0x56, 0x4d, 0x44, 0x7b, 0x72, 0x69, 0x60,
-	0x0e, 0x07, 0x1c, 0x15, 0x2a, 0x23, 0x38, 0x31,
-	0x46, 0x4f, 0x54, 0x5d, 0x62, 0x6b, 0x70, 0x79
-};
-
-static u8 crc7_byte(u8 crc, u8 data)
-{
-	return crc7_syndrome_table[(crc << 1) ^ data];
-}
-
-static u8 crc7(u8 crc, const u8 *buffer, u32 len)
-{
-	while (len--)
-		crc = crc7_byte(crc, *buffer++);
-	return crc;
-}
-
-static u8 wilc_get_crc7(u8 *buffer, u32 len)
-{
-	return crc7(0x7f, (const u8 *)buffer, len) << 1;
-}
-
 /********************************************
  *
  *      Spi protocol Function
@@ -403,6 +346,11 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
  *      Spi Internal Read/Write Function
  *
  ********************************************/
+static u8 wilc_get_crc7(u8 *buffer, u32 len)
+{
+	return crc7_be(0xfe, buffer, len);
+}
+
 static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
 				u8 clockless)
 {

base-commit: 3017e587e36819f87e53d3c8751afdf987c1f542
-- 
2.26.0.rc2

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[Ajay.Kathat]

Hi George,

On 27/03/20 12:58 am, George Spelvin wrote:
> 
> The code in lib/ is the desired polynomial, and even includes
> the 1-bit left shift in the table rather than needing to code
> it explicitly.
> 
> While I'm in Kconfig, add a description of what a WILC1000 is.
> Kconfig questions that require me to look up a data sheet to
> find out that I probably don't have one are a pet peeve.
> 
> Signed-off-by: George Spelvin <lkml@sdf.org>
> Reviewed-by: Ajay Singh <ajay.kathat@microchip.com>
> Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
> Cc: linux-wireless@vger.kernel.org
> ---
> v2: Rebase on staging-next tree
> v2: Resend to Greg.  I sent it to Ajay, maintainer of the driver, for
>     him to forward.  Should I have bypassed him?

Currently, wilc1000 driver is present in staging so while submitting
patches please mark 'devel@driverdev.osuosl.org' mail-id also.
Btw, I have already resubmitted your patch by adding mailing group. The
patch is applied to 'staging-next' branch, so please pull the latest
staging to check the changes.
For future patches, please also add 'devel@driverdev.osuosl.org' mailing-id.

Regards,
Ajay

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[George Spelvin]

On Fri, Mar 27, 2020 at 07:03:28AM +0000, Ajay.Kathat@microchip.com wrote:
> Currently, wilc1000 driver is present in staging so while submitting
> patches please mark 'devel@driverdev.osuosl.org' mail-id also.

Aah, my error was to *only* check the staging/wilc1000/ entry in
MAINTAINERS, and not include the overall staging/ entries, which
scripts/get_maintainer.pl pulled out.

Sorry about that.

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[Greg KH]

On Thu, Mar 26, 2020 at 02:54:41PM +0000, Ajay.Kathat@microchip.com wrote:
> Hi Greg,
> 
> I just noticed this patch is not applied to staging. I suspect, its not
> picked because not sent to devel@driverdev.osuosl.org and 'staging' is
> missing from subject.

That's a very good reason for me to have never seen it, how else would I
have known to take it?  :)

> Please confirm if new version for patch should be submitted to apply in
> staging.

My staging queue is long-empty, please resend.

thanks,

greg k-h

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[Ajay.Kathat]

Hi Greg,

I just noticed this patch is not applied to staging. I suspect, its not
picked because not sent to devel@driverdev.osuosl.org and 'staging' is
missing from subject.
Please confirm if new version for patch should be submitted to apply in
staging.

Regards,
Ajay

On 23/03/20 7:44 pm, George Spelvin wrote:
> 
> The code in lib/ is the desired polynomial, and even includes
> the 1-bit left shift in the table rather than needing to code
> it explicitly.
> 
> While I'm in Kconfig, add a description of what a WILC1000 is.
> Kconfig questions that require me to look up a data sheet to
> find out that I probably don't have one are a pet peeve.
> 
> Signed-off-by: George Spelvin <lkml@sdf.org>
> Cc: Ajay Singh <ajay.kathat@microchip.com>
> Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
> Cc: linux-wireless@vger.kernel.org
> ---
> v2: Rebase on staging-next tree
> 
>  drivers/staging/wilc1000/Kconfig |  5 +++
>  drivers/staging/wilc1000/spi.c   | 64 +++-----------------------------
>  2 files changed, 11 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/Kconfig b/drivers/staging/wilc1000/Kconfig
> index 59e58550d1397..80c92e8bf8a59 100644
> --- a/drivers/staging/wilc1000/Kconfig
> +++ b/drivers/staging/wilc1000/Kconfig
> @@ -2,6 +2,10 @@
>  config WILC1000
>         tristate
>         help
> +         Add support for the Atmel WILC1000 802.11 b/g/n SoC.
> +         This provides Wi-FI over an SDIO or SPI interface, and
> +         is usually found in IoT devices.
> +
>           This module only support IEEE 802.11n WiFi.
> 
>  config WILC1000_SDIO
> @@ -22,6 +26,7 @@ config WILC1000_SPI
>         tristate "Atmel WILC1000 SPI (WiFi only)"
>         depends on CFG80211 && INET && SPI
>         select WILC1000
> +       select CRC7
>         help
>           This module adds support for the SPI interface of adapters using
>           WILC1000 chipset. The Atmel WILC1000 has a Serial Peripheral
> diff --git a/drivers/staging/wilc1000/spi.c b/drivers/staging/wilc1000/spi.c
> index 8d4b8c219c2fc..3f19e3f38a397 100644
> --- a/drivers/staging/wilc1000/spi.c
> +++ b/drivers/staging/wilc1000/spi.c
> @@ -6,6 +6,7 @@
> 
>  #include <linux/clk.h>
>  #include <linux/spi/spi.h>
> +#include <linux/crc7.h>
> 
>  #include "netdev.h"
>  #include "cfg80211.h"
> @@ -16,64 +17,6 @@ struct wilc_spi {
> 
>  static const struct wilc_hif_func wilc_hif_spi;
> 
> -/********************************************
> - *
> - *      Crc7
> - *
> - ********************************************/
> -
> -static const u8 crc7_syndrome_table[256] = {
> -       0x00, 0x09, 0x12, 0x1b, 0x24, 0x2d, 0x36, 0x3f,
> -       0x48, 0x41, 0x5a, 0x53, 0x6c, 0x65, 0x7e, 0x77,
> -       0x19, 0x10, 0x0b, 0x02, 0x3d, 0x34, 0x2f, 0x26,
> -       0x51, 0x58, 0x43, 0x4a, 0x75, 0x7c, 0x67, 0x6e,
> -       0x32, 0x3b, 0x20, 0x29, 0x16, 0x1f, 0x04, 0x0d,
> -       0x7a, 0x73, 0x68, 0x61, 0x5e, 0x57, 0x4c, 0x45,
> -       0x2b, 0x22, 0x39, 0x30, 0x0f, 0x06, 0x1d, 0x14,
> -       0x63, 0x6a, 0x71, 0x78, 0x47, 0x4e, 0x55, 0x5c,
> -       0x64, 0x6d, 0x76, 0x7f, 0x40, 0x49, 0x52, 0x5b,
> -       0x2c, 0x25, 0x3e, 0x37, 0x08, 0x01, 0x1a, 0x13,
> -       0x7d, 0x74, 0x6f, 0x66, 0x59, 0x50, 0x4b, 0x42,
> -       0x35, 0x3c, 0x27, 0x2e, 0x11, 0x18, 0x03, 0x0a,
> -       0x56, 0x5f, 0x44, 0x4d, 0x72, 0x7b, 0x60, 0x69,
> -       0x1e, 0x17, 0x0c, 0x05, 0x3a, 0x33, 0x28, 0x21,
> -       0x4f, 0x46, 0x5d, 0x54, 0x6b, 0x62, 0x79, 0x70,
> -       0x07, 0x0e, 0x15, 0x1c, 0x23, 0x2a, 0x31, 0x38,
> -       0x41, 0x48, 0x53, 0x5a, 0x65, 0x6c, 0x77, 0x7e,
> -       0x09, 0x00, 0x1b, 0x12, 0x2d, 0x24, 0x3f, 0x36,
> -       0x58, 0x51, 0x4a, 0x43, 0x7c, 0x75, 0x6e, 0x67,
> -       0x10, 0x19, 0x02, 0x0b, 0x34, 0x3d, 0x26, 0x2f,
> -       0x73, 0x7a, 0x61, 0x68, 0x57, 0x5e, 0x45, 0x4c,
> -       0x3b, 0x32, 0x29, 0x20, 0x1f, 0x16, 0x0d, 0x04,
> -       0x6a, 0x63, 0x78, 0x71, 0x4e, 0x47, 0x5c, 0x55,
> -       0x22, 0x2b, 0x30, 0x39, 0x06, 0x0f, 0x14, 0x1d,
> -       0x25, 0x2c, 0x37, 0x3e, 0x01, 0x08, 0x13, 0x1a,
> -       0x6d, 0x64, 0x7f, 0x76, 0x49, 0x40, 0x5b, 0x52,
> -       0x3c, 0x35, 0x2e, 0x27, 0x18, 0x11, 0x0a, 0x03,
> -       0x74, 0x7d, 0x66, 0x6f, 0x50, 0x59, 0x42, 0x4b,
> -       0x17, 0x1e, 0x05, 0x0c, 0x33, 0x3a, 0x21, 0x28,
> -       0x5f, 0x56, 0x4d, 0x44, 0x7b, 0x72, 0x69, 0x60,
> -       0x0e, 0x07, 0x1c, 0x15, 0x2a, 0x23, 0x38, 0x31,
> -       0x46, 0x4f, 0x54, 0x5d, 0x62, 0x6b, 0x70, 0x79
> -};
> -
> -static u8 crc7_byte(u8 crc, u8 data)
> -{
> -       return crc7_syndrome_table[(crc << 1) ^ data];
> -}
> -
> -static u8 crc7(u8 crc, const u8 *buffer, u32 len)
> -{
> -       while (len--)
> -               crc = crc7_byte(crc, *buffer++);
> -       return crc;
> -}
> -
> -static u8 wilc_get_crc7(u8 *buffer, u32 len)
> -{
> -       return crc7(0x7f, (const u8 *)buffer, len) << 1;
> -}
> -
>  /********************************************
>   *
>   *      Spi protocol Function
> @@ -403,6 +346,11 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>   *      Spi Internal Read/Write Function
>   *
>   ********************************************/
> +static u8 wilc_get_crc7(u8 *buffer, u32 len)
> +{
> +       return crc7_be(0xfe, buffer, len);
> +}
> +
>  static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
>                                 u8 clockless)
>  {
> 
> base-commit: 3017e587e36819f87e53d3c8751afdf987c1f542
> --
> 2.26.0.rc2
> 

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[Ajay.Kathat]

Hi George,

On 23/03/20 10:35 pm, George Spelvin wrote:
> 
>> Earlier, I also tried to replace crc7 by using existing library but it
>> gave different results with 'crc7_be()' because I didn't modify '0x7f'
>> to '0xfe'.
> 
> I had an afterthought that maybe documenting this in <linux/crc7.h>
> would be useful, since you're unlikely to be the last person to
> make this mistake.
> 
> Something like:
> 
> /*
>  * Generate a CRC with the polynomial x^7 + x^3 + 1 and big-endian
>  * bit order.  (Thus, the polynomial is 0x89.)  The result is in the
>  * most-significant 7 bits of the crc variable.
>  *
>  * This is where most protocols want the CRC (the lsbit is past the
>  * end of CRC coverage and is used for something else), but differs
>  * from most example code, which computes the CRC in the lsbits and
>  * does an explicit 1-bit shift at the end.
>  *
>  * Because the state is maintained left-aligned, the common "preset
>  * to all ones" CRC variant requires the crc be preset to 0xfe.
>  * (Right-aligned example code will show a preset to 0x7f.)
>  */
> 
> Feel free to add that to the patch (preserving my S-o-b) if you like.
> 

Sounds good. I will try to add these comments in a separate patch for
'linux/crc7.h'.

>> Thanks again for submitting the patch.
> 
> Thank you for writing the whole driver!  I know it can be a real PITA;
> Linux kernel developers Really Really Want drivers in a common style
> and using existing kernel facilities as much as possible, but you're
> usually starting from some internal driver that has its own,
> very different, support library.
> 

Over multiple code reviews and contribution from community has helped to
bring this driver to follow kernel recommendations. I hope the driver
will be soon out of staging.

> BTW, one thing I noticed at cfg80211.c:1132:
>         *cookie = prandom_u32();
>         priv->tx_cookie = *cookie;
> 
> I don't know what the cookie is for, but I notice that *cookie
> and priv->tx_cookie are both 64-bit data types.
> 
> Should that be "(u64)prandom_u32() << 32 | prandom_u32()"
> (since there is no prandom_u64())?


Actually, the cookie is used to match the management action frame
requests with its response status received from wilc1000 device.
The driver assign the cookie value for transmit management frame
received from user space and later while publishing status back it uses
the same cookie value. It's validation is taken care in user space  e.g
wpa_supplicant.

Even though the application expects u64-bit value but passing upscaled
u32-bit random value(prandom_u32() alone) should be enough for this case.

Regards,
Ajay

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[George Spelvin]

> Earlier, I also tried to replace crc7 by using existing library but it
> gave different results with 'crc7_be()' because I didn't modify '0x7f'
> to '0xfe'.

I had an afterthought that maybe documenting this in <linux/crc7.h>
would be useful, since you're unlikely to be the last person to
make this mistake.

Something like:

/*
 * Generate a CRC with the polynomial x^7 + x^3 + 1 and big-endian
 * bit order.  (Thus, the polynomial is 0x89.)  The result is in the
 * most-significant 7 bits of the crc variable.
 *
 * This is where most protocols want the CRC (the lsbit is past the
 * end of CRC coverage and is used for something else), but differs
 * from most example code, which computes the CRC in the lsbits and
 * does an explicit 1-bit shift at the end.
 *
 * Because the state is maintained left-aligned, the common "preset
 * to all ones" CRC variant requires the crc be preset to 0xfe.
 * (Right-aligned example code will show a preset to 0x7f.)
 */

Feel free to add that to the patch (preserving my S-o-b) if you like.

> Thanks again for submitting the patch.

Thank you for writing the whole driver!  I know it can be a real PITA;
Linux kernel developers Really Really Want drivers in a common style
and using existing kernel facilities as much as possible, but you're
usually starting from some internal driver that has its own, 
very different, support library.

BTW, one thing I noticed at cfg80211.c:1132:
	*cookie = prandom_u32();
	priv->tx_cookie = *cookie;

I don't know what the cookie is for, but I notice that *cookie
and priv->tx_cookie are both 64-bit data types.

Should that be "(u64)prandom_u32() << 32 | prandom_u32()"
(since there is no prandom_u64())?

Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[Ajay.Kathat]

Thanks George.

On 23/03/20 7:44 pm, George Spelvin wrote:
> 
> The code in lib/ is the desired polynomial, and even includes
> the 1-bit left shift in the table rather than needing to code
> it explicitly.
> 
> While I'm in Kconfig, add a description of what a WILC1000 is.
> Kconfig questions that require me to look up a data sheet to
> find out that I probably don't have one are a pet peeve.
> 
> Signed-off-by: George Spelvin <lkml@sdf.org>

The changes looks fine to me.

Reviewed-by: Ajay Singh <ajay.kathat@microchip.com>

Regards,
Ajay

> Cc: Ajay Singh <ajay.kathat@microchip.com>
> Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
> Cc: linux-wireless@vger.kernel.org
> ---
> v2: Rebase on staging-next tree
> 
>  drivers/staging/wilc1000/Kconfig |  5 +++
>  drivers/staging/wilc1000/spi.c   | 64 +++-----------------------------
>  2 files changed, 11 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/Kconfig b/drivers/staging/wilc1000/Kconfig
> index 59e58550d1397..80c92e8bf8a59 100644
> --- a/drivers/staging/wilc1000/Kconfig
> +++ b/drivers/staging/wilc1000/Kconfig
> @@ -2,6 +2,10 @@
>  config WILC1000
>         tristate
>         help
> +         Add support for the Atmel WILC1000 802.11 b/g/n SoC.
> +         This provides Wi-FI over an SDIO or SPI interface, and
> +         is usually found in IoT devices.
> +
>           This module only support IEEE 802.11n WiFi.
> 
>  config WILC1000_SDIO
> @@ -22,6 +26,7 @@ config WILC1000_SPI
>         tristate "Atmel WILC1000 SPI (WiFi only)"
>         depends on CFG80211 && INET && SPI
>         select WILC1000
> +       select CRC7
>         help
>           This module adds support for the SPI interface of adapters using
>           WILC1000 chipset. The Atmel WILC1000 has a Serial Peripheral
> diff --git a/drivers/staging/wilc1000/spi.c b/drivers/staging/wilc1000/spi.c
> index 8d4b8c219c2fc..3f19e3f38a397 100644
> --- a/drivers/staging/wilc1000/spi.c
> +++ b/drivers/staging/wilc1000/spi.c
> @@ -6,6 +6,7 @@
> 
>  #include <linux/clk.h>
>  #include <linux/spi/spi.h>
> +#include <linux/crc7.h>
> 
>  #include "netdev.h"
>  #include "cfg80211.h"
> @@ -16,64 +17,6 @@ struct wilc_spi {
> 
>  static const struct wilc_hif_func wilc_hif_spi;
> 
> -/********************************************
> - *
> - *      Crc7
> - *
> - ********************************************/
> -
> -static const u8 crc7_syndrome_table[256] = {
> -       0x00, 0x09, 0x12, 0x1b, 0x24, 0x2d, 0x36, 0x3f,
> -       0x48, 0x41, 0x5a, 0x53, 0x6c, 0x65, 0x7e, 0x77,
> -       0x19, 0x10, 0x0b, 0x02, 0x3d, 0x34, 0x2f, 0x26,
> -       0x51, 0x58, 0x43, 0x4a, 0x75, 0x7c, 0x67, 0x6e,
> -       0x32, 0x3b, 0x20, 0x29, 0x16, 0x1f, 0x04, 0x0d,
> -       0x7a, 0x73, 0x68, 0x61, 0x5e, 0x57, 0x4c, 0x45,
> -       0x2b, 0x22, 0x39, 0x30, 0x0f, 0x06, 0x1d, 0x14,
> -       0x63, 0x6a, 0x71, 0x78, 0x47, 0x4e, 0x55, 0x5c,
> -       0x64, 0x6d, 0x76, 0x7f, 0x40, 0x49, 0x52, 0x5b,
> -       0x2c, 0x25, 0x3e, 0x37, 0x08, 0x01, 0x1a, 0x13,
> -       0x7d, 0x74, 0x6f, 0x66, 0x59, 0x50, 0x4b, 0x42,
> -       0x35, 0x3c, 0x27, 0x2e, 0x11, 0x18, 0x03, 0x0a,
> -       0x56, 0x5f, 0x44, 0x4d, 0x72, 0x7b, 0x60, 0x69,
> -       0x1e, 0x17, 0x0c, 0x05, 0x3a, 0x33, 0x28, 0x21,
> -       0x4f, 0x46, 0x5d, 0x54, 0x6b, 0x62, 0x79, 0x70,
> -       0x07, 0x0e, 0x15, 0x1c, 0x23, 0x2a, 0x31, 0x38,
> -       0x41, 0x48, 0x53, 0x5a, 0x65, 0x6c, 0x77, 0x7e,
> -       0x09, 0x00, 0x1b, 0x12, 0x2d, 0x24, 0x3f, 0x36,
> -       0x58, 0x51, 0x4a, 0x43, 0x7c, 0x75, 0x6e, 0x67,
> -       0x10, 0x19, 0x02, 0x0b, 0x34, 0x3d, 0x26, 0x2f,
> -       0x73, 0x7a, 0x61, 0x68, 0x57, 0x5e, 0x45, 0x4c,
> -       0x3b, 0x32, 0x29, 0x20, 0x1f, 0x16, 0x0d, 0x04,
> -       0x6a, 0x63, 0x78, 0x71, 0x4e, 0x47, 0x5c, 0x55,
> -       0x22, 0x2b, 0x30, 0x39, 0x06, 0x0f, 0x14, 0x1d,
> -       0x25, 0x2c, 0x37, 0x3e, 0x01, 0x08, 0x13, 0x1a,
> -       0x6d, 0x64, 0x7f, 0x76, 0x49, 0x40, 0x5b, 0x52,
> -       0x3c, 0x35, 0x2e, 0x27, 0x18, 0x11, 0x0a, 0x03,
> -       0x74, 0x7d, 0x66, 0x6f, 0x50, 0x59, 0x42, 0x4b,
> -       0x17, 0x1e, 0x05, 0x0c, 0x33, 0x3a, 0x21, 0x28,
> -       0x5f, 0x56, 0x4d, 0x44, 0x7b, 0x72, 0x69, 0x60,
> -       0x0e, 0x07, 0x1c, 0x15, 0x2a, 0x23, 0x38, 0x31,
> -       0x46, 0x4f, 0x54, 0x5d, 0x62, 0x6b, 0x70, 0x79
> -};
> -
> -static u8 crc7_byte(u8 crc, u8 data)
> -{
> -       return crc7_syndrome_table[(crc << 1) ^ data];
> -}
> -
> -static u8 crc7(u8 crc, const u8 *buffer, u32 len)
> -{
> -       while (len--)
> -               crc = crc7_byte(crc, *buffer++);
> -       return crc;
> -}
> -
> -static u8 wilc_get_crc7(u8 *buffer, u32 len)
> -{
> -       return crc7(0x7f, (const u8 *)buffer, len) << 1;
> -}
> -
>  /********************************************
>   *
>   *      Spi protocol Function
> @@ -403,6 +346,11 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
>   *      Spi Internal Read/Write Function
>   *
>   ********************************************/
> +static u8 wilc_get_crc7(u8 *buffer, u32 len)
> +{
> +       return crc7_be(0xfe, buffer, len);
> +}
> +
>  static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
>                                 u8 clockless)
>  {
> 
> base-commit: 3017e587e36819f87e53d3c8751afdf987c1f542
> --
> 2.26.0.rc2
> 

[PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy

[George Spelvin]

The code in lib/ is the desired polynomial, and even includes
the 1-bit left shift in the table rather than needing to code
it explicitly.

While I'm in Kconfig, add a description of what a WILC1000 is.
Kconfig questions that require me to look up a data sheet to
find out that I probably don't have one are a pet peeve.

Signed-off-by: George Spelvin <lkml@sdf.org>
Cc: Ajay Singh <ajay.kathat@microchip.com>
Cc: Adham Abozaeid <adham.abozaeid@microchip.com>
Cc: linux-wireless@vger.kernel.org
---
v2: Rebase on staging-next tree

 drivers/staging/wilc1000/Kconfig |  5 +++
 drivers/staging/wilc1000/spi.c   | 64 +++-----------------------------
 2 files changed, 11 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/wilc1000/Kconfig b/drivers/staging/wilc1000/Kconfig
index 59e58550d1397..80c92e8bf8a59 100644
--- a/drivers/staging/wilc1000/Kconfig
+++ b/drivers/staging/wilc1000/Kconfig
@@ -2,6 +2,10 @@
 config WILC1000
 	tristate
 	help
+	  Add support for the Atmel WILC1000 802.11 b/g/n SoC.
+	  This provides Wi-FI over an SDIO or SPI interface, and
+	  is usually found in IoT devices.
+
 	  This module only support IEEE 802.11n WiFi.
 
 config WILC1000_SDIO
@@ -22,6 +26,7 @@ config WILC1000_SPI
 	tristate "Atmel WILC1000 SPI (WiFi only)"
 	depends on CFG80211 && INET && SPI
 	select WILC1000
+	select CRC7
 	help
 	  This module adds support for the SPI interface of adapters using
 	  WILC1000 chipset. The Atmel WILC1000 has a Serial Peripheral
diff --git a/drivers/staging/wilc1000/spi.c b/drivers/staging/wilc1000/spi.c
index 8d4b8c219c2fc..3f19e3f38a397 100644
--- a/drivers/staging/wilc1000/spi.c
+++ b/drivers/staging/wilc1000/spi.c
@@ -6,6 +6,7 @@
 
 #include <linux/clk.h>
 #include <linux/spi/spi.h>
+#include <linux/crc7.h>
 
 #include "netdev.h"
 #include "cfg80211.h"
@@ -16,64 +17,6 @@ struct wilc_spi {
 
 static const struct wilc_hif_func wilc_hif_spi;
 
-/********************************************
- *
- *      Crc7
- *
- ********************************************/
-
-static const u8 crc7_syndrome_table[256] = {
-	0x00, 0x09, 0x12, 0x1b, 0x24, 0x2d, 0x36, 0x3f,
-	0x48, 0x41, 0x5a, 0x53, 0x6c, 0x65, 0x7e, 0x77,
-	0x19, 0x10, 0x0b, 0x02, 0x3d, 0x34, 0x2f, 0x26,
-	0x51, 0x58, 0x43, 0x4a, 0x75, 0x7c, 0x67, 0x6e,
-	0x32, 0x3b, 0x20, 0x29, 0x16, 0x1f, 0x04, 0x0d,
-	0x7a, 0x73, 0x68, 0x61, 0x5e, 0x57, 0x4c, 0x45,
-	0x2b, 0x22, 0x39, 0x30, 0x0f, 0x06, 0x1d, 0x14,
-	0x63, 0x6a, 0x71, 0x78, 0x47, 0x4e, 0x55, 0x5c,
-	0x64, 0x6d, 0x76, 0x7f, 0x40, 0x49, 0x52, 0x5b,
-	0x2c, 0x25, 0x3e, 0x37, 0x08, 0x01, 0x1a, 0x13,
-	0x7d, 0x74, 0x6f, 0x66, 0x59, 0x50, 0x4b, 0x42,
-	0x35, 0x3c, 0x27, 0x2e, 0x11, 0x18, 0x03, 0x0a,
-	0x56, 0x5f, 0x44, 0x4d, 0x72, 0x7b, 0x60, 0x69,
-	0x1e, 0x17, 0x0c, 0x05, 0x3a, 0x33, 0x28, 0x21,
-	0x4f, 0x46, 0x5d, 0x54, 0x6b, 0x62, 0x79, 0x70,
-	0x07, 0x0e, 0x15, 0x1c, 0x23, 0x2a, 0x31, 0x38,
-	0x41, 0x48, 0x53, 0x5a, 0x65, 0x6c, 0x77, 0x7e,
-	0x09, 0x00, 0x1b, 0x12, 0x2d, 0x24, 0x3f, 0x36,
-	0x58, 0x51, 0x4a, 0x43, 0x7c, 0x75, 0x6e, 0x67,
-	0x10, 0x19, 0x02, 0x0b, 0x34, 0x3d, 0x26, 0x2f,
-	0x73, 0x7a, 0x61, 0x68, 0x57, 0x5e, 0x45, 0x4c,
-	0x3b, 0x32, 0x29, 0x20, 0x1f, 0x16, 0x0d, 0x04,
-	0x6a, 0x63, 0x78, 0x71, 0x4e, 0x47, 0x5c, 0x55,
-	0x22, 0x2b, 0x30, 0x39, 0x06, 0x0f, 0x14, 0x1d,
-	0x25, 0x2c, 0x37, 0x3e, 0x01, 0x08, 0x13, 0x1a,
-	0x6d, 0x64, 0x7f, 0x76, 0x49, 0x40, 0x5b, 0x52,
-	0x3c, 0x35, 0x2e, 0x27, 0x18, 0x11, 0x0a, 0x03,
-	0x74, 0x7d, 0x66, 0x6f, 0x50, 0x59, 0x42, 0x4b,
-	0x17, 0x1e, 0x05, 0x0c, 0x33, 0x3a, 0x21, 0x28,
-	0x5f, 0x56, 0x4d, 0x44, 0x7b, 0x72, 0x69, 0x60,
-	0x0e, 0x07, 0x1c, 0x15, 0x2a, 0x23, 0x38, 0x31,
-	0x46, 0x4f, 0x54, 0x5d, 0x62, 0x6b, 0x70, 0x79
-};
-
-static u8 crc7_byte(u8 crc, u8 data)
-{
-	return crc7_syndrome_table[(crc << 1) ^ data];
-}
-
-static u8 crc7(u8 crc, const u8 *buffer, u32 len)
-{
-	while (len--)
-		crc = crc7_byte(crc, *buffer++);
-	return crc;
-}
-
-static u8 wilc_get_crc7(u8 *buffer, u32 len)
-{
-	return crc7(0x7f, (const u8 *)buffer, len) << 1;
-}
-
 /********************************************
  *
  *      Spi protocol Function
@@ -403,6 +346,11 @@ static int spi_data_write(struct wilc *wilc, u8 *b, u32 sz)
  *      Spi Internal Read/Write Function
  *
  ********************************************/
+static u8 wilc_get_crc7(u8 *buffer, u32 len)
+{
+	return crc7_be(0xfe, buffer, len);
+}
+
 static int wilc_spi_single_read(struct wilc *wilc, u8 cmd, u32 adr, void *b,
 				u8 clockless)
 {

base-commit: 3017e587e36819f87e53d3c8751afdf987c1f542
-- 
2.26.0.rc2