* [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib
@ 2011-06-15 11:53 Rafał Miłecki
2011-06-15 12:09 ` Arend van Spriel
0 siblings, 1 reply; 5+ messages in thread
From: Rafał Miłecki @ 2011-06-15 11:53 UTC (permalink / raw)
To: linux-wireless
Cc: John W. Linville, b43-dev, Rafał Miłecki,
Arend van Spriel
Cc: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
We implement crc8 function in bcma, while there is nice lib for this.
The problem is we need to calculate crc8 for SPROM which consists of
u16 entries. So this requires us to do proper convertion to avoid
endianess problems.
Do you think it is worth it? Should we do additional convertion just to
allow using standard library?
Or maybe we should just limit our changes to using crc8_populate_lsb
for generating look up table?
---
drivers/bcma/Kconfig | 1 +
drivers/bcma/sprom.c | 62 ++++++++++---------------------------
drivers/net/wireless/b43/Kconfig | 2 +-
3 files changed, 19 insertions(+), 46 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index 83e9adf..4e28fed 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -9,6 +9,7 @@ menu "Broadcom specific AMBA"
config BCMA
tristate "BCMA support"
depends on BCMA_POSSIBLE
+ select CRC8
help
Bus driver for Broadcom specific Advanced Microcontroller Bus
Architecture.
diff --git a/drivers/bcma/sprom.c b/drivers/bcma/sprom.c
index ffbb0e3..0ad6f01 100644
--- a/drivers/bcma/sprom.c
+++ b/drivers/bcma/sprom.c
@@ -9,6 +9,7 @@
#include <linux/bcma/bcma.h>
#include <linux/bcma/bcma_regs.h>
+#include <linux/crc8.h>
#include <linux/pci.h>
#include <linux/io.h>
#include <linux/dma-mapping.h>
@@ -32,56 +33,27 @@ static void bcma_sprom_read(struct bcma_bus *bus, u16 *sprom)
* Validation.
**************************************************/
-static inline u8 bcma_crc8(u8 crc, u8 data)
-{
- /* Polynomial: x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
- static const u8 t[] = {
- 0x00, 0xF7, 0xB9, 0x4E, 0x25, 0xD2, 0x9C, 0x6B,
- 0x4A, 0xBD, 0xF3, 0x04, 0x6F, 0x98, 0xD6, 0x21,
- 0x94, 0x63, 0x2D, 0xDA, 0xB1, 0x46, 0x08, 0xFF,
- 0xDE, 0x29, 0x67, 0x90, 0xFB, 0x0C, 0x42, 0xB5,
- 0x7F, 0x88, 0xC6, 0x31, 0x5A, 0xAD, 0xE3, 0x14,
- 0x35, 0xC2, 0x8C, 0x7B, 0x10, 0xE7, 0xA9, 0x5E,
- 0xEB, 0x1C, 0x52, 0xA5, 0xCE, 0x39, 0x77, 0x80,
- 0xA1, 0x56, 0x18, 0xEF, 0x84, 0x73, 0x3D, 0xCA,
- 0xFE, 0x09, 0x47, 0xB0, 0xDB, 0x2C, 0x62, 0x95,
- 0xB4, 0x43, 0x0D, 0xFA, 0x91, 0x66, 0x28, 0xDF,
- 0x6A, 0x9D, 0xD3, 0x24, 0x4F, 0xB8, 0xF6, 0x01,
- 0x20, 0xD7, 0x99, 0x6E, 0x05, 0xF2, 0xBC, 0x4B,
- 0x81, 0x76, 0x38, 0xCF, 0xA4, 0x53, 0x1D, 0xEA,
- 0xCB, 0x3C, 0x72, 0x85, 0xEE, 0x19, 0x57, 0xA0,
- 0x15, 0xE2, 0xAC, 0x5B, 0x30, 0xC7, 0x89, 0x7E,
- 0x5F, 0xA8, 0xE6, 0x11, 0x7A, 0x8D, 0xC3, 0x34,
- 0xAB, 0x5C, 0x12, 0xE5, 0x8E, 0x79, 0x37, 0xC0,
- 0xE1, 0x16, 0x58, 0xAF, 0xC4, 0x33, 0x7D, 0x8A,
- 0x3F, 0xC8, 0x86, 0x71, 0x1A, 0xED, 0xA3, 0x54,
- 0x75, 0x82, 0xCC, 0x3B, 0x50, 0xA7, 0xE9, 0x1E,
- 0xD4, 0x23, 0x6D, 0x9A, 0xF1, 0x06, 0x48, 0xBF,
- 0x9E, 0x69, 0x27, 0xD0, 0xBB, 0x4C, 0x02, 0xF5,
- 0x40, 0xB7, 0xF9, 0x0E, 0x65, 0x92, 0xDC, 0x2B,
- 0x0A, 0xFD, 0xB3, 0x44, 0x2F, 0xD8, 0x96, 0x61,
- 0x55, 0xA2, 0xEC, 0x1B, 0x70, 0x87, 0xC9, 0x3E,
- 0x1F, 0xE8, 0xA6, 0x51, 0x3A, 0xCD, 0x83, 0x74,
- 0xC1, 0x36, 0x78, 0x8F, 0xE4, 0x13, 0x5D, 0xAA,
- 0x8B, 0x7C, 0x32, 0xC5, 0xAE, 0x59, 0x17, 0xE0,
- 0x2A, 0xDD, 0x93, 0x64, 0x0F, 0xF8, 0xB6, 0x41,
- 0x60, 0x97, 0xD9, 0x2E, 0x45, 0xB2, 0xFC, 0x0B,
- 0xBE, 0x49, 0x07, 0xF0, 0x9B, 0x6C, 0x22, 0xD5,
- 0xF4, 0x03, 0x4D, 0xBA, 0xD1, 0x26, 0x68, 0x9F,
- };
- return t[crc ^ data];
-}
-
static u8 bcma_sprom_crc(const u16 *sprom)
{
- int word;
- u8 crc = 0xFF;
+ u8 crc;
+ u8 sprom2[SSB_SPROMSIZE_WORDS_R4 * 2 - 1];
+ u8 table[CRC8_TABLE_SIZE];
+ u16 word;
+ /* u16 to u8 */
for (word = 0; word < SSB_SPROMSIZE_WORDS_R4 - 1; word++) {
- crc = bcma_crc8(crc, sprom[word] & 0x00FF);
- crc = bcma_crc8(crc, (sprom[word] & 0xFF00) >> 8);
+ sprom2[word * 2] = sprom[word] & 0x00FF;
+ sprom2[(word * 2) + 1] = (sprom[word] & 0xFF00) >> 8;
}
- crc = bcma_crc8(crc, sprom[SSB_SPROMSIZE_WORDS_R4 - 1] & 0x00FF);
+ /* 127th byte */
+ sprom2[(SSB_SPROMSIZE_WORDS_R4 * 2) - 2] =
+ sprom[SSB_SPROMSIZE_WORDS_R4 - 1] & 0x00FF;
+
+ /* Prepare table, 0xAB is x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
+ crc8_populate_lsb(table, 0xAB);
+
+ /* Calculate */
+ crc = crc8(table, sprom2, (SSB_SPROMSIZE_WORDS_R4 * 2) - 1, 0xFF);
crc ^= 0xFF;
return crc;
--
1.7.3.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib
2011-06-15 11:53 [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib Rafał Miłecki
@ 2011-06-15 12:09 ` Arend van Spriel
2011-06-15 13:59 ` Arend van Spriel
2011-06-15 18:29 ` Rafał Miłecki
0 siblings, 2 replies; 5+ messages in thread
From: Arend van Spriel @ 2011-06-15 12:09 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, John W. Linville,
b43-dev@lists.infradead.org
On 06/15/2011 01:53 PM, Rafał Miłecki wrote:
> static u8 bcma_sprom_crc(const u16 *sprom)
> {
> - int word;
> - u8 crc = 0xFF;
> + u8 crc;
> + u8 sprom2[SSB_SPROMSIZE_WORDS_R4 * 2 - 1];
> + u8 table[CRC8_TABLE_SIZE];
> + u16 word;
>
> + /* u16 to u8 */
> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4 - 1; word++) {
> - crc = bcma_crc8(crc, sprom[word]& 0x00FF);
> - crc = bcma_crc8(crc, (sprom[word]& 0xFF00)>> 8);
> + sprom2[word * 2] = sprom[word]& 0x00FF;
> + sprom2[(word * 2) + 1] = (sprom[word]& 0xFF00)>> 8;
> }
> - crc = bcma_crc8(crc, sprom[SSB_SPROMSIZE_WORDS_R4 - 1]& 0x00FF);
> + /* 127th byte */
> + sprom2[(SSB_SPROMSIZE_WORDS_R4 * 2) - 2] =
> + sprom[SSB_SPROMSIZE_WORDS_R4 - 1]& 0x00FF;
> +
> + /* Prepare table, 0xAB is x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
> + crc8_populate_lsb(table, 0xAB);
> +
> + /* Calculate */
> + crc = crc8(table, sprom2, (SSB_SPROMSIZE_WORDS_R4 * 2) - 1, 0xFF);
> crc ^= 0xFF;
>
> return crc;
You could do (I think):
crc8_populate_lsb(table, 0xAB);
for (word = 0; word< SSB_SPROMSIZE_WORDS_R4; word++) {
u16 val = cpu_to_le16(sprom[word]);
crc = crc8(table,&val, sizeof(u16), crc);
}
Gr. AvS
--
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib
2011-06-15 12:09 ` Arend van Spriel
@ 2011-06-15 13:59 ` Arend van Spriel
2011-06-15 18:29 ` Rafał Miłecki
1 sibling, 0 replies; 5+ messages in thread
From: Arend van Spriel @ 2011-06-15 13:59 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, John W. Linville,
b43-dev@lists.infradead.org
On 06/15/2011 02:09 PM, Arend van Spriel wrote:
> On 06/15/2011 01:53 PM, Rafał Miłecki wrote:
>> static u8 bcma_sprom_crc(const u16 *sprom)
>> {
>> - int word;
>> - u8 crc = 0xFF;
>> + u8 crc;
>> + u8 sprom2[SSB_SPROMSIZE_WORDS_R4 * 2 - 1];
>> + u8 table[CRC8_TABLE_SIZE];
>> + u16 word;
>>
>> + /* u16 to u8 */
>> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4 - 1; word++) {
>> - crc = bcma_crc8(crc, sprom[word]& 0x00FF);
>> - crc = bcma_crc8(crc, (sprom[word]& 0xFF00)>> 8);
>> + sprom2[word * 2] = sprom[word]& 0x00FF;
>> + sprom2[(word * 2) + 1] = (sprom[word]& 0xFF00)>> 8;
>> }
>> - crc = bcma_crc8(crc, sprom[SSB_SPROMSIZE_WORDS_R4 - 1]& 0x00FF);
>> + /* 127th byte */
>> + sprom2[(SSB_SPROMSIZE_WORDS_R4 * 2) - 2] =
>> + sprom[SSB_SPROMSIZE_WORDS_R4 - 1]& 0x00FF;
>> +
>> + /* Prepare table, 0xAB is x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
>> + crc8_populate_lsb(table, 0xAB);
>> +
>> + /* Calculate */
>> + crc = crc8(table, sprom2, (SSB_SPROMSIZE_WORDS_R4 * 2) - 1, 0xFF);
>> crc ^= 0xFF;
>>
>> return crc;
> You could do (I think):
>
> crc8_populate_lsb(table, 0xAB);
> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4; word++) {
> u16 val = cpu_to_le16(sprom[word]);
> crc = crc8(table,&val, sizeof(u16), crc);
Obviously this should have proper cast here so: (u8 *)&val
Gr. AvS
--
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib
2011-06-15 12:09 ` Arend van Spriel
2011-06-15 13:59 ` Arend van Spriel
@ 2011-06-15 18:29 ` Rafał Miłecki
2011-06-15 19:26 ` Arend van Spriel
1 sibling, 1 reply; 5+ messages in thread
From: Rafał Miłecki @ 2011-06-15 18:29 UTC (permalink / raw)
To: Arend van Spriel
Cc: linux-wireless@vger.kernel.org, John W. Linville,
b43-dev@lists.infradead.org
W dniu 15 czerwca 2011 14:09 użytkownik Arend van Spriel
<arend@broadcom.com> napisał:
> On 06/15/2011 01:53 PM, Rafał Miłecki wrote:
>>
>> static u8 bcma_sprom_crc(const u16 *sprom)
>> {
>> - int word;
>> - u8 crc = 0xFF;
>> + u8 crc;
>> + u8 sprom2[SSB_SPROMSIZE_WORDS_R4 * 2 - 1];
>> + u8 table[CRC8_TABLE_SIZE];
>> + u16 word;
>>
>> + /* u16 to u8 */
>> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4 - 1; word++) {
>> - crc = bcma_crc8(crc, sprom[word]& 0x00FF);
>> - crc = bcma_crc8(crc, (sprom[word]& 0xFF00)>> 8);
>> + sprom2[word * 2] = sprom[word]& 0x00FF;
>> + sprom2[(word * 2) + 1] = (sprom[word]& 0xFF00)>> 8;
>> }
>> - crc = bcma_crc8(crc, sprom[SSB_SPROMSIZE_WORDS_R4 - 1]& 0x00FF);
>> + /* 127th byte */
>> + sprom2[(SSB_SPROMSIZE_WORDS_R4 * 2) - 2] =
>> + sprom[SSB_SPROMSIZE_WORDS_R4 - 1]&
>> 0x00FF;
>> +
>> + /* Prepare table, 0xAB is x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
>> + crc8_populate_lsb(table, 0xAB);
>> +
>> + /* Calculate */
>> + crc = crc8(table, sprom2, (SSB_SPROMSIZE_WORDS_R4 * 2) - 1, 0xFF);
>> crc ^= 0xFF;
>>
>> return crc;
>
> You could do (I think):
>
> crc8_populate_lsb(table, 0xAB);
> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4; word++) {
> u16 val = cpu_to_le16(sprom[word]);
> crc = crc8(table,&val, sizeof(u16), crc);
> }
Maybe not a perfect/optimal solution (crc8 focuses on tables, we do
double loop instead) but should work. Thanks for the tip.
--
Rafał
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib
2011-06-15 18:29 ` Rafał Miłecki
@ 2011-06-15 19:26 ` Arend van Spriel
0 siblings, 0 replies; 5+ messages in thread
From: Arend van Spriel @ 2011-06-15 19:26 UTC (permalink / raw)
To: Rafał Miłecki
Cc: linux-wireless@vger.kernel.org, John W. Linville,
b43-dev@lists.infradead.org
On 06/15/2011 08:29 PM, Rafał Miłecki wrote:
> W dniu 15 czerwca 2011 14:09 użytkownik Arend van Spriel
> <arend@broadcom.com> napisał:
>> On 06/15/2011 01:53 PM, Rafał Miłecki wrote:
>>> static u8 bcma_sprom_crc(const u16 *sprom)
>>> {
>>> - int word;
>>> - u8 crc = 0xFF;
>>> + u8 crc;
>>> + u8 sprom2[SSB_SPROMSIZE_WORDS_R4 * 2 - 1];
>>> + u8 table[CRC8_TABLE_SIZE];
>>> + u16 word;
>>>
>>> + /* u16 to u8 */
>>> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4 - 1; word++) {
>>> - crc = bcma_crc8(crc, sprom[word]& 0x00FF);
>>> - crc = bcma_crc8(crc, (sprom[word]& 0xFF00)>> 8);
>>> + sprom2[word * 2] = sprom[word]& 0x00FF;
>>> + sprom2[(word * 2) + 1] = (sprom[word]& 0xFF00)>> 8;
>>> }
>>> - crc = bcma_crc8(crc, sprom[SSB_SPROMSIZE_WORDS_R4 - 1]& 0x00FF);
>>> + /* 127th byte */
>>> + sprom2[(SSB_SPROMSIZE_WORDS_R4 * 2) - 2] =
>>> + sprom[SSB_SPROMSIZE_WORDS_R4 - 1]&
>>> 0x00FF;
>>> +
>>> + /* Prepare table, 0xAB is x^8 + x^7 + x^6 + x^4 + x^2 + 1 */
>>> + crc8_populate_lsb(table, 0xAB);
>>> +
>>> + /* Calculate */
>>> + crc = crc8(table, sprom2, (SSB_SPROMSIZE_WORDS_R4 * 2) - 1, 0xFF);
>>> crc ^= 0xFF;
>>>
>>> return crc;
>> You could do (I think):
>>
>> crc8_populate_lsb(table, 0xAB);
>> for (word = 0; word< SSB_SPROMSIZE_WORDS_R4; word++) {
>> u16 val = cpu_to_le16(sprom[word]);
>> crc = crc8(table,&val, sizeof(u16), crc);
>> }
> Maybe not a perfect/optimal solution (crc8 focuses on tables, we do
> double loop instead) but should work. Thanks for the tip.
Agree. In brcm80211 we convert the entire sprom, calculate, and convert
it back. Also not perfect I think as it loops over de sprom data twice.
Gr. AvS
--
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-06-15 19:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-15 11:53 [RFC][WORTH IT?][PATCH] bcma: make use of crc8 lib Rafał Miłecki
2011-06-15 12:09 ` Arend van Spriel
2011-06-15 13:59 ` Arend van Spriel
2011-06-15 18:29 ` Rafał Miłecki
2011-06-15 19:26 ` Arend van Spriel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).