On Mon, 2006-08-14 at 14:53 +0200, Jan-Bernd Themann wrote: > Michael Ellerman wrote: > >> --- linux-2.6.18-rc4-orig/drivers/net/ehea/ehea.h 1969-12-31 > >> 16:00:00.000000000 -0800 > >> +++ kernel/drivers/net/ehea/ehea.h 2006-08-08 23:59:39.927452928 -0700 > >> + > >> +#define EHEA_PAGESHIFT 12 > >> +#define EHEA_PAGESIZE 4096UL > >> +#define EHEA_CACHE_LINE 128 > > > > This looks like a very bad idea, what happens if you're running on a > > machine with 64K pages? > > > > The EHEA_PAGESIZE define is needed for queue management to hardware side.
You mean the eHEA has its own concept of page size? Separate from the
page size used by the MMU?
> >> +/*
> >> + * h_galpa:
> >> + * for pSeries this is a 64bit memory address where
> >> + * I/O memory is mapped into CPU address space
> >> + */
> >> +
> >> +struct h_galpa {
> >> + u64 fw_handle;
> >> +};
> >
> > What is a h_galpa? And why does it need a struct if it's just a u64?
> >
>
> The eHEA chip is not PCI attached but directly connected to a proprietary
> bus. Currently, we can access it by a simple 64 bit address, but this is not
> true in all cases. Having a struct here allows us to encapsulate the chip
> register access and to respond to changes to system hardware.
>
> We'll change the name to h_epa meaning "ehea physical address"
Hmm, I'm not convinced. Having the struct doesn't really encapsulate
much, because most of the places where you use the h_galpa struct just
pull out the fw_handle anyway. So if you change the layout of the struct
you're going to have to change most of the code anyway. And in the
meantime it makes the code a lot less readable, most people understand
what "u64 addr" is about, whereas "struct h_galpa" is much less
meaningful. </2c>
cheers
--
Michael Ellerman
IBM OzLabs
wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)
We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person
signature.asc
Description: This is a digitally signed message part
