Re: [PATCH] media: adv7180: Fix cppcheck warnings and errors

[Bhavin Sharma <bhavin.sharma@siliconsignals.io>]

Hi,

> Quoting Bhavin Sharma (2023-12-29 13:37:14)
>> Thanks for the reply,�Kieran
>>
>> >> WARNING: Missing a blank line after declarations
>> >> ERROR: else should follow close brace '}'
>> >>
>> >> Signed-off-by: Bhavin Sharma <bhavin.sharma@siliconsignals.io>
>> >>
>> >> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
>> >> index 54134473186b..91756116eff7 100644
>> >> --- a/drivers/media/i2c/adv7180.c
>> >> +++ b/drivers/media/i2c/adv7180.c
>> >> @@ -357,6 +357,7 @@ static int adv7180_querystd(struct v4l2_subdev *sd, v4l2_std_id *std)
>> >>� {
>> >>�������� struct adv7180_state *state = to_state(sd);
>>
>> >>Personally, I would keep the if (err) hugging the line it's associated
>> with.
>>
>> If we follow the code base pattern for this diver, we are getting same online space in conditional if statements.
>> So, we need to make changes there also.

> If there are multiple places in a file for the same fixup, then indeed -
> make them all in a single patch as a single cleanup.


>> >>�������� int err = mutex_lock_interruptible(&state->mutex);
>> >> +
>> >>�������� if (err)
>> >>���������������� return err;
>> >>�
>> >> @@ -411,6 +412,7 @@ static int adv7180_g_input_status(struct v4l2_subdev *sd, u32 *status)
>> >>� {
>> >>�������� struct adv7180_state *state = to_state(sd);
>> >>�������� int ret = mutex_lock_interruptible(&state->mutex);
>> >> +
>> >>�������� if (ret)
>> >>���������������� return ret;
>> >>�
>> >> @@ -1046,8 +1048,7 @@ static int adv7182_init(struct adv7180_state *state)
>> >>���������������������������������������������� ADV7180_REG_EXTENDED_OUTPUT_CONTROL,
>> >>���������������������������������������������� 0x17);
>> >>������������������������ }
>> >> -�������������� }
>> >> -�������������� else
>> >> +�������������� } else
>>
>> >>I think kernel code style requires an else clause following a multiline
>> scope to also have its scope enclosed in braces even if it's a single
>> statement.
>>
>> On many places in driver there is single statement after else without closing�
>> So, we have to make changes in those places also.
>>
>> So, better I should make changes in all places and make version V2 patch.

> Yes, but you should probably tackle both cleanups as two patches
> covering the whole file for each cleanup.

I have submitted two different patches each for separate cleanups. 

Any update on those patches?

Thank you,
Bhavin Sharma