While I know you're just dealing with mechanical style checking tools in
this case, that indentation was in fact deliberate in order to make
visually obvious the regions of code where the lock was being held. It
was helpful to keep me from accidentally escaping a region without
releasing the lock.
But this is an area of code that hasn't been touched in a very long time
so I imagine the visually obvious aspect is not so important anymore.
-Mike
On Wed, 24 Nov 2021, Hans Verkuil wrote:
> Fix these smatch warnings:
>
> drivers/media/usb/pvrusb2/pvrusb2-encoder.c:288 pvr2_encoder_cmd() warn: inconsistent indenting
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:1730 pvr2_hdw_set_streaming() warn: inconsistent indenting
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3461 pvr2_hdw_cpufw_set_enabled() warn: inconsistent indenting
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c:3501 pvr2_hdw_cpufw_get() warn: inconsistent indenting
>
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx>
> ---
> drivers/media/usb/pvrusb2/pvrusb2-encoder.c | 4 +--
> drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 33 +++++++++++----------
> 2 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-encoder.c b/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
> index f6005d1296ef..c8102772344b 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
> @@ -188,8 +188,8 @@ static int pvr2_encoder_cmd(void *ctxt,
> }
>
>
> - LOCK_TAKE(hdw->ctl_lock); while (1) {
> -
> + LOCK_TAKE(hdw->ctl_lock);
> + while (1) {
> if (!hdw->state_encoder_ok) {
> ret = -EIO;
> break;
> diff --git a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> index 3915d551d59e..cd7b118d5929 100644
> --- a/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> +++ b/drivers/media/usb/pvrusb2/pvrusb2-hdw.c
> @@ -1718,16 +1718,16 @@ int pvr2_hdw_get_streaming(struct pvr2_hdw *hdw)
> int pvr2_hdw_set_streaming(struct pvr2_hdw *hdw,int enable_flag)
> {
> int ret,st;
> - LOCK_TAKE(hdw->big_lock); do {
> - pvr2_hdw_untrip_unlocked(hdw);
> - if ((!enable_flag) != !(hdw->state_pipeline_req)) {
> - hdw->state_pipeline_req = enable_flag != 0;
> - pvr2_trace(PVR2_TRACE_START_STOP,
> - "/*--TRACE_STREAM--*/ %s",
> - enable_flag ? "enable" : "disable");
> - }
> - pvr2_hdw_state_sched(hdw);
> - } while (0); LOCK_GIVE(hdw->big_lock);
> + LOCK_TAKE(hdw->big_lock);
> + pvr2_hdw_untrip_unlocked(hdw);
> + if (!enable_flag != !hdw->state_pipeline_req) {
> + hdw->state_pipeline_req = enable_flag != 0;
> + pvr2_trace(PVR2_TRACE_START_STOP,
> + "/*--TRACE_STREAM--*/ %s",
> + enable_flag ? "enable" : "disable");
> + }
> + pvr2_hdw_state_sched(hdw);
> + LOCK_GIVE(hdw->big_lock);
> if ((ret = pvr2_hdw_wait(hdw,0)) < 0) return ret;
> if (enable_flag) {
> while ((st = hdw->master_state) != PVR2_STATE_RUN) {
> @@ -3394,7 +3394,8 @@ void pvr2_hdw_cpufw_set_enabled(struct pvr2_hdw *hdw,
> int ret;
> u16 address;
> unsigned int pipe;
> - LOCK_TAKE(hdw->big_lock); do {
> + LOCK_TAKE(hdw->big_lock);
> + do {
> if ((hdw->fw_buffer == NULL) == !enable_flag) break;
>
> if (!enable_flag) {
> @@ -3457,8 +3458,8 @@ void pvr2_hdw_cpufw_set_enabled(struct pvr2_hdw *hdw,
> pvr2_trace(PVR2_TRACE_FIRMWARE,
> "Done sucking down EEPROM contents");
> }
> -
> - } while (0); LOCK_GIVE(hdw->big_lock);
> + } while (0);
> + LOCK_GIVE(hdw->big_lock);
> }
>
>
> @@ -3473,7 +3474,8 @@ int pvr2_hdw_cpufw_get(struct pvr2_hdw *hdw,unsigned int offs,
> char *buf,unsigned int cnt)
> {
> int ret = -EINVAL;
> - LOCK_TAKE(hdw->big_lock); do {
> + LOCK_TAKE(hdw->big_lock);
> + do {
> if (!buf) break;
> if (!cnt) break;
>
> @@ -3498,7 +3500,8 @@ int pvr2_hdw_cpufw_get(struct pvr2_hdw *hdw,unsigned int offs,
> "Read firmware data offs=%d cnt=%d",
> offs,cnt);
> ret = cnt;
> - } while (0); LOCK_GIVE(hdw->big_lock);
> + } while (0);
> + LOCK_GIVE(hdw->big_lock);
>
> return ret;
> }
>
