prism54: bring up to sync with prism54.org cvs rep
Subsystems (2)
cover prism54: bring up to sync with prism54.org cvs rep
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
[PATCH 2/14 linux-2.6.7-rc1] prism54: reset card on tx_timeout
[PATCH 3/14 linux-2.6.7-rc1] prism54: add iwspy support
[PATCH 4/14 linux-2.6.7-rc1] prism54: add support for avs header in monitor mode [PATCH 5/14 linux-2.6.7-rc1] prism54: new prism54 kernel compatibility [PATCH 6/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 74, 75
[PATCH 7/14 linux-2.6.7-rc1] prism54: Fix 2.4 build
[PATCH 8/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 39, 73
[PATCH 9/14 linux-2.6.7-rc1] prism54: Fix prism54.org bug 77; strengthened oid transaction [PATCH 10/14 linux-2.6.7-rc1] prism54: Don't allow mib reads while unconfigured [PATCH 11/14 linux-2.6.7-rc1] prism54: Touched up kernel compatibility [PATCH 12/14 linux-2.6.7-rc1] prism54: Start using likely/unlikely
[PATCH 13/14 linux-2.6.7-rc1] prism54: Fix 2.4 SMP build
[PATCH 14/14 linux-2.6.7-rc1] prism54: Fix channel stats; bump to 1.2
Luis
Signature · 1 line
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84 A34A 6ADD 4937 E20A 525E
BTW we can supply a 2.4 prism54 patch (as new driver). Would that be wanted?
Luis
Quoted 25 lines · Luis R. Rodriguez
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
[PATCH 2/14 linux-2.6.7-rc1] prism54: reset card ontx_timeout
[PATCH 3/14 linux-2.6.7-rc1] prism54: add iwspy support
[PATCH 4/14 linux-2.6.7-rc1] prism54: add support for avs header in monitor mode [PATCH 5/14 linux-2.6.7-rc1] prism54: new prism54 kernel compatibility [PATCH 6/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 74, 75
[PATCH 7/14 linux-2.6.7-rc1] prism54: Fix 2.4 build
[PATCH 8/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 39, 73
[PATCH 9/14 linux-2.6.7-rc1] prism54: Fix prism54.org bug 77; strengthened oid transaction [PATCH 10/14 linux-2.6.7-rc1] prism54: Don't allow mib reads while unconfigured [PATCH 11/14 linux-2.6.7-rc1] prism54: Touched up kernel compatibility [PATCH 12/14 linux-2.6.7-rc1] prism54: Start using likely/unlikely
[PATCH 13/14 linux-2.6.7-rc1] prism54: Fix 2.4 SMP build
[PATCH 14/14 linux-2.6.7-rc1] prism54: Fix channel stats; bump to 1.2Luis
Signature · 1 line
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84 A34A 6ADD 4937 E20A 525E
Signature · 1 line
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84 A34A 6ADD 4937 E20A 525E
Quoted 2 lines · Luis R. Rodriguez
BTW we can supply a 2.4 prism54 patch (as new driver). Would that be wanted?
No objections here.
2.4.x is vaguely closed, though, so Marcelo may not be interested in new drivers.
Jeff
Quoted 8 lines · Jeff Garzik
Quoted 2 lines · Luis R. Rodriguez
BTW we can supply a 2.4 prism54 patch (as new driver). Would that be wanted?
No objections here.
2.4.x is vaguely closed, though, so Marcelo may not be interested in new drivers.
IMO support for new hardware (new drivers) which dont break existing setups are fine.
Jeff, you are actively maintaining most of the network drivers in v2.4, if you are OK with the inclusion of this, I'm OK.
Does this make sense?
Quoted 5 lines · Marcelo Tosatti
IMO support for new hardware (new drivers) which dont break existing setups are fine.
Jeff, you are actively maintaining most of the network drivers in v2.4, if you are OK with the inclusion of this, I'm OK.
Does this make sense?
Yup, makes sense and sounds good to me.
Luis, feel free to send me the 2.4.x version of the prism54 driver -- after addressing my comments, of course ;-)
Jeff
Quoted 7 lines · Jeff Garzik
Quoted 6 lines · Marcelo Tosatti
IMO support for new hardware (new drivers) which dont break existing setups are fine.
Jeff, you are actively maintaining most of the network drivers in v2.4, if you are OK with the inclusion of this, I'm OK.
Quoted 7 lines
Quoted 1 line
Does this make sense?
Yup, makes sense and sounds good to me.
Luis, feel free to send me the 2.4.x version of the prism54 driver -- after addressing my comments, of course ;-)
Great. I'll fix all those patches up again for you and Marcelo. For future development I've asked other prism54 developers to first send patches to netdev for review/approval. Maybe we *should* do away with prism54-devel mailing list and just use netdev as was once suggested by someone...
Luis
Signature · 1 line
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84 A34A 6ADD 4937 E20A 525E
Quoted 2 lines · Luis R. Rodriguez
Great. I'll fix all those patches up again for you and Marcelo. For future development I've asked other prism54 developers to first send patches to
For what it's worth, you may send a "one big patch" to add the driver to 2.4, since the individual changes to that point will be in 2.6.x BitKeeper changelog.
Once the driver is merged, I do request split-up patches just like 2.6.x.
Quoted 1 line
netdev for review/approval. Maybe we *should* do away with prism54-devel mailing list and just use netdev as was once suggested by someone...
Up to you :)
Jeff
Quoted 7 lines · Jeff Garzik
Quoted 2 lines · Luis R. Rodriguez
Great. I'll fix all those patches up again for you and Marcelo. For future development I've asked other prism54 developers to first send patches to
For what it's worth, you may send a "one big patch" to add the driver to 2.4, since the individual changes to that point will be in 2.6.x BitKeeper changelog.
Will do. My plan was for us to get the pathes for 2.6 all set. Once we were all happy and our trees in sync we'd send a large 2.4 patch for inclusion in 2.4
Quoted 1 line
Once the driver is merged, I do request split-up patches just like 2.6.x.
OK. Sure thing.
Quoted 4 lines
Quoted 2 lines
netdev for review/approval. Maybe we *should* do away with prism54-devel mailing list and just use netdev as was once suggested by someone...
Up to you :)
I think we should. Anyone against this? Please reply or else we'll just move to netdev.
Luis
Signature · 1 line
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84 A34A 6ADD 4937 E20A 525E
Quoted 6 lines · Luis R. Rodriguez
[PATCH 1/14 linux-2.6.7-rc1] prism54: add new private ioctls
[PATCH 2/14 linux-2.6.7-rc1] prism54: reset card ontx_timeout[PATCH 3/14 linux-2.6.7-rc1] prism54: add iwspy support
[PATCH 4/14 linux-2.6.7-rc1] prism54: add support for avs header in monitor mode [PATCH 5/14 linux-2.6.7-rc1] prism54: new prism54 kernel compatibility [PATCH 6/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 74, 75
Quoted 8 lines
[PATCH 7/14 linux-2.6.7-rc1] prism54: Fix 2.4 build
[PATCH 8/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 39, 73
[PATCH 9/14 linux-2.6.7-rc1] prism54: Fix prism54.org bug 77; strengthened oid transaction [PATCH 10/14 linux-2.6.7-rc1] prism54: Don't allow mib reads while unconfigured [PATCH 11/14 linux-2.6.7-rc1] prism54: Touched up kernel compatibility [PATCH 12/14 linux-2.6.7-rc1] prism54: Start using likely/unlikely
[PATCH 13/14 linux-2.6.7-rc1] prism54: Fix 2.4 SMP build
[PATCH 14/14 linux-2.6.7-rc1] prism54: Fix channel stats; bump to 1.2
I'm reviewing these now.
I am worried though -- the first couple patches include a TON of whitespace changes and cleanups unrelated to the patch at hand. This makes it MUCH more difficult to review each patch.
I'm considering rejecting the entire series because of this obfuscation of changes, and getting you to resend with the whitespace crapola separated out.
Jeff
Quoted 5 lines
Quoted 1 line
[PATCH 8/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 39, 73
I'm considering rejecting the entire series because of this obfuscation of changes, and getting you to resend with the whitespace crapola separated out.
Please at least apply the changes in the "8/14" patch, because without them the driver is unusable on big-endian architectures.
Quoted 8 lines · Clint Adams
Quoted 4 lines
Quoted 1 line
[PATCH 8/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 39, 73
I'm considering rejecting the entire series because of this obfuscation of changes, and getting you to resend with the whitespace crapola separated out.
Please at least apply the changes in the "8/14" patch, because without them the driver is unusable on big-endian architectures.
The entire patch series was dropped.
Luis, you, or somebody should create a new patch series with just the critical fixes, NO WHITESPACE/FORMATTING CHANGES mixed in, and send those first.
Jeff
Quoted 4 lines · Jeff Garzik
Luis, you, or somebody should create a new patch series with just the critical fixes, NO WHITESPACE/FORMATTING CHANGES mixed in, and send those first.
Whitespace changes are often nice, but they should be the very first patch[es] in the series. You should be able to verify that the .o file was unchanged before and after.
That way they become a no-brainer and it becomes easier to review and understand the substantive changes.
Quoted 8 lines · Andrew Morton
Quoted 1 line · Jeff Garzik
Luis, you, or somebody should create a new patch series with just the critical fixes, NO WHITESPACE/FORMATTING CHANGES mixed in, and send those first.
Whitespace changes are often nice, but they should be the very first patch[es] in the series. You should be able to verify that the .o file was unchanged before and after.
Very first, or very last. I leave that up to the maintainer.
Quoted 2 lines
That way they become a no-brainer and it becomes easier to review and understand the substantive changes.
Agreed.
Further, when someone mixes an Lindent in with functional changes, I become very suspicious. That is precisely the method that certain high profile Linux hackers have used in the past to intentionally obfuscate security changes.
Jeff
Quoted 6 lines · Luis R. Rodriguez
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.
Quoted 1 line
[PATCH 2/14 linux-2.6.7-rc1] prism54: reset card on
tx_timeout
OK
Quoted 1 line
[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.
Quoted 1 line
[PATCH 4/14 linux-2.6.7-rc1] prism54: add support for avs header in monitor mode
OK
Quoted 1 line
[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.
Quoted 1 line
[PATCH 6/14 linux-2.6.7-rc1] prism54: Fix prism54.org bugs 74, 75
OK
Quoted 1 line
[PATCH 7/14 linux-2.6.7-rc1] prism54: Fix 2.4 build
rejected, as per comment for patch #5
Quoted 1 line
[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.
Quoted 1 line
[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.
Quoted 1 line
[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.
Quoted 1 line
[PATCH 11/14 linux-2.6.7-rc1] prism54: Touched up kernel compatibility
Rejected due to patch #5 comments.
Quoted 1 line
[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.
Quoted 1 line
[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!!
Quoted 1 line
[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