| Message ID | cover.1619199344.git.drv@mailo.com |
|---|---|
| Headers | show |
| Series | staging: media: atomisp: code cleanup fixes | expand |
On Sun, Apr 25, 2021 at 02:12:20PM +0530, Deepak R Varma wrote: > Balance braces around the if else blocks as per the code style guidelines. > Resolves checkpatch script CHECK / WARNING feedback messages. > > Signed-off-by: Deepak R Varma <drv@mailo.com> > --- > > Changes since v2: > - None. > Changes since v1: > - None. > > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++-- > drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++-- > drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++-- > drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 7 ++++--- > drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++-- > 5 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > index 6be3ee1d93a5..d68a2bcc9ae1 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > @@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on) > { > int ret; > > - if (on == 0) > + if (on == 0) { > return power_down(sd); > - else { > + } else { > ret = power_up(sd); > if (!ret) > return gc0310_init(sd); > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > index 6ba4a8adff7c..e722c639b60d 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > @@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on) > { > int ret; > > - if (on == 0) > + if (on == 0) { > ret = power_down(sd); > - else { > + } else { > ret = power_up(sd); > if (!ret) > ret = __gc2235_init(sd); > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > index f5de81132177..465fc4468442 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > @@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd) > > static int mt9m114_s_power(struct v4l2_subdev *sd, int power) > { > - if (power == 0) > + if (power == 0) { > return power_down(sd); > - else { > + } else { > if (power_up(sd)) > return -EINVAL; > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > index c90730513438..92c52431bd8f 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > @@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value) > ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val); > if (ret) > return ret; > - if (value) { > + > + if (value) > val |= OV2680_FLIP_MIRROR_BIT_ENABLE; > - } else { > + else > val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE; > - } > + Hi Deepak, what you did above is not what is written in the commit message description about. Here unneeded bracks are removed in both branches, is not a matter of braces balancing. thank you, fabio > ret = ov2680_write_reg(client, 1, > OV2680_FLIP_REG, val); > if (ret) > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > index aec7392fd1de..d046a9804f63 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > @@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on) > { > int ret; > > - if (on == 0) > + if (on == 0) { > return power_down(sd); > - else { > + } else { > ret = power_up(sd); > if (!ret) > return ov2722_init(sd); > -- > 2.25.1 > > > >
On Mon, Apr 26, 2021 at 11:56:11AM +0200, Fabio Aiuto wrote: > On Sun, Apr 25, 2021 at 02:12:20PM +0530, Deepak R Varma wrote: > > Balance braces around the if else blocks as per the code style guidelines. > > Resolves checkpatch script CHECK / WARNING feedback messages. > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > --- > > > > Changes since v2: > > - None. > > Changes since v1: > > - None. > > > > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++-- > > drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++-- > > drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++-- > > drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 7 ++++--- > > drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++-- > > 5 files changed, 12 insertions(+), 11 deletions(-) > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > index 6be3ee1d93a5..d68a2bcc9ae1 100644 > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > @@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on) > > { > > int ret; > > > > - if (on == 0) > > + if (on == 0) { > > return power_down(sd); > > - else { > > + } else { > > ret = power_up(sd); > > if (!ret) > > return gc0310_init(sd); > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > > index 6ba4a8adff7c..e722c639b60d 100644 > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > > @@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on) > > { > > int ret; > > > > - if (on == 0) > > + if (on == 0) { > > ret = power_down(sd); > > - else { > > + } else { > > ret = power_up(sd); > > if (!ret) > > ret = __gc2235_init(sd); > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > > index f5de81132177..465fc4468442 100644 > > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > > @@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd) > > > > static int mt9m114_s_power(struct v4l2_subdev *sd, int power) > > { > > - if (power == 0) > > + if (power == 0) { > > return power_down(sd); > > - else { > > + } else { > > if (power_up(sd)) > > return -EINVAL; > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > > index c90730513438..92c52431bd8f 100644 > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > > @@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value) > > ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val); > > if (ret) > > return ret; > > - if (value) { > > + > > + if (value) > > val |= OV2680_FLIP_MIRROR_BIT_ENABLE; > > - } else { > > + else > > val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE; > > - } > > + > > Hi Deepak, > > what you did above is not what is written in the commit message > description about. Here unneeded bracks are removed in both > branches, is not a matter of braces balancing. Okay. I was thinking adding where necessary and removing where not would lead to expected balancing. I will send this as a separate patch in this patch set. Is it okay to add a new patch to the set now? Thank you, deepak. > > thank you, > > fabio > > > ret = ov2680_write_reg(client, 1, > > OV2680_FLIP_REG, val); > > if (ret) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > index aec7392fd1de..d046a9804f63 100644 > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > @@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on) > > { > > int ret; > > > > - if (on == 0) > > + if (on == 0) { > > return power_down(sd); > > - else { > > + } else { > > ret = power_up(sd); > > if (!ret) > > return ov2722_init(sd); > > -- > > 2.25.1 > > > > > > > >
On Mon, Apr 26, 2021 at 08:00:22PM +0530, Deepak R Varma wrote: > On Mon, Apr 26, 2021 at 11:56:11AM +0200, Fabio Aiuto wrote: > > On Sun, Apr 25, 2021 at 02:12:20PM +0530, Deepak R Varma wrote: > > > Balance braces around the if else blocks as per the code style guidelines. > > > Resolves checkpatch script CHECK / WARNING feedback messages. > > > > > > Signed-off-by: Deepak R Varma <drv@mailo.com> > > > --- > > > > > > Changes since v2: > > > - None. > > > Changes since v1: > > > - None. > > > > > > drivers/staging/media/atomisp/i2c/atomisp-gc0310.c | 4 ++-- > > > drivers/staging/media/atomisp/i2c/atomisp-gc2235.c | 4 ++-- > > > drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c | 4 ++-- > > > drivers/staging/media/atomisp/i2c/atomisp-ov2680.c | 7 ++++--- > > > drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 4 ++-- > > > 5 files changed, 12 insertions(+), 11 deletions(-) > > > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > index 6be3ee1d93a5..d68a2bcc9ae1 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc0310.c > > > @@ -872,9 +872,9 @@ static int gc0310_s_power(struct v4l2_subdev *sd, int on) > > > { > > > int ret; > > > > > > - if (on == 0) > > > + if (on == 0) { > > > return power_down(sd); > > > - else { > > > + } else { > > > ret = power_up(sd); > > > if (!ret) > > > return gc0310_init(sd); > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > > > index 6ba4a8adff7c..e722c639b60d 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-gc2235.c > > > @@ -658,9 +658,9 @@ static int gc2235_s_power(struct v4l2_subdev *sd, int on) > > > { > > > int ret; > > > > > > - if (on == 0) > > > + if (on == 0) { > > > ret = power_down(sd); > > > - else { > > > + } else { > > > ret = power_up(sd); > > > if (!ret) > > > ret = __gc2235_init(sd); > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > > > index f5de81132177..465fc4468442 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-mt9m114.c > > > @@ -568,9 +568,9 @@ static int power_down(struct v4l2_subdev *sd) > > > > > > static int mt9m114_s_power(struct v4l2_subdev *sd, int power) > > > { > > > - if (power == 0) > > > + if (power == 0) { > > > return power_down(sd); > > > - else { > > > + } else { > > > if (power_up(sd)) > > > return -EINVAL; > > > > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > > > index c90730513438..92c52431bd8f 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2680.c > > > @@ -461,11 +461,12 @@ static int ov2680_v_flip(struct v4l2_subdev *sd, s32 value) > > > ret = ov2680_read_reg(client, 1, OV2680_FLIP_REG, &val); > > > if (ret) > > > return ret; > > > - if (value) { > > > + > > > + if (value) > > > val |= OV2680_FLIP_MIRROR_BIT_ENABLE; > > > - } else { > > > + else > > > val &= ~OV2680_FLIP_MIRROR_BIT_ENABLE; > > > - } > > > + > > > > Hi Deepak, > > > > what you did above is not what is written in the commit message > > description about. Here unneeded bracks are removed in both > > branches, is not a matter of braces balancing. > > Okay. I was thinking adding where necessary and removing where not > would lead to expected balancing. > I will send this as a separate patch in this patch set. Is it okay to > add a new patch to the set now? yes, a patchset from vN to v(N + 1) is allowed to increase in patch number, at least in my experience. > > Thank you, > deepak. > > > > > thank you, > > > > fabio > > > > > ret = ov2680_write_reg(client, 1, > > > OV2680_FLIP_REG, val); > > > if (ret) > > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > > index aec7392fd1de..d046a9804f63 100644 > > > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > > > @@ -772,9 +772,9 @@ static int ov2722_s_power(struct v4l2_subdev *sd, int on) > > > { > > > int ret; > > > > > > - if (on == 0) > > > + if (on == 0) { > > > return power_down(sd); > > > - else { > > > + } else { > > > ret = power_up(sd); > > > if (!ret) > > > return ov2722_init(sd); > > > -- > > > 2.25.1 > > > > > > > > > > > > > > thank you, fabio
Hi Deepak, I'm dropping this patch since others have fixed this already. Regards, Hans On 25/04/2021 10:41, 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> > --- > > Changes since v2: > - None. > Changes since v1: > - None. > > > 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; >
This patch set addresses different kinds of checkpatch WARNING and CHECK complaints. Note: - Patch set being resent due a email messageID issue in the earlier patch set. - The patches should be applied in the ascending order. Changes since v2: Generic change: 1. Correct patch versioning in patch subject Patch Specific change: 1. patch 1/6 : none 2. patch 2/6 : none 3. patch 3/6 : none 4. patch 4/6 : a. Tag Fabio Auito for the patch suggestion 5. patch 5/6 : none 6. patch 6/6: a. Tag Fabio Auito for the patch suggestion Changes since v1: Generic change: 1. The patch set is being resent from an email account that matches with the patch signed-of-by tag. Issue highlighted by Hans Verkuil. Patch specific changes: 1. patch 1/6 : none 2. patch 2/6 : none 3. patch 3/6 : none 4. patch 4/6 : implement following changes suggested by Fabio Aiuto a. Corrected commenting style b. Similar style implemented for other comment blocks in the same files. 5. patch 5/6 : none 6. patch 6/6: implement following changes suggested by Fabio Aiuto a. use dev_info instead of pr_info b. update patch log message accordingly 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: replace raw printk() by dev_info() .../media/atomisp/i2c/atomisp-gc0310.c | 14 +-- .../media/atomisp/i2c/atomisp-gc2235.c | 29 ++--- .../atomisp/i2c/atomisp-libmsrlisthelper.c | 6 +- .../media/atomisp/i2c/atomisp-lm3554.c | 2 +- .../media/atomisp/i2c/atomisp-mt9m114.c | 106 ++++++++++-------- .../media/atomisp/i2c/atomisp-ov2680.c | 43 ++++--- .../media/atomisp/i2c/atomisp-ov2722.c | 10 +- 7 files changed, 116 insertions(+), 94 deletions(-)