RE: [PATCH 1/4] i2c: nvidia-gpu: add runtime pm support

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

 



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






[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux