Re: [PATCH v7 3/4] media: imagination: e5010: Update

[Brandon Brnich <b-brnich@ti.com>]

Hi All,

Will need to resend this series because the subject got cut. Will 
re-send shortly.

On 11/10/26 09:58, Brandon Brnich wrote:
> The PM framework skips the runtime resume hook if the device was never on
> in the first place. This results in a crash in the system resume hook when
> a call to e5010_init_device attempts a write to a powered down device.
> 
> Move e5010_init_device to the resume hook to ensure register write only
> occurs when device is properly powered on.
> 
> e5010_init_device needs to be called after e5010_reset for next
> device_run to have all registers properly reset. Without this call,
> register values are stale. Similarly, e5010_last_context_run should be
> set to NULL in e5010_init_device so the next device_run call does not
> use stale QP values.
> 
> Fixes: a1e294045885 ("media: imagination: Add E5010 JPEG Encoder driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Brandon Brnich <b-brnich@ti.com>
> ---
>   .../platform/imagination/e5010-jpeg-enc.c     | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> index 17ea616ff1795..4e77112b59197 100644
> --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c
> +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c
> @@ -986,6 +986,8 @@ static int e5010_init_device(struct e5010_dev *e5010)
>   {
>   	int ret = 0;
>   
> +	e5010->last_context_run = NULL;
> +
>   	/*TODO: Set MMU in bypass mode until support for the same is added in driver*/
>   	e5010_hw_bypass_mmu(e5010->mmu_base, 1);
>   
> @@ -1488,7 +1490,9 @@ static void e5010_device_run(void *priv)
>   
>   device_busy_err:
>   	e5010_reset(e5010->dev, e5010->core_base, e5010->mmu_base);
> -
> +	ret = e5010_init_device(e5010);
> +	if (ret)
> +		v4l2_err(&e5010->v4l2_dev, "failed to initialize device after reset\n");
>   no_ready_buf_err:
>   	if (s_vb) {
>   		v4l2_m2m_src_buf_remove_by_buf(ctx->fh.m2m_ctx, s_vb);
> @@ -1517,6 +1521,13 @@ static int e5010_runtime_resume(struct device *dev)
>   		return ret;
>   	}
>   
> +	ret = e5010_init_device(e5010);
> +	if (ret) {
> +		dev_err(dev, "Failed to re-enable e5010 device\n");
> +		clk_disable_unprepare(e5010->clk);
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>   
> @@ -1549,12 +1560,6 @@ static int e5010_resume(struct device *dev)
>   	if (ret < 0)
>   		return ret;
>   
> -	ret = e5010_init_device(e5010);
> -	if (ret) {
> -		dev_err(dev, "Failed to re-enable e5010 device\n");
> -		return ret;
> -	}
> -
>   	v4l2_m2m_resume(e5010->m2m_dev);
>   
>   	return ret;