[v2,1/3] lib/crc7: Shift crc7() output left 1 bit
diff mbox

Message ID 20140511100211.1334.qmail@ns.horizon.com
State New, archived
Headers show

Commit Message

George Spelvin May 11, 2014, 10:02 a.m. UTC
From 770aa22e6c9c92027e3e21797192ccabb3e7c70e Mon Sep 17 00:00:00 2001
From: George Spelvin <linux@horizon.com>
Date: Sat, 10 May 2014 10:32:57 -0400
Subject: [PATCH v2 1/3] lib/crc7: Shift crc7() output left 1 bit

This eliminates a 1-bit left shift in every single caller,
and makes the inner loop of the CRC computation more efficient.

Renamed crc7 to crc7_be (big-endian) since the interface changed.

Also purged #include <linux/crc7.h> from files that don't use it at all.

Signed-off-by: George Spelvin <linux@horizon.com>
---
v2: Functions renamed to reflect different results; Pavel Machek
prompted me to think of something not too ugly.

Since all of the affected drivers are officially orphaned, this is
being sent to a hodge-podge of people who touched the files recently.
Assuming the MMC people don't complain, is it okay to send this through
the wireless tree?

 drivers/mmc/host/mmc_spi.c           |  2 +-
 drivers/net/wireless/ti/wl1251/acx.c |  1 -
 drivers/net/wireless/ti/wl1251/cmd.c |  1 -
 drivers/net/wireless/ti/wl1251/spi.c |  3 +-
 drivers/net/wireless/ti/wlcore/spi.c |  3 +-
 include/linux/crc7.h                 |  8 ++--
 lib/crc7.c                           | 84 ++++++++++++++++++++----------------
 7 files changed, 53 insertions(+), 49 deletions(-)

Comments

Pavel Machek May 11, 2014, 10:32 a.m. UTC | #1
On Sun 2014-05-11 06:02:11, George Spelvin wrote:
> >From 770aa22e6c9c92027e3e21797192ccabb3e7c70e Mon Sep 17 00:00:00 2001
> From: George Spelvin <linux@horizon.com>
> Date: Sat, 10 May 2014 10:32:57 -0400
> Subject: [PATCH v2 1/3] lib/crc7: Shift crc7() output left 1 bit
> 
> This eliminates a 1-bit left shift in every single caller,
> and makes the inner loop of the CRC computation more efficient.
> 
> Renamed crc7 to crc7_be (big-endian) since the interface changed.
> 
> Also purged #include <linux/crc7.h> from files that don't use it at all.
> 
> Signed-off-by: George Spelvin <linux@horizon.com>

Reviewed-by: Pavel Machek <pavel@ucw.cz>

> v2: Functions renamed to reflect different results; Pavel Machek
> prompted me to think of something not too ugly.

Thanks!
									Pavel
Ulf Hansson May 14, 2014, 10:14 a.m. UTC | #2
On 11 May 2014 12:02, George Spelvin <linux@horizon.com> wrote:
> From 770aa22e6c9c92027e3e21797192ccabb3e7c70e Mon Sep 17 00:00:00 2001
> From: George Spelvin <linux@horizon.com>
> Date: Sat, 10 May 2014 10:32:57 -0400
> Subject: [PATCH v2 1/3] lib/crc7: Shift crc7() output left 1 bit
>
> This eliminates a 1-bit left shift in every single caller,
> and makes the inner loop of the CRC computation more efficient.
>
> Renamed crc7 to crc7_be (big-endian) since the interface changed.
>
> Also purged #include <linux/crc7.h> from files that don't use it at all.
>
> Signed-off-by: George Spelvin <linux@horizon.com>
> ---
> v2: Functions renamed to reflect different results; Pavel Machek
> prompted me to think of something not too ugly.
>
> Since all of the affected drivers are officially orphaned, this is
> being sent to a hodge-podge of people who touched the files recently.
> Assuming the MMC people don't complain, is it okay to send this through
> the wireless tree?

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Feel free to take it through the wireless tree.

Kind regards
Ulf Hansson

