On Sat, Oct 27, 2012 at 2:56 PM, Paulo Zanoni <przanoni at gmail.com> wrote: > After reading your patch everything looks correct (even though it's > complicated, so a proper testing is better than 1000 reviews here). My > only worry is: do we properly disable all the resources we need to > disable when we fail here? I am assuming you tested this :) I've hoped to document/check the requirements with WARN_ONs. And I've hit them until the code seemed right. So I think I'd be good if you could review those, and if you see any spot that isn't properly checked with WARNs. > My only last bikeshed would be to try to reuse "ret" instead of > "fdi_config_ok" :) I've had that early, but then dropped it again. My fear is that we can't just bail out in the middle of the modeset sequence, which is why I've sanitized the fdi_lanes, and let the entire sequence run through. But at the end we still need to check whether the mode fits and return an error (so that the caller can restore things). Hence a separate error value. btw, I've tested this, and it seems to work as advertised. > With the typos fixed, the double signed-off-by removed, the failure > path properly tested and, optionally, the fdi_config_ok variable > removed. Will do. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
