[PATCH] ps3: add vuart support

[Geert Uytterhoeven]

On Thu, 7 Dec 2006, Geoff Levand wrote:
> Arnd Bergmann wrote:
> > On Thursday 07 December 2006 01:17, Geoff Levand wrote:
> >> Adds support for the PS3 virtual UART (vuart).  The vuart provides a
> >> bi-directional byte stream data link between logical partitions.
> > 
> > Looks good now, just a few minor comments:
> > 
> >> +struct ports_bmp
> >> +{
> >> +	u64 status;
> >> +	u64 unused[3];
> >> +} __attribute__ ((packed));
> > 
> > Do you really want to have this structure packed? It will produce
> > better code if it isn't.
> 
> Yes, it is a little obscure in the code, but the address of the ports_bmp
> instance is passed to ps3_alloc_vuart_irq(), which in turn calls
> lv1_configure_virtual_uart_irq().  The HV (actually the vuart service
> in the policy module) uses this as a 256 bit bitmap to indicate the
> interrupt status of the ports.  The system supports 256 ports, but
> currently just two ports, 1 and 3, are used, so I took advantage of
> that to simplify the code and just used the first word of the bitmap.

I think Arnd meant that the `__attribute__ ((packed))' is useless for structs
containing u64 members only. The natural alignment of 64-bit values is on a
64-bit boundary, so there won't be padding anyway.

IIRC, someone (davem?) told me a long time ago gcc _always_ uses byte-accesses
when accessing members in a struct with `__attribute__ ((packed))', since the
members may be unaligned. Even when the values are perfectly aligned. So it's
better not to use `__attribute__ ((packed))' unless it's really needed.

Why is there a comment about `Needs 4 byte alignment.'? The way it's used, it
will always be 8-byte aligned anyway.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- Sony Network and Software Technology Center Europe (NSCE)
Geert.Uytterhoeven at sonycom.com ------- The Corporate Village, Da Vincilaan 7-D1
Voice +32-2-7008453 Fax +32-2-7008622 ---------------- B-1935 Zaventem, Belgium