Hi Wolfram, > -----Original Message----- > From: linux-usb-owner@xxxxxxxxxxxxxxx <linux-usb-owner@xxxxxxxxxxxxxxx> > On Behalf Of Wolfram Sang > Sent: Saturday, May 25, 2019 12:57 PM > To: Ajay Gupta <ajaykuee@xxxxxxxxx> > Cc: heikki.krogerus@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; linux- > i2c@xxxxxxxxxxxxxxx; Ajay Gupta <ajayg@xxxxxxxxxx> > Subject: Re: [PATCH v3 2/5] i2c: nvidia-gpu: add runtime pm support > > > > @@ -211,6 +212,8 @@ static int gpu_i2c_master_xfer(struct i2c_adapter > *adap, > > goto exit; > > } > > > > + pm_runtime_mark_last_busy(i2cd->dev); > > + pm_runtime_put_autosuspend(i2cd->dev); > > return i; > > exit: > > if (send_stop) { > > @@ -218,6 +221,8 @@ static int gpu_i2c_master_xfer(struct i2c_adapter > *adap, > > if (status2 < 0) > > dev_err(i2cd->dev, "i2c stop failed %d\n", status2); > > } > > + pm_runtime_mark_last_busy(i2cd->dev); > > + pm_runtime_put_autosuspend(i2cd->dev); > > Maybe another worthwhile refactorization possible here? The exit code path > and 'all good' code path look very similar. > This can be done incrementally, though. I think for now it is okay. Thanks for suggestion. Yes , it is possible to refactor a bit further here. > > +static __maybe_unused int gpu_i2c_suspend(struct device *dev) { > > + return 0; > > +} > > Why do we need this? I2C function of PCI bus remains in D0 (lspci output) without this function. Following document also seems to insist on this. " For this to work, the device's driver has to provide a pm->runtime_suspend() callback " Documentation/power/pci.txt "First, a PCI device is put into a low-power state, or suspended, with the help of pm_schedule_suspend() or pm_runtime_suspend() which for PCI devices call pci_pm_runtime_suspend() to do the actual job. For this to work, the device's driver has to provide a pm->runtime_suspend() callback (see below), which is run by pci_pm_runtime_suspend() as the first action. If the driver's callback returns successfully, the device's standard configuration registers are saved, the device is prepared to generate wakeup signals and, finally, it is put into the target low-power state." Thanks Ajay > nvpublic