2021-04-19 22:16:45

by Deepak R Varma

[permalink] [raw]
Subject: [PATCH 4/6] staging: media: atomisp: reformat code comment blocks

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 <[email protected]>
---
.../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);
+ */
AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
ret = mt9m114_write_reg(client, MISENSOR_16BIT,
REG_GAIN, AnalogGainToWrite);
@@ -1096,7 +1099,8 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
}

/* This returns the exposure time being used. This should only be used
- for filling in EXIF data, not for actual image processing. */
+ * for filling in EXIF data, not for actual image processing.
+ */
static int mt9m114_g_exposure(struct v4l2_subdev *sd, s32 *value)
{
struct i2c_client *client = v4l2_get_subdevdata(sd);
@@ -1297,7 +1301,7 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)

/* Fake interface
* mt9m114 now can not support 3a_lock
-*/
+ */
static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
{
aaalock = val;
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
index c17615149f46..91dca275a08e 100644
--- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
+++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
@@ -732,7 +732,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
* existing integrations often wire two (reset/power_down)
* because that is the way other sensors work. There is no
* way to tell how it is wired internally, so existing
- * firmwares expose both and we drive them symmetrically. */
+ * firmwares expose both and we drive them symmetrically.
+ */
if (flag) {
ret = dev->platform_data->gpio0_ctrl(sd, 1);
usleep_range(10000, 15000);
--
2.25.1


2021-04-20 08:47:02

by Fabio Aiuto

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

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 <[email protected]>
> ---
> .../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); */



> AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
> ret = mt9m114_write_reg(client, MISENSOR_16BIT,
> REG_GAIN, AnalogGainToWrite);
> @@ -1096,7 +1099,8 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
> }
>
> /* This returns the exposure time being used. This should only be used
> - for filling in EXIF data, not for actual image processing. */
> + * for filling in EXIF data, not for actual image processing.
> + */
> static int mt9m114_g_exposure(struct v4l2_subdev *sd, s32 *value)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> @@ -1297,7 +1301,7 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)
>
> /* Fake interface
> * mt9m114 now can not support 3a_lock
> -*/
> + */
> static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
> {
> aaalock = val;
> diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> index c17615149f46..91dca275a08e 100644
> --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> @@ -732,7 +732,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
> * existing integrations often wire two (reset/power_down)
> * because that is the way other sensors work. There is no
> * way to tell how it is wired internally, so existing
> - * firmwares expose both and we drive them symmetrically. */
> + * firmwares expose both and we drive them symmetrically.
> + */
> if (flag) {
> ret = dev->platform_data->gpio0_ctrl(sd, 1);
> usleep_range(10000, 15000);
> --
> 2.25.1
>
>

multiline comment format should appear this way:

/*
* This is a comment, a
* multiline one.
*/

with the first line of the comment empty.

thank you,

fabio

2021-04-20 17:18:24

by Deepak R Varma

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

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 <[email protected]>
> > ---
> > .../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?

>
>
>
> > AnalogGainToWrite = (u16)((DigitalGain << 12) | (u16)AnalogGain);
> > ret = mt9m114_write_reg(client, MISENSOR_16BIT,
> > REG_GAIN, AnalogGainToWrite);
> > @@ -1096,7 +1099,8 @@ static long mt9m114_ioctl(struct v4l2_subdev *sd, unsigned int cmd, void *arg)
> > }
> >
> > /* This returns the exposure time being used. This should only be used
> > - for filling in EXIF data, not for actual image processing. */
> > + * for filling in EXIF data, not for actual image processing.
> > + */
> > static int mt9m114_g_exposure(struct v4l2_subdev *sd, s32 *value)
> > {
> > struct i2c_client *client = v4l2_get_subdevdata(sd);
> > @@ -1297,7 +1301,7 @@ static int mt9m114_g_ev(struct v4l2_subdev *sd, s32 *val)
> >
> > /* Fake interface
> > * mt9m114 now can not support 3a_lock
> > -*/
> > + */
> > static int mt9m114_s_3a_lock(struct v4l2_subdev *sd, s32 val)
> > {
> > aaalock = val;
> > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > index c17615149f46..91dca275a08e 100644
> > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c
> > @@ -732,7 +732,8 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag)
> > * existing integrations often wire two (reset/power_down)
> > * because that is the way other sensors work. There is no
> > * way to tell how it is wired internally, so existing
> > - * firmwares expose both and we drive them symmetrically. */
> > + * firmwares expose both and we drive them symmetrically.
> > + */
> > if (flag) {
> > ret = dev->platform_data->gpio0_ctrl(sd, 1);
> > usleep_range(10000, 15000);
> > --
> > 2.25.1
> >
> >
>
> multiline comment format should appear this way:
>
> /*
> * This is a comment, a
> * multiline one.
> */
>
> with the first line of the comment empty.

Sounds good. The coding style guide also allows the other style for
specific driver types. I will improve this change according to your
feedback and resubmit.

thank you,
deepak.

>
> thank you,
>
> fabio

2021-04-21 07:50:31

by Fabio Aiuto

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

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 <[email protected]>
> > > ---
> > > .../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

2021-04-28 07:54:31

by Dan Carpenter

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

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:
> > 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); */
>

I'm going through old emails...

Just delete all commented out code.

regards,
dan carpenter