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

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

[Luis R. Rodriguez]

[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]


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
	
-- 
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Luis R. Rodriguez]

[-- Attachment #1: Type: text/plain, Size: 1507 bytes --]


BTW we can supply a 2.4 prism54 patch (as new driver). Would that be
wanted?

	Luis

On Mon, May 24, 2004 at 04:30:03AM -0400, 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
> [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
> 	
> -- 
> GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E



-- 
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Jeff Garzik]

Luis R. Rodriguez wrote:
> 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

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

[Jeff Garzik]

Luis R. Rodriguez wrote:
> [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


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

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

[Clint Adams]

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

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

[Jeff Garzik]

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

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

[Jeff Garzik]

Clint Adams wrote:
>>>[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

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

[Andrew Morton]

Jeff Garzik <jgarzik@pobox.com> wrote:
>
>  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.

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

[Jeff Garzik]

Andrew Morton wrote:
> Jeff Garzik <jgarzik@pobox.com> wrote:
> 
>> 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.


> 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

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

[Marcelo Tosatti]

On Thu, May 27, 2004 at 02:10:49PM -0400, Jeff Garzik wrote:
> Luis R. Rodriguez wrote:
> >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?

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

[Jeff Garzik]

Marcelo Tosatti wrote:
> 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

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

[Luis R. Rodriguez]

[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

On Thu, May 27, 2004 at 03:32:29PM -0400, Jeff Garzik wrote:
> Marcelo Tosatti wrote:
> >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 ;-)

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

-- 
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

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

[Jeff Garzik]

Luis R. Rodriguez wrote:
> 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.


> 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

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

[Luis R. Rodriguez]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

On Thu, May 27, 2004 at 03:56:11PM -0400, Jeff Garzik wrote:
> Luis R. Rodriguez wrote:
> >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

> Once the driver is merged, I do request split-up patches just like 2.6.x.

OK. Sure thing.

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

-- 
GnuPG Key fingerprint = 113F B290 C6D2 0251 4D84  A34A 6ADD 4937 E20A 525E

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]