[parisc-linux] [PATCH] PDC cleanup and encapsulation

[Ryan Bradetich]

On Wed, Apr 04, 2001 at 09:57:25AM -0700, Bjorn Helgaas wrote:
> Looks good to me.  A couple minor questions/comments, more for my 
> edification than anything else:
> 
> - pdc.h contains a commented-out struct pdc_iodc.  The comment suggests 
> possible issues with using it with 64-bit firmware, but I think the fear 
> is unfounded.  There are several places that use pdc_iodc_read(), but they 
> all seem to define their own equivalent of struct pdc_iodc.  I'm not sure 
> why they don't use struct pdc_iodc.  Also, include/asm-parisc/hardware.h 
> contains #defines for the IODC_TYPE field (HPHW_NPROC, etc); should these 
> be defined alongside struct pdc_iodc?

>From reading the on-line docs and looking at how it pdc_iodc_read was 
used, I decided to let the user allocate the buffer instead of using
the predefinded struct.  The on-line docs indicated to me that only
the first 16 bytes are standard, after that the call was HVERSION dependent.
Since I could not determine what to use in all cases, I left it upto 
the user.  If it makes more sense to use a predefined struct for
better typecasting, etc ... I would be agreeable, I just didn't want to
break anything. :)

> - What are the guidelines for #ifdef __LP64__ vs CONFIG_PA20?  You 
> replaced __LP64__ with CONFIG_PA20 in some places but not others and I'm 
> not clear on the difference.

The one instance I changed this was for the block TLB pdc calls. These
calls are not defined in the 2.0 arch so I changed the define to make
the more obvious.  [Note: it also helped solve some compiling issues.]

> - In firmware.c, the "yes 'int', not 'long' -- IODC I/O is always 32-bit 
> stuff" comment moved from pdc.h seems obsolete, or at least, I can't 
> figure out what the 'int' refers to.  I'd remove the "This means Cxxx 
> boxes can't run wide kernels right now." comment, since CONFIG_PDC_NARROW 
> addresses that issue.

This is true.... actually the Cxxx boxes won't run the 64-bit kernel now,
but it isn't a firmware issue, it is a driver issue.  I'm working on this
and hopefully will have it solved soon.  But, your point is well taken,
I should have removed the comment.  Nice catch :)

> I attached a diff that fixes a few typos.

Thanks and much appreciated!

- Ryan

> Bjorn