Re: [PATCH 4/6] staging: media: atomisp: reformat code comment blocks

[Fabio Aiuto]

On Tue, Apr 20, 2021 at 10:47:29PM +0530, Deepak R Varma wrote:
> On Tue, Apr 20, 2021 at 10:44:48AM +0200, Fabio Aiuto wrote:
> > On Tue, Apr 20, 2021 at 12:45:04AM +0530, Deepak R Varma wrote:
> > > Reformat code comment blocks according to the coding style guidelines.
> > > This resolves different checkpatch script WARNINGs around block comments.
> > > 
> > > Signed-off-by: Deepak R Varma <drv@xxxxxxxxx>
> > > ---
> > >  .../media/atomisp/i2c/atomisp-gc2235.c        |  8 +++----
> > >  .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 ++-
> > >  .../media/atomisp/i2c/atomisp-mt9m114.c       | 22 +++++++++++--------
> > >  .../media/atomisp/i2c/atomisp-ov2680.c        |  3 ++-
> > >  4 files changed, 21 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > index 548c572d3b57..a585d73665a6 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c
> > > @@ -747,10 +747,10 @@ static int startup(struct v4l2_subdev *sd)
> > >  
> > >  	if (is_init == 0) {
> > >  		/* force gc2235 to do a reset in res change, otherwise it
> > > -		* can not output normal after switching res. and it is not
> > > -		* necessary for first time run up after power on, for the sack
> > > -		* of performance
> > > -		*/
> > > +		 * can not output normal after switching res. and it is not
> > > +		 * necessary for first time run up after power on, for the sack
> > > +		 * of performance
> > > +		 */
> > >  		power_down(sd);
> > >  		power_up(sd);
> > >  		gc2235_write_reg_array(client, gc2235_init_settings);
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > > index b93c80471f22..750b3484eb19 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-libmsrlisthelper.c
> > > @@ -57,7 +57,8 @@ static int set_msr_configuration(struct i2c_client *client, uint8_t *bufptr,
> > >  	 * By convention, the first two bytes of actual data should be
> > >  	 * understood as an address in the sensor address space (hibyte
> > >  	 * followed by lobyte) where the remaining data in the sequence
> > > -	 * will be written. */
> > > +	 * will be written.
> > > +	 */
> > >  
> > >  	u8 *ptr = bufptr;
> > >  
> > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > index 465fc4468442..160bb58ce708 100644
> > > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c
> > > @@ -478,7 +478,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
> > >  	/* Note: current modules wire only one GPIO signal (RESET#),
> > >  	 * but the schematic wires up two to the connector.  BIOS
> > >  	 * versions have been unfortunately inconsistent with which
> > > -	 * ACPI index RESET# is on, so hit both */
> > > +	 * ACPI index RESET# is on, so hit both
> > > +	 */
> > >  
> > >  	if (flag) {
> > >  		ret = dev->platform_data->gpio0_ctrl(sd, 0);
> > > @@ -1019,8 +1020,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  		dev->first_gain = AnalogGain;
> > >  		dev->first_diggain = DigitalGain;
> > >  	}
> > > -	/* DigitalGain = 0x400 * (((u16) DigitalGain) >> 8) +
> > > -	((unsigned int)(0x400 * (((u16) DigitalGain) & 0xFF)) >>8); */
> > > +	/* DigitalGain = 0x400 * (((u16) DigitalGain) >> 8) +		*/
> > > +	/* ((unsigned int)(0x400 * (((u16) DigitalGain) & 0xFF)) >>8);	*/
> > >  
> > >  	/* set frame length */
> > >  	if (FLines < coarse_integration + 6)
> > > @@ -1035,7 +1036,8 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  
> > >  	/* set coarse integration */
> > >  	/* 3A provide real exposure time.
> > > -		should not translate to any value here. */
> > > +	 * should not translate to any value here.
> > > +	 */
> > >  	ret = mt9m114_write_reg(client, MISENSOR_16BIT,
> > >  				REG_EXPO_COARSE, (u16)(coarse_integration));
> > >  	if (ret) {
> > > @@ -1044,7 +1046,7 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  	}
> > >  
> > >  	/*
> > > -	// set analog/digital gain
> > > +	 * set analog/digital gain
> > >  	switch(AnalogGain)
> > >  	{
> > >  	case 0:
> > > @@ -1069,8 +1071,9 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,
> > >  	*/
> > >  	if (DigitalGain >= 16 || DigitalGain <= 1)
> > >  		DigitalGain = 1;
> > > -	/* AnalogGainToWrite =
> > > -		(u16)((DigitalGain << 12) | AnalogGainToWrite); */
> > > +
> > > +	/* AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite);
> > > +	 */
> > 
> > this is best recommended for one line comment:
> > 
> > /* AnalogGainToWrite = (u16)((DigitalGain << 12) | AnalogGainToWrite); */
> 
> It then extends beyond 80 character in length. Will that be acceptable?
> 

if the intention is to stay within 80 characters in length then it's better

	/*
	 * AnalogGainToWrite = (u16)(DigitalGain << 12) | AnalogGainToWrite);
	 */

thank you,

fabio