[PATCH 0/3] Fix initialization when iopl is forbidden

Adam Jackson ajax at redhat.com
Wed Jul 18 11:56:30 PDT 2012


On Wed, 2012-07-18 at 19:00 +0100, Simon Farnsworth wrote:

> For the whole series:
> Reviewed-by: Simon Farnsworth <simon.farnsworth at onelan.co.uk>
> 
> I twice thought I'd seen bugs, but a closer look showed me wrong - the huge 
> chain of "#if !defined(<arch>) || !defined(<arch>)" in lnx_video.c being 
> replaced by a smaller chain of "if defined(<arch>) || defined(<arch>)" caught me 
> at first, but appears to be what was actually intended.

Yeah, was trying to turn it into the positive statement of "if we're on
an arch with iopl", instead of the much longer list of arches without.

> The sequence by which xorgHWAccess is changed from meaning "we want access" to 
> "we have access" between InitOutput and the driver probe function wasn't 
> immediately obvious.
> 
> It might be nice to mention in the comment in xf86Init.c where you say 'this 
> is "do we want it" at this point' that xf86BusConfig() will call xf86EnableIO() 
> when xorgHWAccess is TRUE, or move the call of xf86EnableIO() into InitOutput. 
> But that's nit-picking, so feel free to ignore me.

No, that's fair, I'm often too terse in my comments.  Trying to figure
out what the code was doing _before_ the change was complicated enough,
I should at least make it better after I'm done.

Will resend with better commentary, thanks.

- ajax
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg-devel/attachments/20120718/c4dfb977/attachment.pgp>


More information about the xorg-devel mailing list