linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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())?

  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).