Re: [PATCH 2/2] drm: zynqmp_dp: Use scope-based mutex helpers

From: Bart Van Assche
Date: Thu Feb 06 2025 - 15:31:36 EST


On 2/6/25 11:41 AM, Sean Anderson wrote:
static int zynqmp_dp_enhanced_set(void *data, u64 val)
{
struct zynqmp_dp *dp = data;
- int ret = 0;
- mutex_lock(&dp->lock);
+ guard(mutex)(&dp->lock);
dp->test.enhanced = val;
if (dp->test.active)
- ret = zynqmp_dp_test_setup(dp);
- mutex_unlock(&dp->lock);
+ return zynqmp_dp_test_setup(dp);
- return ret;
+ return 0;
}

Has it been considered to combine the two return statements into one
with the ternary operator (?:)?

static int zynqmp_dp_downspread_set(void *data, u64 val)
{
struct zynqmp_dp *dp = data;
- int ret = 0;
- mutex_lock(&dp->lock);
+ guard(mutex)(&dp->lock);
dp->test.downspread = val;
if (dp->test.active)
- ret = zynqmp_dp_test_setup(dp);
- mutex_unlock(&dp->lock);
+ return zynqmp_dp_test_setup(dp);
- return ret;
+ return 0;
}

Same question here.

@@ -2053,7 +2039,8 @@ static ssize_t zynqmp_dp_custom_read(struct file *file, char __user *user_buf,
return ret;
mutex_lock(&dp->lock);
- ret = simple_read_from_buffer(user_buf, count, ppos, &dp->test.custom,
+ ret = simple_read_from_buffer(user_buf, count, ppos,
+ &dp->test.custom,
sizeof(dp->test.custom));

This change has not been mentioned in the patch description and is not
related to the other changes in this patch?

- return ret;
+ dp->test.link_cnt = val;
+ if (dp->test.active)
+ return zynqmp_dp_test_setup(dp);
+
+ return 0;
}

Has it been considered to use the ternary operator here?

Thanks,

Bart.