mbox series

[0/6] staging: media: atomisp: code cleanup fixes

Message ID cover.1618859059.git.drv@mailo.com
Headers show
Series staging: media: atomisp: code cleanup fixes | expand

Message

Deepak R Varma April 19, 2021, 7:11 p.m. UTC 
This patch set addresses different kinds of checkpatch WARNING and
CHECK complaints. 

Note: The patches should be applied in the ascending order.

Deepak R Varma (6):
  staging: media: atomisp: improve function argument alignment
  staging: media: atomisp: balance braces around if...else block
  staging: media: atomisp: use __func__ over function names
  staging: media: atomisp: reformat code comment blocks
  staging: media: atomisp: fix CamelCase variable naming
  staging: media: atomisp: use printk with KERN facility level

 .../media/atomisp/i2c/atomisp-gc0310.c        | 12 +--
 .../media/atomisp/i2c/atomisp-gc2235.c        | 18 ++--
 .../atomisp/i2c/atomisp-libmsrlisthelper.c    |  3 +-
 .../media/atomisp/i2c/atomisp-lm3554.c        |  2 +-
 .../media/atomisp/i2c/atomisp-mt9m114.c       | 82 ++++++++++---------
 .../media/atomisp/i2c/atomisp-ov2680.c        | 26 +++---
 .../media/atomisp/i2c/atomisp-ov2722.c        | 10 +--
 7 files changed, 80 insertions(+), 73 deletions(-)

Comments

Fabio Aiuto April 20, 2021, 8:39 a.m. UTC | #1 
Hi,

On Tue, Apr 20, 2021 at 12:45:57AM +0530, Deepak R Varma wrote:
> Mixed case variable names are discouraged and they result in checkpatch

> script "Avoid CamelCase" warnings. Replace such CamelCase variable names

> by lower case strings according to the coding style guidelines.

> 

> Signed-off-by: Deepak R Varma <drv@mailo.com>

> ---

>  .../media/atomisp/i2c/atomisp-mt9m114.c       | 62 +++++++++----------

>  1 file changed, 31 insertions(+), 31 deletions(-)

> 

> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c

> index 160bb58ce708..e63906a69e30 100644

> --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c

> +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c

> @@ -999,10 +999,10 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,

>  	struct mt9m114_device *dev = to_mt9m114_sensor(sd);

>  	int ret = 0;

>  	unsigned int coarse_integration = 0;

> -	unsigned int FLines = 0;

> -	unsigned int FrameLengthLines = 0; /* ExposureTime.FrameLengthLines; */

> -	unsigned int AnalogGain, DigitalGain;

> -	u32 AnalogGainToWrite = 0;

> +	unsigned int f_lines = 0;

> +	unsigned int frame_len_lines = 0; /* ExposureTime.FrameLengthLines; */

> +	unsigned int analog_gain, digital_gain;

> +	u32 analog_gain_to_write = 0;

> 


this patch is made of multiple logical changes, i.e. more than one
variable at a time are renamed. Maybe this should be split in
one patch per variable name.

thank you,

fabio
Hans Verkuil April 20, 2021, 1:24 p.m. UTC | #2 
On 19/04/2021 21:12, Deepak R Varma wrote:
> Improve multi-line function argument alignment according to the code style

> guidelines. Resolves checkpatch feedback: "Alignment should match

> open parenthesis".

> 

> Signed-off-by: Deepak R Varma <drv@mailo.com>


WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma
<drv@mailo.com>'

Which email should I use? Ideally you should post from the same email
as the Signed-off-by.

Regards,

	Hans

> ---

>  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--

>  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++--

>  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++--

>  3 files changed, 6 insertions(+), 6 deletions(-)

> 

> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> index 2b71de722ec3..6be3ee1d93a5 100644

> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> @@ -192,8 +192,8 @@ static int __gc0310_buf_reg_array(struct i2c_client *client,

>  }

>  

>  static int __gc0310_write_reg_is_consecutive(struct i2c_client *client,

> -	struct gc0310_write_ctrl *ctrl,

> -	const struct gc0310_reg *next)

> +					     struct gc0310_write_ctrl *ctrl,

> +					     const struct gc0310_reg *next)

