[PATCH 1/2] [v3][POWERPC] refactor dcr code

[Josh Boyer]

On Sun, 20 Apr 2008 20:56:20 -0700
"Stephen Neuendorffer" <stephen.neuendorffer at xilinx.com> wrote:

> 
> 
> > > +void dcr_unmap_generic(dcr_host_t host, unsigned int dcr_c)
> > > +{
> > > +	if (host.type == NATIVE)
> > > +		dcr_unmap_native(host.host.native, dcr_c);
> > > +	else
> > > +		dcr_unmap_mmio(host.host.mmio, dcr_c);
> >
> > What happens if host.type == INVALID?  Same question for the other
> > accessors in dcr_*_generic.
> 
> I guess looking back on it, I assumed that MAP_OK would return 0, meaning that behavior was undefined,
> but I agree it's probably safer to have some error reporting there...  There starts to become a speed tradeoff
> at some point, which would make function pointers more attractive.  If the ioremap does fail, or the
> dcr-access-method can't be determined, then dcr_unmap_mmio would probably SEGV anyway, although that's
> not something I'd really want to rely on.  I'll put an error case in there.

Well, MAP_OK would return 0, and the caller of the original dcr_map
should probably return an error or something.  But there's nothing to
enforce that.  Which is true for the current code today as well, so
it's not something you've introduced.

I just thought that if you were going to go to the trouble of defining
an invalid host type, that you'd check for it in the accessor
functions.  There is the concern of the speed tradeoffs.  I suppose you
could just omit INVALID altogether and have it match the existing code
in behavior if it was too big of a deal.

Ben, any thoughts?

josh