Re: [PATCH 0/14] prism54: bring up to sync with prism54.org cvs rep

Jeff Garzik
2004-05-27T18:39:43Z

Luis R. Rodriguez wrote:
> Jeff,
>
> Please apply the following patches to linux-2.6.7-rc1. These patches
> bring the kernel tree up to sync with prism54.org's 1.2's release.
>
> [PATCH 1/14 linux-2.6.7-rc1] prism54: add new private ioctls

Change OK once cleanups are moved to separate patch.

> [PATCH 2/14 linux-2.6.7-rc1] prism54: reset card on tx_timeout

OK

> [PATCH 3/14 linux-2.6.7-rc1] prism54: add iwspy support

Change OK, patch rejected due to Lindent (!!!) being included in this
patch as well as functional changes.

> [PATCH 4/14 linux-2.6.7-rc1] prism54: add support for avs header in monitor mode

OK

> [PATCH 5/14 linux-2.6.7-rc1] prism54: new prism54 kernel compatibility

rejected, see Arjan's comments

also, prismcompat24.h doesn't belong in the 2.6 kernel.

> [PATCH 6/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 74, 75

OK

> [PATCH 7/14 linux-2.6.7-rc1] prism54: Fix 2.4 build

rejected, as per comment for patch #5

> [PATCH 8/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 39, 73

cleanups and bug fixes mixed together in same patch.

Change OK once cleanups are moved to separate patch.

> [PATCH 9/14 linux-2.6.7-rc1] prism54: Fix prism54.org bug 77; strengthened oid transaction

More functional changes mixed in with whitespace and formatting cleanups.

Change OK once cleanups are moved to separate patch.

> [PATCH 10/14 linux-2.6.7-rc1] prism54: Don't allow mib reads while unconfigured

Change OK once cleanups are moved to separate patch.

> [PATCH 11/14 linux-2.6.7-rc1] prism54: Touched up kernel compatibility

Rejected due to patch #5 comments.

> [PATCH 12/14 linux-2.6.7-rc1] prism54: Start using likely/unlikely

Use of likely()/unlikely() is OK.

A change to skb_reserve() was snuck into this patch, completely
unrelated to $subject. Also, meaningless cleanups obscure things here too.

> [PATCH 13/14 linux-2.6.7-rc1] prism54: Fix 2.4 SMP build

I agree with this change, but this patch also includes unrelated changes!!

> [PATCH 14/14 linux-2.6.7-rc1] prism54: Fix channel stats; bump to 1.2

OK

Summary: I'm glad you broke up the changes into multiple patches.
Thank you. However, the patches were separated in non-sensical ways.
Follow these guidelines:

1) With this many patches, cosmetic changes (whitespace, formatting,
Lindent) should be in separate patches from functional changes.

2) Fully describe all the changes in the patch. If a patch says "fix
2.4 SMP build", it should do that and nothing else.

3) Kernel compatibility is achieved by coding for the latest kernel
(2.6.x), and then creating back-compat definitions that make the 2.6.x
API (as it's used in your driver) work under earlier kernels. This is
known as the "kcompat" approach. See the kcompat toolkit at
http://sf.net/projects/gkernel/ for examples.

Please resend the patch series with the changes requested. It is OK if
you lump Lindent/whitespace/formatting changes into one big patch, if
you wish -- assuming that patch contains nothing else.

Jeff