>  {

>  	if (ctrl->index == 0)

>  		return 1;

> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> index 78147ffb6099..6ba4a8adff7c 100644

> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> @@ -171,8 +171,8 @@ static int __gc2235_buf_reg_array(struct i2c_client *client,

>  }

>  

>  static int __gc2235_write_reg_is_consecutive(struct i2c_client *client,

> -	struct gc2235_write_ctrl *ctrl,

> -	const struct gc2235_reg *next)

> +					     struct gc2235_write_ctrl *ctrl,

> +					     const struct gc2235_reg *next)

>  {

>  	if (ctrl->index == 0)

>  		return 1;

> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> index eecefcd734d0..aec7392fd1de 100644

> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> @@ -212,8 +212,8 @@ static int __ov2722_buf_reg_array(struct i2c_client *client,

>  }

>  

>  static int __ov2722_write_reg_is_consecutive(struct i2c_client *client,

> -	struct ov2722_write_ctrl *ctrl,

> -	const struct ov2722_reg *next)

> +					     struct ov2722_write_ctrl *ctrl,

> +					     const struct ov2722_reg *next)

>  {

>  	if (ctrl->index == 0)

>  		return 1;

>
Hans Verkuil April 20, 2021, 1:26 p.m. UTC | #3 
On 20/04/2021 10:39, Fabio Aiuto wrote:
> Hi,

> 

> On Tue, Apr 20, 2021 at 12:45:57AM +0530, Deepak R Varma wrote:

>> Mixed case variable names are discouraged and they result in checkpatch

>> script "Avoid CamelCase" warnings. Replace such CamelCase variable names

>> by lower case strings according to the coding style guidelines.

>>

>> Signed-off-by: Deepak R Varma <drv@mailo.com>

>> ---

>>  .../media/atomisp/i2c/atomisp-mt9m114.c       | 62 +++++++++----------

>>  1 file changed, 31 insertions(+), 31 deletions(-)

>>

>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c

>> index 160bb58ce708..e63906a69e30 100644

>> --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c

>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c

>> @@ -999,10 +999,10 @@ static long mt9m114_s_exposure(struct v4l2_subdev *sd,

>>  	struct mt9m114_device *dev = to_mt9m114_sensor(sd);

>>  	int ret = 0;

>>  	unsigned int coarse_integration = 0;

>> -	unsigned int FLines = 0;

>> -	unsigned int FrameLengthLines = 0; /* ExposureTime.FrameLengthLines; */

>> -	unsigned int AnalogGain, DigitalGain;

>> -	u32 AnalogGainToWrite = 0;

>> +	unsigned int f_lines = 0;

>> +	unsigned int frame_len_lines = 0; /* ExposureTime.FrameLengthLines; */

>> +	unsigned int analog_gain, digital_gain;

>> +	u32 analog_gain_to_write = 0;

>>

> 

> this patch is made of multiple logical changes, i.e. more than one

> variable at a time are renamed. Maybe this should be split in

> one patch per variable name.


I'm OK with this, it's pretty readable and obvious what is going on.

Regards,

	Hans
Deepak R Varma April 20, 2021, 5:19 p.m. UTC | #4 
On Tue, Apr 20, 2021 at 03:24:32PM +0200, Hans Verkuil wrote:
> On 19/04/2021 21:12, Deepak R Varma wrote:

> > Improve multi-line function argument alignment according to the code style

> > guidelines. Resolves checkpatch feedback: "Alignment should match

> > open parenthesis".

> > 

> > Signed-off-by: Deepak R Varma <drv@mailo.com>

> 

> WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma

> <drv@mailo.com>'

> 

> Which email should I use? Ideally you should post from the same email

> as the Signed-off-by.


I am really sorry for this. I was trying to set up mutt to handle both
my accounts and guess that led to this issue. I will resubmit the patch
set with the appropriate email match. Will that be okay?

Thank you,
deepak.

> 

> Regards,

> 

> 	Hans

> 

> > ---

> >  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--

> >  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++--

> >  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++--

> >  3 files changed, 6 insertions(+), 6 deletions(-)

> > 

> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> > index 2b71de722ec3..6be3ee1d93a5 100644

> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> > @@ -192,8 +192,8 @@ static int __gc0310_buf_reg_array(struct i2c_client *client,

> >  }

> >  

> >  static int __gc0310_write_reg_is_consecutive(struct i2c_client *client,

> > -	struct gc0310_write_ctrl *ctrl,

> > -	const struct gc0310_reg *next)

> > +					     struct gc0310_write_ctrl *ctrl,

> > +					     const struct gc0310_reg *next)

> >  {

> >  	if (ctrl->index == 0)

> >  		return 1;

> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> > index 78147ffb6099..6ba4a8adff7c 100644

> > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> > @@ -171,8 +171,8 @@ static int __gc2235_buf_reg_array(struct i2c_client *client,

> >  }

> >  

> >  static int __gc2235_write_reg_is_consecutive(struct i2c_client *client,

> > -	struct gc2235_write_ctrl *ctrl,

> > -	const struct gc2235_reg *next)

> > +					     struct gc2235_write_ctrl *ctrl,

> > +					     const struct gc2235_reg *next)

> >  {

> >  	if (ctrl->index == 0)

> >  		return 1;

> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> > index eecefcd734d0..aec7392fd1de 100644

> > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> > @@ -212,8 +212,8 @@ static int __ov2722_buf_reg_array(struct i2c_client *client,

> >  }

> >  

> >  static int __ov2722_write_reg_is_consecutive(struct i2c_client *client,

> > -	struct ov2722_write_ctrl *ctrl,

> > -	const struct ov2722_reg *next)

> > +					     struct ov2722_write_ctrl *ctrl,

> > +					     const struct ov2722_reg *next)

> >  {

> >  	if (ctrl->index == 0)

> >  		return 1;

> > 

>
Hans Verkuil April 20, 2021, 8:59 p.m. UTC | #5 
On 20/04/2021 19:19, Deepak R Varma wrote:
> On Tue, Apr 20, 2021 at 03:24:32PM +0200, Hans Verkuil wrote:

>> On 19/04/2021 21:12, Deepak R Varma wrote:

>>> Improve multi-line function argument alignment according to the code style

>>> guidelines. Resolves checkpatch feedback: "Alignment should match

>>> open parenthesis".

>>>

>>> Signed-off-by: Deepak R Varma <drv@mailo.com>

>>

>> WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma

>> <drv@mailo.com>'

>>

>> Which email should I use? Ideally you should post from the same email

>> as the Signed-off-by.

> 

> I am really sorry for this. I was trying to set up mutt to handle both

> my accounts and guess that led to this issue. I will resubmit the patch

> set with the appropriate email match. Will that be okay?


It is sufficient to just let me know which email I should use. I can edit
it so it shows the correct author email.

Regards,

	Hans

> 

> Thank you,

> deepak.

> 

>>

>> Regards,

>>

>> 	Hans

>>

>>> ---

>>>  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--

>>>  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++--

>>>  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++--

>>>  3 files changed, 6 insertions(+), 6 deletions(-)

>>>

>>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

>>> index 2b71de722ec3..6be3ee1d93a5 100644

>>> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

>>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

>>> @@ -192,8 +192,8 @@ static int __gc0310_buf_reg_array(struct i2c_client *client,

>>>  }

>>>  

>>>  static int __gc0310_write_reg_is_consecutive(struct i2c_client *client,

>>> -	struct gc0310_write_ctrl *ctrl,

>>> -	const struct gc0310_reg *next)

>>> +					     struct gc0310_write_ctrl *ctrl,

>>> +					     const struct gc0310_reg *next)

>>>  {

>>>  	if (ctrl->index == 0)

>>>  		return 1;

>>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

>>> index 78147ffb6099..6ba4a8adff7c 100644

>>> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

>>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

>>> @@ -171,8 +171,8 @@ static int __gc2235_buf_reg_array(struct i2c_client *client,

>>>  }

>>>  

>>>  static int __gc2235_write_reg_is_consecutive(struct i2c_client *client,

>>> -	struct gc2235_write_ctrl *ctrl,

>>> -	const struct gc2235_reg *next)

>>> +					     struct gc2235_write_ctrl *ctrl,

>>> +					     const struct gc2235_reg *next)

>>>  {

>>>  	if (ctrl->index == 0)

>>>  		return 1;

>>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

>>> index eecefcd734d0..aec7392fd1de 100644

>>> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

>>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

>>> @@ -212,8 +212,8 @@ static int __ov2722_buf_reg_array(struct i2c_client *client,

>>>  }

>>>  

>>>  static int __ov2722_write_reg_is_consecutive(struct i2c_client *client,

>>> -	struct ov2722_write_ctrl *ctrl,

>>> -	const struct ov2722_reg *next)

>>> +					     struct ov2722_write_ctrl *ctrl,

>>> +					     const struct ov2722_reg *next)

>>>  {

>>>  	if (ctrl->index == 0)

>>>  		return 1;

>>>

>>
Deepak R Varma April 21, 2021, 10:19 a.m. UTC | #6 
On Tue, Apr 20, 2021 at 10:59:16PM +0200, Hans Verkuil wrote:
> On 20/04/2021 19:19, Deepak R Varma wrote:

> > On Tue, Apr 20, 2021 at 03:24:32PM +0200, Hans Verkuil wrote:

> >> On 19/04/2021 21:12, Deepak R Varma wrote:

> >>> Improve multi-line function argument alignment according to the code style

> >>> guidelines. Resolves checkpatch feedback: "Alignment should match

> >>> open parenthesis".

> >>>

> >>> Signed-off-by: Deepak R Varma <drv@mailo.com>

> >>

> >> WARNING: From:/Signed-off-by: email address mismatch: 'From: Deepak R Varma <mh12gx2825@gmail.com>' != 'Signed-off-by: Deepak R Varma

> >> <drv@mailo.com>'

> >>

> >> Which email should I use? Ideally you should post from the same email

> >> as the Signed-off-by.

> > 

> > I am really sorry for this. I was trying to set up mutt to handle both

> > my accounts and guess that led to this issue. I will resubmit the patch

> > set with the appropriate email match. Will that be okay?

> 

> It is sufficient to just let me know which email I should use. I can edit

> it so it shows the correct author email.


Hello Hans,
You can use drv@mailo.com as the signed off email id.

I have received feedback on other patches of this set, so I am going to
resubmit the patch set v1 from the correct email ID.

Thank you,
deepak.


> 

> Regards,

> 

> 	Hans

> 

> > 

> > Thank you,

> > deepak.

> > 

> >>

> >> Regards,

> >>

> >> 	Hans

> >>

> >>> ---

> >>>  drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++--

> >>>  drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++--

> >>>  drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++--

> >>>  3 files changed, 6 insertions(+), 6 deletions(-)

> >>>

> >>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> >>> index 2b71de722ec3..6be3ee1d93a5 100644

> >>> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> >>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c

> >>> @@ -192,8 +192,8 @@ static int __gc0310_buf_reg_array(struct i2c_client *client,

> >>>  }

> >>>  

> >>>  static int __gc0310_write_reg_is_consecutive(struct i2c_client *client,

> >>> -	struct gc0310_write_ctrl *ctrl,

> >>> -	const struct gc0310_reg *next)

> >>> +					     struct gc0310_write_ctrl *ctrl,

> >>> +					     const struct gc0310_reg *next)

> >>>  {

> >>>  	if (ctrl->index == 0)

> >>>  		return 1;

> >>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> >>> index 78147ffb6099..6ba4a8adff7c 100644

> >>> --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> >>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c

> >>> @@ -171,8 +171,8 @@ static int __gc2235_buf_reg_array(struct i2c_client *client,

> >>>  }

> >>>  

> >>>  static int __gc2235_write_reg_is_consecutive(struct i2c_client *client,

> >>> -	struct gc2235_write_ctrl *ctrl,

> >>> -	const struct gc2235_reg *next)

> >>> +					     struct gc2235_write_ctrl *ctrl,

> >>> +					     const struct gc2235_reg *next)

> >>>  {

> >>>  	if (ctrl->index == 0)

> >>>  		return 1;

> >>> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> >>> index eecefcd734d0..aec7392fd1de 100644

> >>> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> >>> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c

> >>> @@ -212,8 +212,8 @@ static int __ov2722_buf_reg_array(struct i2c_client *client,

> >>>  }

> >>>  

> >>>  static int __ov2722_write_reg_is_consecutive(struct i2c_client *client,

> >>> -	struct ov2722_write_ctrl *ctrl,

> >>> -	const struct ov2722_reg *next)

> >>> +					     struct ov2722_write_ctrl *ctrl,

> >>> +					     const struct ov2722_reg *next)

> >>>  {

> >>>  	if (ctrl->index == 0)

> >>>  		return 1;

> >>>

> >>

>