On Wed, Oct 29, 2008 at 12:18:47PM -0700, David Daney wrote:
> That file contains the bit definitions for all on-chip registers.
>
> We are interested in transforming this information into a form suitable
> for inclusion in the kernel. Any specific suggestions as to improve the
> patch will be considered.
>
> Several possibilities are:
>
> 1) Don't typedef all the unions in cvmx-csr-typedefs.h. An rename the
> file so it doesn't contain the reprehensible word 'typedef'
>
> 2) Break cvmx-csr-addresses.h and cvmx-csr-typedefs.h into several
> parts, one for each functional block in the processor.
>
> There are obviously other options as well...
The use of bitfields also fires back:
+ struct cvmx_ciu_intx_en1_w1c_cn58xx {
+#if __BYTE_ORDER == __BIG_ENDIAN
+ uint64_t reserved_16_63:48;
+ uint64_t wdog:16;
+#else
+ uint64_t wdog:16;
+ uint64_t reserved_16_63:48;
+#endif
You see, everything was defined twice. And gcc even recent gccs tend to
do silly stuff with bitfields when combined with volatile:
struct foo {
unsigned int x:1;
unsigned int y:4;
};
void bar(volatile struct foo *p)
{
p->x = 1;
p->y++;
}
which gcc 4.3 for a MIPS32R2 target will compile into:
lw $3, 0($4)
li $2, 1
ins $3, $2, 31, 1
sw $3, 0($4)
lw $2, 0($4)
lw $3, 0($4)
ext $2, $2, 27, 4
addiu $2, $2, 1
ins $3, $2, 27, 4
sw $3, 0($4)
j $31
nop
Imagine struct foo was describing a hardware register so the pointer to it
was marked volatile. A human coder wouldn't have done multiple loads /
stores. Worse, if you actually want to change multiple fields in a
register atomically then with bitfields you have _no_ possibility to enforce
that.
The Linux programming programming model relies on accessor functions like
readl, ioread32 etc. Those take addresses as arguments - but bitfields
don't have addresses in C ...
So exec summary: bitfields bad for such low-level stuff.
Ralf
