On Wed, 2012-07-18 at 19:00 +0100, Simon Farnsworth wrote: > For the whole series: > Reviewed-by: Simon Farnsworth <[email protected]> > > 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
signature.asc
Description: This is a digitally signed message part
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
