RE: [PATCH v3 2/5] i2c: nvidia-gpu: add runtime pm support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux