Hi Wolfram,
> -----Original Message-----
> From: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> Sent: Sunday, May 19, 2019 7:47 AM
> To: Ajay Gupta <ajaykuee@xxxxxxxxx>
> Cc: heikki.krogerus@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; linux-
> i2c@xxxxxxxxxxxxxxx; Ajay Gupta <ajayg@xxxxxxxxxx>
> 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