>
>  drivers/mmc/host/mmc_spi.c           |  2 +-
>  drivers/net/wireless/ti/wl1251/acx.c |  1 -
>  drivers/net/wireless/ti/wl1251/cmd.c |  1 -
>  drivers/net/wireless/ti/wl1251/spi.c |  3 +-
>  drivers/net/wireless/ti/wlcore/spi.c |  3 +-
>  include/linux/crc7.h                 |  8 ++--
>  lib/crc7.c                           | 84 ++++++++++++++++++++----------------
>  7 files changed, 53 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
> index 0a87e56913..338e2202ea 100644
> --- a/drivers/mmc/host/mmc_spi.c
> +++ b/drivers/mmc/host/mmc_spi.c
> @@ -472,7 +472,7 @@ mmc_spi_command_send(struct mmc_spi_host *host,
>         *cp++ = (u8)(arg >> 16);
>         *cp++ = (u8)(arg >> 8);
>         *cp++ = (u8)arg;
> -       *cp++ = (crc7(0, &data->status[1], 5) << 1) | 0x01;
> +       *cp++ = crc7_be(0, &data->status[1], 5) | 0x01;
>
>         /* Then, read up to 13 bytes (while writing all-ones):
>          *  - N(CR) (== 1..8) bytes of all-ones
> diff --git a/drivers/net/wireless/ti/wl1251/acx.c b/drivers/net/wireless/ti/wl1251/acx.c
> index 5a4ec56c83..5695628757 100644
> --- a/drivers/net/wireless/ti/wl1251/acx.c
> +++ b/drivers/net/wireless/ti/wl1251/acx.c
> @@ -2,7 +2,6 @@
>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> -#include <linux/crc7.h>
>
>  #include "wl1251.h"
>  #include "reg.h"
> diff --git a/drivers/net/wireless/ti/wl1251/cmd.c b/drivers/net/wireless/ti/wl1251/cmd.c
> index bf1fa18b97..ede31f048e 100644
> --- a/drivers/net/wireless/ti/wl1251/cmd.c
> +++ b/drivers/net/wireless/ti/wl1251/cmd.c
> @@ -2,7 +2,6 @@
>
>  #include <linux/module.h>
>  #include <linux/slab.h>
> -#include <linux/crc7.h>
>  #include <linux/etherdevice.h>
>
>  #include "wl1251.h"
> diff --git a/drivers/net/wireless/ti/wl1251/spi.c b/drivers/net/wireless/ti/wl1251/spi.c
> index b06d36d993..e94b57cd5a 100644
> --- a/drivers/net/wireless/ti/wl1251/spi.c
> +++ b/drivers/net/wireless/ti/wl1251/spi.c
> @@ -122,8 +122,7 @@ static void wl1251_spi_wake(struct wl1251 *wl)
>         crc[3] = cmd[6];
>         crc[4] = cmd[5];
>
> -       cmd[4] |= crc7(0, crc, WSPI_INIT_CMD_CRC_LEN) << 1;
> -       cmd[4] |= WSPI_INIT_CMD_END;
> +       cmd[4] = crc7_be(0, crc, WSPI_INIT_CMD_CRC_LEN) | WSPI_INIT_CMD_END;
>
>         t.tx_buf = cmd;
>         t.len = WSPI_INIT_CMD_LEN;
> diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
> index dbe826dd7c..0497353c4a 100644
> --- a/drivers/net/wireless/ti/wlcore/spi.c
> +++ b/drivers/net/wireless/ti/wlcore/spi.c
> @@ -151,8 +151,7 @@ static void wl12xx_spi_init(struct device *child)
>         crc[3] = cmd[6];
>         crc[4] = cmd[5];
>
> -       cmd[4] |= crc7(0, crc, WSPI_INIT_CMD_CRC_LEN) << 1;
> -       cmd[4] |= WSPI_INIT_CMD_END;
> +       cmd[4] = crc7_be(0, crc, WSPI_INIT_CMD_CRC_LEN) | WSPI_INIT_CMD_END;
>
>         t.tx_buf = cmd;
>         t.len = WSPI_INIT_CMD_LEN;
> diff --git a/include/linux/crc7.h b/include/linux/crc7.h
> index 1786e772d5..d590765106 100644
> --- a/include/linux/crc7.h
> +++ b/include/linux/crc7.h
> @@ -2,13 +2,13 @@
>  #define _LINUX_CRC7_H
>  #include <linux/types.h>
>
> -extern const u8 crc7_syndrome_table[256];
> +extern const u8 crc7_be_syndrome_table[256];
>
> -static inline u8 crc7_byte(u8 crc, u8 data)
> +static inline u8 crc7_be_byte(u8 crc, u8 data)
>  {
> -       return crc7_syndrome_table[(crc << 1) ^ data];
> +       return crc7_be_syndrome_table[crc ^ data];
>  }
>
> -extern u8 crc7(u8 crc, const u8 *buffer, size_t len);
> +extern u8 crc7_be(u8 crc, const u8 *buffer, size_t len);
>
>  #endif
> diff --git a/lib/crc7.c b/lib/crc7.c
> index f1c3a144ce..bf6255e239 100644
> --- a/lib/crc7.c
> +++ b/lib/crc7.c
> @@ -10,42 +10,47 @@
>  #include <linux/crc7.h>
>
>
> -/* Table for CRC-7 (polynomial x^7 + x^3 + 1) */
> -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
> +/*
> + * Table for CRC-7 (polynomial x^7 + x^3 + 1).
> + * This is a big-endian CRC (msbit is highest power of x),
> + * aligned so the msbit of the byte is the x^6 coefficient
> + * and the lsbit is not used.
> + */
> +const u8 crc7_be_syndrome_table[256] = {
> +       0x00, 0x12, 0x24, 0x36, 0x48, 0x5a, 0x6c, 0x7e,
> +       0x90, 0x82, 0xb4, 0xa6, 0xd8, 0xca, 0xfc, 0xee,
> +       0x32, 0x20, 0x16, 0x04, 0x7a, 0x68, 0x5e, 0x4c,
> +       0xa2, 0xb0, 0x86, 0x94, 0xea, 0xf8, 0xce, 0xdc,
> +       0x64, 0x76, 0x40, 0x52, 0x2c, 0x3e, 0x08, 0x1a,
> +       0xf4, 0xe6, 0xd0, 0xc2, 0xbc, 0xae, 0x98, 0x8a,
> +       0x56, 0x44, 0x72, 0x60, 0x1e, 0x0c, 0x3a, 0x28,
> +       0xc6, 0xd4, 0xe2, 0xf0, 0x8e, 0x9c, 0xaa, 0xb8,
> +       0xc8, 0xda, 0xec, 0xfe, 0x80, 0x92, 0xa4, 0xb6,
> +       0x58, 0x4a, 0x7c, 0x6e, 0x10, 0x02, 0x34, 0x26,
> +       0xfa, 0xe8, 0xde, 0xcc, 0xb2, 0xa0, 0x96, 0x84,
> +       0x6a, 0x78, 0x4e, 0x5c, 0x22, 0x30, 0x06, 0x14,
> +       0xac, 0xbe, 0x88, 0x9a, 0xe4, 0xf6, 0xc0, 0xd2,
> +       0x3c, 0x2e, 0x18, 0x0a, 0x74, 0x66, 0x50, 0x42,
> +       0x9e, 0x8c, 0xba, 0xa8, 0xd6, 0xc4, 0xf2, 0xe0,
> +       0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62, 0x70,
> +       0x82, 0x90, 0xa6, 0xb4, 0xca, 0xd8, 0xee, 0xfc,
> +       0x12, 0x00, 0x36, 0x24, 0x5a, 0x48, 0x7e, 0x6c,
> +       0xb0, 0xa2, 0x94, 0x86, 0xf8, 0xea, 0xdc, 0xce,
> +       0x20, 0x32, 0x04, 0x16, 0x68, 0x7a, 0x4c, 0x5e,
> +       0xe6, 0xf4, 0xc2, 0xd0, 0xae, 0xbc, 0x8a, 0x98,
> +       0x76, 0x64, 0x52, 0x40, 0x3e, 0x2c, 0x1a, 0x08,
> +       0xd4, 0xc6, 0xf0, 0xe2, 0x9c, 0x8e, 0xb8, 0xaa,
> +       0x44, 0x56, 0x60, 0x72, 0x0c, 0x1e, 0x28, 0x3a,
> +       0x4a, 0x58, 0x6e, 0x7c, 0x02, 0x10, 0x26, 0x34,
> +       0xda, 0xc8, 0xfe, 0xec, 0x92, 0x80, 0xb6, 0xa4,
> +       0x78, 0x6a, 0x5c, 0x4e, 0x30, 0x22, 0x14, 0x06,
> +       0xe8, 0xfa, 0xcc, 0xde, 0xa0, 0xb2, 0x84, 0x96,
> +       0x2e, 0x3c, 0x0a, 0x18, 0x66, 0x74, 0x42, 0x50,
> +       0xbe, 0xac, 0x9a, 0x88, 0xf6, 0xe4, 0xd2, 0xc0,
> +       0x1c, 0x0e, 0x38, 0x2a, 0x54, 0x46, 0x70, 0x62,
> +       0x8c, 0x9e, 0xa8, 0xba, 0xc4, 0xd6, 0xe0, 0xf2
>  };
> -EXPORT_SYMBOL(crc7_syndrome_table);
> +EXPORT_SYMBOL(crc7_be_syndrome_table);
>
>  /**
>   * crc7 - update the CRC7 for the data buffer
> @@ -55,14 +60,17 @@ EXPORT_SYMBOL(crc7_syndrome_table);
>   * Context: any
>   *
>   * Returns the updated CRC7 value.
> + * The CRC7 is left-aligned in the byte (the lsbit is always 0), as that
> + * makes the computation easier, and all callers want it in that form.
> + *
>   */
> -u8 crc7(u8 crc, const u8 *buffer, size_t len)
> +u8 crc7_be(u8 crc, const u8 *buffer, size_t len)
>  {
>         while (len--)
> -               crc = crc7_byte(crc, *buffer++);
> +               crc = crc7_be_byte(crc, *buffer++);
>         return crc;
>  }
> -EXPORT_SYMBOL(crc7);
> +EXPORT_SYMBOL(crc7_be);
>
>  MODULE_DESCRIPTION("CRC7 calculations");
>  MODULE_LICENSE("GPL");
> --
> 1.9.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Peter Anvin May 15, 2014, 12:37 a.m. UTC | #3
On 05/11/2014 03:02 AM, George Spelvin wrote:
> From 770aa22e6c9c92027e3e21797192ccabb3e7c70e Mon Sep 17 00:00:00 2001
> From: George Spelvin <linux@horizon.com>
> Date: Sat, 10 May 2014 10:32:57 -0400
> Subject: [PATCH v2 1/3] lib/crc7: Shift crc7() output left 1 bit
> 
> This eliminates a 1-bit left shift in every single caller,
> and makes the inner loop of the CRC computation more efficient.
> 
> Renamed crc7 to crc7_be (big-endian) since the interface changed.
> 
> Also purged #include <linux/crc7.h> from files that don't use it at all.
> 
> Signed-off-by: George Spelvin <linux@horizon.com>

If the whole point of this is to use it for MMC/SD cards, why not just
also subsume the OR 1 and call it crc7_mmc() or something like that.

(Which I'm all for doing... I don't know of any other crc7 users.)

	-hpa


--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
George Spelvin May 15, 2014, 1:15 a.m. UTC | #4
H. Peter Anvin wrote:
> If the whole point of this is to use it for MMC/SD cards, why not just
> also subsume the OR 1 and call it crc7_mmc() or something like that.
>
> (Which I'm all for doing... I don't know of any other crc7 users.)

You'll find all users in my patch series.  2/3 updates the MMC
card, while 3/3 hits drivers/net/wireless/ti/wl1251/spi.c and
drivers/net/wireless/ti/wlcore/spi.c (which I'm pretty sure aren't
MMC/SPI cards).

Now, it turns out that they *also* set the lsbit (calling it
WSPI_INIT_CMD_END).  However, it's not possible to put that into the CRC
table (it would mess up all bytes but the last), so an explicitly coded
"| 1" is required at the end.  Thic ends up being no saving at all to
execution path length, and only moves one instruction from three drivers
to shared code.  While making it harder to read the drivers.

A microscopic space saving (if and only if you have more than one of these
drivers loaded) and no performance improvement didn't seem worth it to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
H. Peter Anvin May 15, 2014, 1:26 a.m. UTC | #5
Seems to me to make the code easier to read, not harder. That was the whole point.

On May 14, 2014 6:15:51 PM PDT, George Spelvin <linux@horizon.com> wrote:
>H. Peter Anvin wrote:
>> If the whole point of this is to use it for MMC/SD cards, why not
>just
>> also subsume the OR 1 and call it crc7_mmc() or something like that.
>>
>> (Which I'm all for doing... I don't know of any other crc7 users.)
>
>You'll find all users in my patch series.  2/3 updates the MMC
>card, while 3/3 hits drivers/net/wireless/ti/wl1251/spi.c and
>drivers/net/wireless/ti/wlcore/spi.c (which I'm pretty sure aren't
>MMC/SPI cards).
>
>Now, it turns out that they *also* set the lsbit (calling it
>WSPI_INIT_CMD_END).  However, it's not possible to put that into the
>CRC
>table (it would mess up all bytes but the last), so an explicitly coded
>"| 1" is required at the end.  Thic ends up being no saving at all to
>execution path length, and only moves one instruction from three
>drivers
>to shared code.  While making it harder to read the drivers.
>
>A microscopic space saving (if and only if you have more than one of
>these
>drivers loaded) and no performance improvement didn't seem worth it to
>me.
George Spelvin May 15, 2014, 2:02 a.m. UTC | #6
H. Peter Anvin wrote:
> Seems to me to make the code easier to read, not harder. That was the
> whole point.

I thought it made it harder by moving the terminating 1 bit out of the
driver proper, where it has a symbolic name.  Thus someone comparing
the driver to the spec has to read another source file to figure out
how the driver gets that bit set.

As it is, the straight-line code in the driver corresponds very simply
with the packet as illustrated in the data sheets.

I agree it's really really minor either way, but so is the saving
we're going for.

Since it's not *clearly* an improvement, I err on the side of not
messing with a driver I can't test.
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
index 0a87e56913..338e2202ea 100644
--- a/drivers/mmc/host/mmc_spi.c
+++ b/drivers/mmc/host/mmc_spi.c
@@ -472,7 +472,7 @@  mmc_spi_command_send(struct mmc_spi_host *host,
 	*cp++ = (u8)(arg >> 16);
 	*cp++ = (u8)(arg >> 8);
 	*cp++ = (u8)arg;
-	*cp++ = (crc7(0, &data->status[1], 5) << 1) | 0x01;
+	*cp++ = crc7_be(0, &data->status[1], 5) | 0x01;
 
 	/* Then, read up to 13 bytes (while writing all-ones):
 	 *  - N(CR) (== 1..8) bytes of all-ones
diff --git a/drivers/net/wireless/ti/wl1251/acx.c b/drivers/net/wireless/ti/wl1251/acx.c
index 5a4ec56c83..5695628757 100644
--- a/drivers/net/wireless/ti/wl1251/acx.c
+++ b/drivers/net/wireless/ti/wl1251/acx.c
@@ -2,7 +2,6 @@ 
 
 #include <linux/module.h>
 #include <linux/slab.h>
-#include <linux/crc7.h>
 
 #include "wl1251.h"
 #include "reg.h"
diff --git a/drivers/net/wireless/ti/wl1251/cmd.c b/drivers/net/wireless/ti/wl1251/cmd.c
index bf1fa18b97..ede31f048e 100644
--- a/drivers/net/wireless/ti/wl1251/cmd.c
+++ b/drivers/net/wireless/ti/wl1251/cmd.c
@@ -2,7 +2,6 @@ 
 
 #include <linux/module.h>
 #include <linux/slab.h>
-#include <linux/crc7.h>
 #include <linux/etherdevice.h>
 
 #include "wl1251.h"
diff --git a/drivers/net/wireless/ti/wl1251/spi.c b/drivers/net/wireless/ti/wl1251/spi.c
index b06d36d993..e94b57cd5a 100644
--- a/drivers/net/wireless/ti/wl1251/spi.c
+++ b/drivers/net/wireless/ti/wl1251/spi.c
@@ -122,8 +122,7 @@  static void wl1251_spi_wake(struct wl1251 *wl)
 	crc[3] = cmd[6];
 	crc[4] = cmd[5];
 
-	cmd[4] |= crc7(0, crc, WSPI_INIT_CMD_CRC_LEN) << 1;
-	cmd[4] |= WSPI_INIT_CMD_END;
+	cmd[4] = crc7_be(0, crc, WSPI_INIT_CMD_CRC_LEN) | WSPI_INIT_CMD_END;
 
 	t.tx_buf = cmd;
 	t.len = WSPI_INIT_CMD_LEN;
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index dbe826dd7c..0497353c4a 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -151,8 +151,7 @@  static void wl12xx_spi_init(struct device *child)
 	crc[3] = cmd[6];
 	crc[4] = cmd[5];
 
-	cmd[4] |= crc7(0, crc, WSPI_INIT_CMD_CRC_LEN) << 1;
-	cmd[4] |= WSPI_INIT_CMD_END;
+	cmd[4] = crc7_be(0, crc, WSPI_INIT_CMD_CRC_LEN) | WSPI_INIT_CMD_END;
 
 	t.tx_buf = cmd;
 	t.len = WSPI_INIT_CMD_LEN;
diff --git a/include/linux/crc7.h b/include/linux/crc7.h
index 1786e772d5..d590765106 100644
--- a/include/linux/crc7.h
+++ b/include/linux/crc7.h
@@ -2,13 +2,13 @@ 
 #define _LINUX_CRC7_H
 #include <linux/types.h>
 
-extern const u8 crc7_syndrome_table[256];
+extern const u8 crc7_be_syndrome_table[256];
 
-static inline u8 crc7_byte(u8 crc, u8 data)
+static inline u8 crc7_be_byte(u8 crc, u8 data)
 {
-	return crc7_syndrome_table[(crc << 1) ^ data];
+	return crc7_be_syndrome_table[crc ^ data];
 }
 
-extern u8 crc7(u8 crc, const u8 *buffer, size_t len);
+extern u8 crc7_be(u8 crc, const u8 *buffer, size_t len);
 
 #endif
diff --git a/lib/crc7.c b/lib/crc7.c
index f1c3a144ce..bf6255e239 100644
--- a/lib/crc7.c
+++ b/lib/crc7.c
@@ -10,42 +10,47 @@ 
 #include <linux/crc7.h>
 
 
-/* Table for CRC-7 (polynomial x^7 + x^3 + 1) */
-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
+/*
+ * Table for CRC-7 (polynomial x^7 + x^3 + 1).
+ * This is a big-endian CRC (msbit is highest power of x),
+ * aligned so the msbit of the byte is the x^6 coefficient
+ * and the lsbit is not used.
+ */
+const u8 crc7_be_syndrome_table[256] = {
+	0x00, 0x12, 0x24, 0x36, 0x48, 0x5a, 0x6c, 0x7e,
+	0x90, 0x82, 0xb4, 0xa6, 0xd8, 0xca, 0xfc, 0xee,
+	0x32, 0x20, 0x16, 0x04, 0x7a, 0x68, 0x5e, 0x4c,
+	0xa2, 0xb0, 0x86, 0x94, 0xea, 0xf8, 0xce, 0xdc,
+	0x64, 0x76, 0x40, 0x52, 0x2c, 0x3e, 0x08, 0x1a,
+	0xf4, 0xe6, 0xd0, 0xc2, 0xbc, 0xae, 0x98, 0x8a,
+	0x56, 0x44, 0x72, 0x60, 0x1e, 0x0c, 0x3a, 0x28,
+	0xc6, 0xd4, 0xe2, 0xf0, 0x8e, 0x9c, 0xaa, 0xb8,
+	0xc8, 0xda, 0xec, 0xfe, 0x80, 0x92, 0xa4, 0xb6,
+	0x58, 0x4a, 0x7c, 0x6e, 0x10, 0x02, 0x34, 0x26,
+	0xfa, 0xe8, 0xde, 0xcc, 0xb2, 0xa0, 0x96, 0x84,
+	0x6a, 0x78, 0x4e, 0x5c, 0x22, 0x30, 0x06, 0x14,
+	0xac, 0xbe, 0x88, 0x9a, 0xe4, 0xf6, 0xc0, 0xd2,
+	0x3c, 0x2e, 0x18, 0x0a, 0x74, 0x66, 0x50, 0x42,
+	0x9e, 0x8c, 0xba, 0xa8, 0xd6, 0xc4, 0xf2, 0xe0,
+	0x0e, 0x1c, 0x2a, 0x38, 0x46, 0x54, 0x62, 0x70,
+	0x82, 0x90, 0xa6, 0xb4, 0xca, 0xd8, 0xee, 0xfc,
+	0x12, 0x00, 0x36, 0x24, 0x5a, 0x48, 0x7e, 0x6c,
+	0xb0, 0xa2, 0x94, 0x86, 0xf8, 0xea, 0xdc, 0xce,
+	0x20, 0x32, 0x04, 0x16, 0x68, 0x7a, 0x4c, 0x5e,
+	0xe6, 0xf4, 0xc2, 0xd0, 0xae, 0xbc, 0x8a, 0x98,
+	0x76, 0x64, 0x52, 0x40, 0x3e, 0x2c, 0x1a, 0x08,
+	0xd4, 0xc6, 0xf0, 0xe2, 0x9c, 0x8e, 0xb8, 0xaa,
+	0x44, 0x56, 0x60, 0x72, 0x0c, 0x1e, 0x28, 0x3a,
+	0x4a, 0x58, 0x6e, 0x7c, 0x02, 0x10, 0x26, 0x34,
+	0xda, 0xc8, 0xfe, 0xec, 0x92, 0x80, 0xb6, 0xa4,
+	0x78, 0x6a, 0x5c, 0x4e, 0x30, 0x22, 0x14, 0x06,
+	0xe8, 0xfa, 0xcc, 0xde, 0xa0, 0xb2, 0x84, 0x96,
+	0x2e, 0x3c, 0x0a, 0x18, 0x66, 0x74, 0x42, 0x50,
+	0xbe, 0xac, 0x9a, 0x88, 0xf6, 0xe4, 0xd2, 0xc0,
+	0x1c, 0x0e, 0x38, 0x2a, 0x54, 0x46, 0x70, 0x62,
+	0x8c, 0x9e, 0xa8, 0xba, 0xc4, 0xd6, 0xe0, 0xf2
 };
-EXPORT_SYMBOL(crc7_syndrome_table);
+EXPORT_SYMBOL(crc7_be_syndrome_table);
 
 /**
  * crc7 - update the CRC7 for the data buffer
@@ -55,14 +60,17 @@  EXPORT_SYMBOL(crc7_syndrome_table);
  * Context: any
  *
  * Returns the updated CRC7 value.
+ * The CRC7 is left-aligned in the byte (the lsbit is always 0), as that
+ * makes the computation easier, and all callers want it in that form.
+ *
  */
-u8 crc7(u8 crc, const u8 *buffer, size_t len)
+u8 crc7_be(u8 crc, const u8 *buffer, size_t len)
 {
 	while (len--)
-		crc = crc7_byte(crc, *buffer++);
+		crc = crc7_be_byte(crc, *buffer++);
 	return crc;
 }
-EXPORT_SYMBOL(crc7);
+EXPORT_SYMBOL(crc7_be);
 
 MODULE_DESCRIPTION("CRC7 calculations");
 MODULE_LICENSE("GPL");