| Message ID | 20190517163818.5007-2-ajayg@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: typec: ucsi: ccg: add runtime pm support | expand |
> diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c > index 1c8f708f212b..9d347583f8dc 100644 > --- a/drivers/i2c/busses/i2c-nvidia-gpu.c > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > @@ -175,6 +175,7 @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, > * The controller supports maximum 4 byte read due to known > * limitation of sending STOP after every read. > */ > + pm_runtime_get_sync(i2cd->dev); > for (i = 0; i < num; i++) { > if (msgs[i].flags & I2C_M_RD) { > /* program client address before starting read */ > @@ -189,7 +190,7 @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, > status = gpu_i2c_start(i2cd); > if (status < 0) { > if (i == 0) > - return status; > + goto exit; > goto stop; Hmm, goto here, goto there... OK, the code didn't have a good flow even before this patch. > } > > @@ -206,13 +207,18 @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, > } > status = gpu_i2c_stop(i2cd); > if (status < 0) > - return status; > + goto exit; > > + pm_runtime_mark_last_busy(i2cd->dev); > + pm_runtime_put_autosuspend(i2cd->dev); > return i; > stop: > status2 = gpu_i2c_stop(i2cd); > if (status2 < 0) > dev_err(i2cd->dev, "i2c stop failed %d\n", status2); > +exit: > + pm_runtime_mark_last_busy(i2cd->dev); > + pm_runtime_put_autosuspend(i2cd->dev); > return status; > } I am not nacking this, yet the use of goto here seems too much for my taste. If you could try refactoring the whole code (dunno, maybe using a flag when to use stop?), I'd appreciate this.
Hi Wolfram, > -----Original Message----- > From: Wolfram Sang <wsa@the-dreams.de> > Sent: Sunday, May 19, 2019 7:47 AM > To: Ajay Gupta <ajaykuee@gmail.com> > Cc: heikki.krogerus@linux.intel.com; linux-usb@vger.kernel.org; linux- > i2c@vger.kernel.org; Ajay Gupta <ajayg@nvidia.com> > Subject: Re: [PATCH 1/4] i2c: nvidia-gpu: add runtime pm support > > > diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c > > b/drivers/i2c/busses/i2c-nvidia-gpu.c > > index 1c8f708f212b..9d347583f8dc 100644 > > --- a/drivers/i2c/busses/i2c-nvidia-gpu.c > > +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c > > @@ -175,6 +175,7 @@ static int gpu_i2c_master_xfer(struct i2c_adapter > *adap, > > * The controller supports maximum 4 byte read due to known > > * limitation of sending STOP after every read. > > */ > > + pm_runtime_get_sync(i2cd->dev); > > for (i = 0; i < num; i++) { > > if (msgs[i].flags & I2C_M_RD) { > > /* program client address before starting read */ @@ - > 189,7 +190,7 > > @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, > > status = gpu_i2c_start(i2cd); > > if (status < 0) { > > if (i == 0) > > - return status; > > + goto exit; > > goto stop; > > Hmm, goto here, goto there... OK, the code didn't have a good flow even before > this patch. [...] > > > } > > > > @@ -206,13 +207,18 @@ static int gpu_i2c_master_xfer(struct i2c_adapter > *adap, > > } > > status = gpu_i2c_stop(i2cd); > > if (status < 0) > > - return status; > > + goto exit; > > > > + pm_runtime_mark_last_busy(i2cd->dev); > > + pm_runtime_put_autosuspend(i2cd->dev); > > return i; > > stop: > > status2 = gpu_i2c_stop(i2cd); > > if (status2 < 0) > > dev_err(i2cd->dev, "i2c stop failed %d\n", status2); > > +exit: > > + pm_runtime_mark_last_busy(i2cd->dev); > > + pm_runtime_put_autosuspend(i2cd->dev); > > return status; > > } > > I am not nacking this, yet the use of goto here seems too much for my taste. If > you could try refactoring the whole code (dunno, maybe using a flag when to > use stop?), I'd appreciate this. Ok, I will add a local variable to make it more readable. Thanks Ajay > nvpublic
> > I am not nacking this, yet the use of goto here seems too much for my taste. If > > you could try refactoring the whole code (dunno, maybe using a flag when to > > use stop?), I'd appreciate this. > Ok, I will add a local variable to make it more readable. I was brainstorming here, I am not sure if it will work or not. But you will see. Maybe you get other ideas on the way. However, thanks for trying out!
diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c index 1c8f708f212b..9d347583f8dc 100644 --- a/drivers/i2c/busses/i2c-nvidia-gpu.c +++ b/drivers/i2c/busses/i2c-nvidia-gpu.c @@ -175,6 +175,7 @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, * The controller supports maximum 4 byte read due to known * limitation of sending STOP after every read. */ + pm_runtime_get_sync(i2cd->dev); for (i = 0; i < num; i++) { if (msgs[i].flags & I2C_M_RD) { /* program client address before starting read */ @@ -189,7 +190,7 @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, status = gpu_i2c_start(i2cd); if (status < 0) { if (i == 0) - return status; + goto exit; goto stop; } @@ -206,13 +207,18 @@ static int gpu_i2c_master_xfer(struct i2c_adapter *adap, } status = gpu_i2c_stop(i2cd); if (status < 0) - return status; + goto exit; + pm_runtime_mark_last_busy(i2cd->dev); + pm_runtime_put_autosuspend(i2cd->dev); return i; stop: status2 = gpu_i2c_stop(i2cd); if (status2 < 0) dev_err(i2cd->dev, "i2c stop failed %d\n", status2); +exit: + pm_runtime_mark_last_busy(i2cd->dev); + pm_runtime_put_autosuspend(i2cd->dev); return status; } @@ -332,6 +338,11 @@ static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id) goto del_adapter; } + pm_runtime_set_autosuspend_delay(&pdev->dev, 3000); + pm_runtime_use_autosuspend(&pdev->dev); + pm_runtime_put_autosuspend(&pdev->dev); + pm_runtime_allow(&pdev->dev); + return 0; del_adapter: @@ -345,10 +356,16 @@ static void gpu_i2c_remove(struct pci_dev *pdev) { struct gpu_i2c_dev *i2cd = dev_get_drvdata(&pdev->dev); + pm_runtime_get_noresume(i2cd->dev); i2c_del_adapter(&i2cd->adapter); pci_free_irq_vectors(pdev); } +static int gpu_i2c_suspend(struct device *dev) +{ + return 0; +} + static __maybe_unused int gpu_i2c_resume(struct device *dev) { struct gpu_i2c_dev *i2cd = dev_get_drvdata(dev); @@ -357,7 +374,8 @@ static __maybe_unused int gpu_i2c_resume(struct device *dev) return 0; } -static UNIVERSAL_DEV_PM_OPS(gpu_i2c_driver_pm, NULL, gpu_i2c_resume, NULL); +static UNIVERSAL_DEV_PM_OPS(gpu_i2c_driver_pm, gpu_i2c_suspend, gpu_i2c_resume, + NULL); static struct pci_driver gpu_i2c_driver = { .name = "nvidia-gpu",