Hi Dafna,
On Tue, Sep 22, 2020 at 01:33:59PM +0200, Dafna Hirschfeld wrote:
> The field stats->is_streaming is written in 'start_streaming' callback
> without the stats->lock protection.
> The isr might run together with the callback so 'spin_lock_irq'
> should be used.
>
> Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@xxxxxxxxxxxxx>
> Acked-by: Helen Koike <helen.koike@xxxxxxxxxxxxx>
> ---
> drivers/staging/media/rkisp1/rkisp1-stats.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Thank you for the patch. Please see my comments inline.
> diff --git a/drivers/staging/media/rkisp1/rkisp1-stats.c b/drivers/staging/media/rkisp1/rkisp1-stats.c
> index 6aa18d970f2b..51c64f75fe29 100644
> --- a/drivers/staging/media/rkisp1/rkisp1-stats.c
> +++ b/drivers/staging/media/rkisp1/rkisp1-stats.c
> @@ -157,7 +157,9 @@ rkisp1_stats_vb2_start_streaming(struct vb2_queue *queue, unsigned int count)
> {
> struct rkisp1_stats *stats = queue->drv_priv;
>
> + spin_lock_irq(&stats->lock);
> stats->is_streaming = true;
Do we need this separate stream? This is a similar note to the
parameters patch - it should be possible to handle this based on the
presence of the buffers in the list, without a custom flag.
Best regards,
Tomasz
