[parisc-linux-cvs] [PATCH] 32-bit PDC wrapper patch

[Paul Bame]

= On Tue, Mar 13, 2001 at 06:41:31PM -0700, Ryan Bradetich wrote:
= > Here is the patch.
= 
= I've never liked the PDC interface.

Me neither.  Nor do I particularly enjoy our C wrappers around PDC
even though I rewrote them last.

= Too much magic, unnecessary
= use of varargs... ick.  This only makes it worse :-)

Some clues in addition to loaded terms like "too much", "magic",
"unnecessary" would be helpful guidance for someone to re-design
our PDC interface.

IMO, the use of varargs for PDC is almost as sensible as using it for
printk.  The first two PDC args tell PDC how to interpret the
rest of the args, much as the prink format string does.  Varargs
leads to rather compact code (both C and object) which
is amenable to doing algorithmic translations of its arguments when
needed (and it's needed for the 64 on 32 case).  It see no compelling
reason to replicate for each number of arguments passable to a PDC call.

Unfortunately ugliness and magic are absolutely required at some level
of our code, because calling the firmware is unavoidably complex.  IMO
(but then I wrote it) we are at a decent compromise where most of the
ugliness (and "magic": constructing a stack frame by hand in C!) is
hidden under mem_pdc_call() [I wish the alignment and phys/virt issues
were hidden there too alas] Not that it's necessarily a good example, but
hp-ux settled on a similar design.

Or, to take this to logical extreme, if we had mem_pdc_call3()
through mem_pdc_call_11() as separate calls, I could make the argument
that we should also have mem_pdc_call3i(int p1, int p2, long arg1)
plus mem_pdc_call3p(int p1, int p2, void *arg1) plus
mem_pdc_call3r(int p1, int p2, RADDR *arg1) so that we
could dispense further with the "magic" of passing everything as
unsigned longs and gain stronger type checking.  But
at this point the explosion of mem_pdc_call*()s gets so large that
it makes sense to only craft the ones you need, defined
with interfaces containing correct number of arguments and types,
which is roughly what's in pdc.c today.

A criticism of mem_pdc_call() which I consciously own is that it is
effectively one function which takes a fixed large number of
arguments all of which are passed to the firmware: mem_pdc_call(a,b,c,d,e...).
Unfortunately actually defining it that way causes the C compiler to choke
when people pass fewer args (yes macros could fix that, but to what end?),
and omitting the extern definition causes the C compiler to omit warnings
about calling an undefined function, so varargs is the compromise I made
at the time.  The penalty for passing unused args to PDC is minor since
PDC calls aren't part of a performance path.

= Here's a proposed (I hope) simplification.  feedback welcome, as ever.
= > +	return mem_pdc_call(PDC_ARG2_IS_NOT_RADDR, PDC_CHASSIS, PDC_CHASSIS_DIS
P, disp);
= 	return mem_pdc_call3(PDC_CHASSIS, PDC_CHASSIS_DISP, disp);

mem_pdc_call3() would still have to know whether or not to translate ARG2
as an RADDR.  This suggested design change may have significant
ramifications to real1.c and possibly real2.S.

	-P