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

Stephen Neuendorffer stephen.neuendorffer at xilinx.com
Mon Apr 21 13:56:20 EST 2008



> > +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.

> > +enum host_type_t {MMIO, NATIVE, INVALID};
>
> Should these be DCR_HOST_MMIO, DCR_HOST_NATIVE, DCR_HOST_INVALID?
>
> I worry about the generic nature of the names.

Also seems reasonable, 

Steve
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20080420/f7b3bc2f/attachment.htm>


More information about the Linuxppc-dev mailing list