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
Attachment:
signature.asc
Description: This is a digitally signed message part
- Follow-Ups:
- Re: [PATCH 4/6] ehea: header files
- From: Jan-Bernd Themann <[email protected]>
- Re: [PATCH 4/6] ehea: header files
- References:
- [PATCH 4/6] ehea: header files
- From: Jan-Bernd Themann <[email protected]>
- Re: [PATCH 4/6] ehea: header files
- From: Michael Ellerman <[email protected]>
- Re: [PATCH 4/6] ehea: header files
- From: Jan-Bernd Themann <[email protected]>
- [PATCH 4/6] ehea: header files
- Prev by Date: drivers/usb/misc/cypress_cy7c63.c: NULL dereference
- Next by Date: Re: [PATCHv3] sunrpc/auth_gss: NULL pointer deref in gss_pipe_release()
- Previous by thread: Re: [PATCH 4/6] ehea: header files
- Next by thread: Re: [PATCH 4/6] ehea: header files
- Index(es):
 |