On 27-07-20, 14:19, Andy Shevchenko wrote: > On Mon, Jul 27, 2020 at 1:16 PM Vinod Koul <vkoul@xxxxxxxxxx> wrote: > > On 20-07-20, 17:07, Vaibhav Gupta wrote: > > > Drivers using legacy PM have to manage PCI states and device's PM states > > > themselves. They also need to take care of configuration registers. > > > > > > With improved and powerful support of generic PM, PCI Core takes care of > > > above mentioned, device-independent, jobs. > > > > > > This driver makes use of PCI helper functions like > > > pci_save/restore_state(), pci_enable/disable_device(), > > > and pci_set_power_state() to do required operations. In generic mode, they > > > are no longer needed. > > > > > > Change function parameter in both .suspend() and .resume() to > > > "struct device*" type. Use dev_get_drvdata() to get drv data. > > > > You are doing too many things in One patch. Do consider splitting them > > up to a change per patch. for example using __maybe could be one patch, > > removing code is suspend-resume callbacks would be other one. > > Vinod, while I completely agree with you in general, in this case it > would make more unnecessary churn and will be rather unhelpful in all > ways: for the contributor to do this work, for the reader to collect > all the pieces. It also will be a bisectability issue, because the > #ifdeffery replacement (IIRC you need to move from CONFIG_PM to > CONFIG_PM_SLEEP). I really don't see any advantages of the splitting > here. > > > > Compile-tested only. > > > > I would like to see some testing before we merge this > > I have hardware to test (Intel Minnowboard v1) but have no time. And > taking into account that I did similar changes for many other drivers, > I can give my > Reviewed-by: Andy Shevchenko <andy.shevchenko@xxxxxxxxx> > and take responsibility if somebody complains in the future (I don't > believe it will be one). > > P.S. Another scenario if Vaibhav can find that board (there were > dozens of thousands at least produced and floating on the second hand > market) and test himself. It may be good since he touches the full lot > of PCH (EGT20) drivers. Applied now, thanks -- ~Vinod
