On Tue, Jul 30, 2002 at 11:48:41AM +0200, Joerg Dorchain wrote:
> On Tue, Jul 30, 2002 at 11:38:03AM +1000, David Gibson wrote:
> > >
> > > The patch is appended. You can also find it at
> > > http://dorchain.net/~joerg/code/orinoco_plx-patch_for_pheecom
> > >
> > > If you have more questions about this patch, just ask me.
> >
> > Argh. When you're making patches, for crying out loud, *please* don't
> > make changes unrelated to what you're trying to accomplish. You've
> > changed a whole lot of the indentation in the code, making vast
> > amounts of suprious diff. Don't do that, if you have any wish for the
> > patch to be applied. Or indeed looked at.

> Sorry that this now goes into a style discussion. Maybe you can help me
> by telling what your preferred way is.

See Documentation/CodingStyle in the kernel source.

> The reason for changing identication of many lines is an if clause:
> if ((pdev->vendor != 0x15e8) && (pdev->device != 0x0131)) {
> /* Not the card that needs special handling, so the known working code
> goes here */

Ok, sorry, it's not quite as bad as I thought. But this highlights
part of the point - I missed the extra if statements in your patch
because your indentation doesn't match mine.

> I was thinking that if-block should be idented, even when submitting
> patches. I don't see many chances to deliver equivalent functionality
> without this case distinction.

Yes - but you should match the indentation style of the code you're
changing. In this case 1 tab (8 space) indents.

> I have no idea in what form you like to see this patch. A chance would be
> to move code into a init_plx_chip function, then I could add a
> init_tmd_chip function with less line, but altogether the diff for this
> is even longer.

How similar are the PLX and TMD chips really? Is it worth putting
support into the same module, or should we create a new orinoco_tmd.c
with support for the TMD chips?

-- 
David Gibson			| For every complex problem there is a
davidgibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

• Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]