From: George Spelvin <lkml@SDF.ORG>
To: Ajay.Kathat@microchip.com
Cc: Adham.Abozaeid@microchip.com, linux-wireless@vger.kernel.org,
lkml@sdf.org
Subject: Re: [PATCH v2] wilc1000: Use crc7 in lib/ rather than a private copy
Date: Mon, 23 Mar 2020 17:05:16 +0000 [thread overview]
Message-ID: <20200323170516.GB3769@SDF.ORG> (raw)
In-Reply-To: <48611e28-5a55-ab05-3865-71992a5be327@microchip.com>
> 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())?
next prev parent reply other threads:[~2020-03-23 17:05 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-22 12:04 [PATCH] wilc1000: Use crc7 in lib/ rather than a private copy George Spelvin
2020-03-23 5:03 ` Ajay.Kathat
2020-03-23 6:45 ` George Spelvin
2020-03-23 8:22 ` Ajay.Kathat
2020-03-23 14:14 ` [PATCH v2] " George Spelvin
2020-03-23 16:29 ` Ajay.Kathat
2020-03-26 14:54 ` Ajay.Kathat
2020-03-26 14:57 ` Greg KH
2020-03-23 17:05 ` George Spelvin [this message]
2020-03-24 7:19 ` Ajay.Kathat
2020-03-26 19:28 George Spelvin
2020-03-27 7:03 ` Ajay.Kathat
2020-03-27 15:32 ` George Spelvin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200323170516.GB3769@SDF.ORG \
--to=lkml@sdf.org \
--cc=Adham.Abozaeid@microchip.com \
--cc=Ajay.Kathat@microchip.com \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).