Re: [PATCH v4 02/12] drm/panthor: Minor scheduler refactoring

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unassigned return value from csg_slot_prog_locked() causes dead error path
--

commit 7eff0da835de3ef4c810c3a0eb36aa6ce221f295
Author: Florent Tomasin <florent.tomasin@arm.com>

drm/panthor: Minor scheduler refactoring

This commit refactors parts of the group scheduling logic into new helper
functions. This change simplifies the addition of the protected mode feature
and removes redundant assignments of csg_slot.

> diff --git a/drivers/gpu/drm/panthor/panthor_sched.c b/drivers/gpu/drm/panthor/panthor_sched.c
> index 0493858e55d76..68425fe561d1a 100644
> --- a/drivers/gpu/drm/panthor/panthor_sched.c
> +++ b/drivers/gpu/drm/panthor/panthor_sched.c
> @@ -2345,12 +2345,85 @@ tick_ctx_cleanup(struct panthor_scheduler *sched,
[ ... ]
> +static int
> +tick_ctx_schedule_group(struct panthor_scheduler *sched,
> +			struct panthor_csg_slots_upd_ctx *upd_ctx,
> +			struct panthor_group *group,
> +			int csg_id, int csg_prio)
> +{
> +	struct panthor_device *ptdev = sched->ptdev;
> +	struct panthor_fw_csg_iface *csg_iface =
> +		panthor_fw_get_csg_iface(ptdev, csg_id);
> +	int ret;
> +
> +	ret = group_bind_locked(group, csg_id);
> +	if (ret)
> +		return ret;
> +
> +	csg_slot_prog_locked(ptdev, csg_id, csg_prio);
> +	if (ret) {
> +		group_unbind_locked(group);
> +		return ret;
> +	}

[Severity: Medium]
Is the return value from csg_slot_prog_locked() intentionally ignored here?

Because `ret` was successfully checked after group_bind_locked(), it is
guaranteed to be 0 at this point. As a result, the `if (ret)` check will
always evaluate to false, making the error handling block dead code.

If csg_slot_prog_locked() fails, the failure is silently ignored and the
function proceeds to queue CSG_STATE_START hardware requests for a slot that
hasn't been successfully programmed, which can lead to hardware timeouts or
undefined behavior.

Should this be `ret = csg_slot_prog_locked(ptdev, csg_id, csg_prio);` instead,
matching the patch's stated intention in the v4 changelog to handle the error
from this function?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012125940.3291951-1-ketil.johnsen@arm.com?part=2