[PATCH][2.6] IBM PowerPC Virtual Ethernet Driver
Santiago Leon
newer IBM iSeries and pSeries systems:
The patch applies against 2.6.3-rc3...
Jeff, can you formally add this driver to 2.6?... The differences
between this driver and the 2.4 driver that you accepted are fairly
trivial (i.e. workqueues instead of tasklets)... The architectural
additions that I was waiting for have been applied to the mainline tree...
Comments and suggestions are always welcome...
--
Santiago A. Leon
Power Linux Development
IBM Linux Technology Center
Benjamin Herrenschmidt
> Here's a patch that adds the inter-partition Virtual Ethernet driver for
> newer IBM iSeries and pSeries systems:
>
> The patch applies against 2.6.3-rc3...
>
> Jeff, can you formally add this driver to 2.6?... The differences
> between this driver and the 2.4 driver that you accepted are fairly
> trivial (i.e. workqueues instead of tasklets)... The architectural
> additions that I was waiting for have been applied to the mainline tree...
>
> Comments and suggestions are always welcome...
Let me put my though police hat for a short while...
BITFIELDS ARE EVIL !!!
Especially when mapping things like HW registers... I know the p/iSeries
code is full of them, I'd strongly recommend getting rid of them.
The compiler is perfectly free, afaik, to re-order them, you have no
guarantee of your actual layout, and it's not portable (I know nothing
but PPC uses those drivers _right_ now, but still...)
So please, don't add more ;)
Ben.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Dave Jones
> BITFIELDS ARE EVIL !!!
>
> Especially when mapping things like HW registers... I know the p/iSeries
> code is full of them, I'd strongly recommend getting rid of them.
>
> The compiler is perfectly free, afaik, to re-order them
That can't be right surely ? That would make them utterly useless afaics.
I've not seen this happen in practice either with the 2 x86 cpufreq drivers
I wrote that both use bitfields extensively.
Dave
Keith Owens
Benjamin Herrenschmidt <be...@kernel.crashing.org> wrote:
>BITFIELDS ARE EVIL !!!
Not it is not. C standard (ISO/IEC 9899:1999 (E)), section 6.7.2.1.
10 ... If enough space remains, a bit-field that immediately follows
another bit-field in a structure shall be packed into adjacent bits
of the same unit. ...
13 Within a structure object, the non-bit-field members and the units
in which bit-fields reside have addresses that increase in the order
in which they are declared. ...
There is no scope for a compiler to reorder the members or the bit
fields of a structure.
vi...@parcelfarce.linux.theplanet.co.uk
> On Wed, Feb 18, 2004 at 11:45:20AM +1100, Benjamin Herrenschmidt wrote:
>
> > BITFIELDS ARE EVIL !!!
> >
> > Especially when mapping things like HW registers... I know the p/iSeries
> > code is full of them, I'd strongly recommend getting rid of them.
> >
> > The compiler is perfectly free, afaik, to re-order them
>
> That can't be right surely ? That would make them utterly useless afaics.
> I've not seen this happen in practice either with the 2 x86 cpufreq drivers
> I wrote that both use bitfields extensively.
It _is_ right and they are utterly useless. Original rationale was, indeed,
"describe the layout of hardware registers" but it had gone to hell may years
ago. Any assumptions regarding their allocation are non-portable.
Andrew Morton
>
> On Wed, 18 Feb 2004 11:45:20 +1100,
> Benjamin Herrenschmidt <be...@kernel.crashing.org> wrote:
> >BITFIELDS ARE EVIL !!!
> >The compiler is perfectly free, afaik, to re-order them
>
> Not it is not. C standard (ISO/IEC 9899:1999 (E)), section 6.7.2.1.
>
> 10 ... If enough space remains, a bit-field that immediately follows
> another bit-field in a structure shall be packed into adjacent bits
> of the same unit. ...
>
> 13 Within a structure object, the non-bit-field members and the units
> in which bit-fields reside have addresses that increase in the order
> in which they are declared. ...
>
> There is no scope for a compiler to reorder the members or the bit
> fields of a structure.
Problem is portability between different architectures, especially little
endian versus big endian. The 3c59x driver used to use a bitfield to
represent hardware registers and had some lovely ifdeffery to make it work
on ppc hardware.
It's safe enough in arch-specific code but one may as well use the same
idioms as are used in generic code.
Linus Torvalds
On Wed, 18 Feb 2004 vi...@parcelfarce.linux.theplanet.co.uk wrote:
> >
> > That can't be right surely ? That would make them utterly useless afaics.
> > I've not seen this happen in practice either with the 2 x86 cpufreq drivers
> > I wrote that both use bitfields extensively.
>
> It _is_ right and they are utterly useless. Original rationale was, indeed,
> "describe the layout of hardware registers" but it had gone to hell may years
> ago. Any assumptions regarding their allocation are non-portable.
Well, to be fair, most compilers still aim to make them useful.
There's a difference between "the standard doesn't guarantee anything" and
"the implementation makes no sense".
(Sadly, a lot of compiler people do seem to look to standards more than
actual users for guides to do things, but at the same time I do believe
that gcc has useful semantics for bitfields and hardware accesses. You
just have to know what the implementation-specific rules are)
Linus
Linus Torvalds
On Wed, 18 Feb 2004, Keith Owens wrote:
>
> There is no scope for a compiler to reorder the members or the bit
> fields of a structure.
The actual bit ordering is implementation-defined, though, so in effect
the bitfield setup _does_ depend on the compiler and architecture details.
Linus
Benjamin Herrenschmidt
> There's a difference between "the standard doesn't guarantee anything" and
> "the implementation makes no sense".
>
> (Sadly, a lot of compiler people do seem to look to standards more than
> actual users for guides to do things, but at the same time I do believe
> that gcc has useful semantics for bitfields and hardware accesses. You
> just have to know what the implementation-specific rules are)
Well... I still think it's asking for trouble in the long term
to rely on them .... It's definitely broken for drivers of devices
that can be used on different archs (PCI cards for example) since
the layout of the bitfields is different at least with gcc between
big and little endian machines. But I've also been bitten by
alignement issues within the bitfield in the past (not with gcc
though) and other funny things like that...
So overall, I just recommend to get rid of them.
Ben.
vi...@parcelfarce.linux.theplanet.co.uk
> > ago. Any assumptions regarding their allocation are non-portable.
>
> Well, to be fair, most compilers still aim to make them useful.
>
> There's a difference between "the standard doesn't guarantee anything" and
> "the implementation makes no sense".
>
> (Sadly, a lot of compiler people do seem to look to standards more than
> actual users for guides to do things, but at the same time I do believe
> that gcc has useful semantics for bitfields and hardware accesses. You
> just have to know what the implementation-specific rules are)
True. However, most of the bitfield uses I've seen would become much cleaner
from rewrite to explicit mask-and-shift stuff.
Linus Torvalds
On Wed, 18 Feb 2004, Benjamin Herrenschmidt wrote:
>
> Well... I still think it's asking for trouble in the long term
> to rely on them .... It's definitely broken for drivers of devices
> that can be used on different archs (PCI cards for example) since
> the layout of the bitfields is different at least with gcc between
> big and little endian machines. But I've also been bitten by
> alignement issues within the bitfield in the past (not with gcc
> though) and other funny things like that...
I mostly agree.
You _can_ get bitfields correct, but it takes some care. And btw, bit
endianness within a word does not necessarily match the byte endianness,
although they are usually connected.
See for example <linux/tcp.h> for what you need to do to make bitfields
work right across architectures.
> So overall, I just recommend to get rid of them.
In general, yes. You can make bitfields be more readable, but you need to
be careful. And we all know how careful most driver writers tend to be.
Linus
Santiago Leon
>
> BITFIELDS ARE EVIL !!!
He he...
Thanks for the comments and discussion... I changed the bitfields to
masks and shifts and I'm attaching the full patch (not just the bitfield
changes)...
Jeff, If everything else looks ok, can you add it to 2.6?... I'll send
you a patch that fixes the bitfields for the 2.4 version soon...
Pavel Machek
> Here's a patch that adds the inter-partition Virtual Ethernet driver for
> newer IBM iSeries and pSeries systems:
>
> The patch applies against 2.6.3-rc3...
>
> Jeff, can you formally add this driver to 2.6?... The differences
> between this driver and the 2.4 driver that you accepted are fairly
> trivial (i.e. workqueues instead of tasklets)... The architectural
> additions that I was waiting for have been applied to the mainline tree...
>
> Comments and suggestions are always welcome...
Could we get
..less asciiart:
/*
*
*/
instead of
/**************
* *
**************/
and shorter identifiers so it fits on, say 100 columns?
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]