On Wed, Jul 10, 2013 at 10:48 PM, Paulo Zanoni <przanoni at gmail.com> wrote: > Which means we're now initializing GEN6_PM* code on SNB, which seems > good. You might want to dedicate a paragraph for this on the commit > message. > > On the IRQ patches I wrote (but did not sent yet) I unified the > ILK/SNB irq_handler vfuncs with IVB/HSW ones. I guess bugs like the > one you've just fixed here and in the previous patch are a good way to > justify my patches :) I think we have a language communication fail going on here in these few patches. With "unify" I don't mean "extract identical code, simple refactoring with no functional change but "make them work the same way since currently they don't". Note that with the exception of the gt_irq_mask initialization on vlv there's no bugfix in here: PM interrupte setup simply works differently on snb/vlv than on ivb/hsw after Ben's VECS enabling patches. What these few patches here try to do is unify the sequences again so all platforms set up the PM registers the same way. Note that the PMIER update in gen6_enable_rps isn't a bugfix either: The interrupt handler doesn't touch this register. The only other place is the hsw vecs interrupt setup in the ivybridge irq vfuncs, but those two codepaths can never run in parrallel due to init/teardown sequence ordering (even though that gen6_enable_rps runs from a work item). I've read through the commit message again and for me it seems to be clear what's going on (but I'm obviously biased). So again, do you have ideas for improvements? Wrt patch splitting I'm not sure whether it's worth it, since the current code is simply a bit a confusing mess imo. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
