Re: [PATCH 04/10] media: microchip-isc: disable histogram and flush AWB work on stop

[Eugen Hristev <ehristev@kernel.org>]

On 6/16/26 14:50, Balakrishnan Sambath wrote:
> isc_stop_streaming() masked the DMA done interrupt but left the
> histogram enabled, so a HISDONE that fired just before the stop could
> still queue isc_awb_work() after pm_runtime_put_sync() gated the clocks.
> isc_awb_work() reads the histogram registers in isc_hist_count() before
> taking its own PM reference, so the access faults on the suspended
> device.
> 
> Disable the histogram and flush the work before dropping the PM
> reference, mirroring the start_streaming error path.
> 

In here you reference some error path that you mirror, but you add it in
the next commit in which you say you mirror this exact commit :/
Circular mirroring of paths which initially did not exist.
I would squash this commit with the next one and show the facts as they
are : stop histogram and work queue in different stop/error scenarios,
no more mirroring.

Eugen

> Fixes: 91b4e487b0c6 ("media: microchip: add ISC driver as Microchip ISC")
> Cc: stable@vger.kernel.org
> Signed-off-by: Balakrishnan Sambath <balakrishnan.s@microchip.com>
> ---
>  drivers/media/platform/microchip/microchip-isc-base.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/platform/microchip/microchip-isc-base.c b/drivers/media/platform/microchip/microchip-isc-base.c
> index 3245dd7cb980..8f255a4c4e7a 100644
> --- a/drivers/media/platform/microchip/microchip-isc-base.c
> +++ b/drivers/media/platform/microchip/microchip-isc-base.c
> @@ -425,9 +425,13 @@ static void isc_stop_streaming(struct vb2_queue *vq)
>  	/* Disable DMA interrupt */
>  	regmap_write(isc->regmap, ISC_INTDIS, ISC_INT_DDONE);
>  
> +	isc_set_histogram(isc, false);
> +
>  	/* let a running IRQ handler finish before the clock is disabled */
>  	synchronize_irq(isc->irq);
>  
> +	cancel_work_sync(&isc->awb_work);
> +
>  	pm_runtime_put_sync(isc->dev);
>  
>  	/* Disable stream on the sub device */
>