| Message ID | 20250502-qcom-iris-hevc-vp9-v3-5-552158a10a7d@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3,01/23] media: iris: Skip destroying internal buffer if not dequeued | expand |
On 01/05/2025 20:13, Dikshita Agarwal wrote: > - if (core->state == IRIS_CORE_ERROR) > + if (core->state == IRIS_CORE_ERROR || core->state == IRIS_CORE_DEINIT) > return -EINVAL; Instead of checking for 2/3 of the states why not just check for the 1/3 ? enum iris_core_state { IRIS_CORE_DEINIT, IRIS_CORE_INIT, IRIS_CORE_ERROR, }; if (core->state != IRIS_CORE_INIT) return -EINVAL; Cleaner and more explicit - declaring the state you must be in, as opposed to a list of states you should not be in. Assuming you accept that suggested change: Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
On 5/2/25 2:22 PM, Bryan O'Donoghue wrote: > On 01/05/2025 20:13, Dikshita Agarwal wrote: >> - if (core->state == IRIS_CORE_ERROR) >> + if (core->state == IRIS_CORE_ERROR || core->state == IRIS_CORE_DEINIT) >> return -EINVAL; > > Instead of checking for 2/3 of the states why not just check for the 1/3 ? > > enum iris_core_state { > IRIS_CORE_DEINIT, > IRIS_CORE_INIT, > IRIS_CORE_ERROR, > }; > > if (core->state != IRIS_CORE_INIT) > return -EINVAL; > > Cleaner and more explicit - declaring the state you must be in, as opposed to a list of states you should not be in. Being explicit in state machines helps maintainability - if we get e.g. IRIS_CORE_LIGHT_SLEEP down the line, this could easily fail Konrad
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_queue.c b/drivers/media/platform/qcom/iris/iris_hfi_queue.c index fac7df0c4d1a..221dcd09e1e1 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_queue.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_queue.c @@ -113,7 +113,7 @@ int iris_hfi_queue_cmd_write_locked(struct iris_core *core, void *pkt, u32 pkt_s { struct iris_iface_q_info *q_info = &core->command_queue; - if (core->state == IRIS_CORE_ERROR) + if (core->state == IRIS_CORE_ERROR || core->state == IRIS_CORE_DEINIT) return -EINVAL; if (!iris_hfi_queue_write(q_info, pkt, pkt_size)) {
The current check only considers the core error state before allowing writes to the HFI queues. However, the core can also transition to the deinit state due to a system error triggered by the response thread. In such cases, writing to the HFI queues should not be allowed. Fix this by adding a check for the core deinit state, ensuring that writes are rejected when core is not in a valid state. Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Acked-by: Vikash Garodia <quic_vgarodia@quicinc.com> Signed-off-by: Dikshita Agarwal <quic_dikshita@quicinc.com> --- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)