| Message ID | 20201112225147.1672622-2-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kieran Bingham |
| Headers | show |
| Series | rcar-csi2: Update handling of transfer error | expand |
Hi Niklas, On Thu, Nov 12, 2020 at 11:51:44PM +0100, Niklas Söderlund wrote: > Do not attempt to stop the streaming if the stream is not running. > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- > drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c > index 5a5f0e5007478c8d..eae25972ed7df2b6 100644 > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > @@ -1302,6 +1302,11 @@ void rvin_stop_streaming(struct rvin_dev *vin) > > spin_lock_irqsave(&vin->qlock, flags); > > + if (vin->state == STOPPED) { > + spin_unlock_irqrestore(&vin->qlock, flags); > + return; Do I read it right that, in case a double stop is attempted, returning here is not enough as the caller: { rvin_stop_streaming(vin); /* Free scratch buffer. */ dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, vin->scratch_phys); return_unused_buffers(vin, VB2_BUF_STATE_ERROR); } Are the potential double call to dma_free_coherent and the buffer return procedure harmless ? Thanks j > + } > + > vin->state = STOPPING; > > /* Wait until only scratch buffer is used, max 3 interrupts. */ > -- > 2.29.2 >
Hi Jacopo, Thanks for your feedback. On 2020-11-16 17:28:38 +0100, Jacopo Mondi wrote: > Hi Niklas, > > On Thu, Nov 12, 2020 at 11:51:44PM +0100, Niklas Söderlund wrote: > > Do not attempt to stop the streaming if the stream is not running. > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c > > index 5a5f0e5007478c8d..eae25972ed7df2b6 100644 > > --- a/drivers/media/platform/rcar-vin/rcar-dma.c > > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c > > @@ -1302,6 +1302,11 @@ void rvin_stop_streaming(struct rvin_dev *vin) > > > > spin_lock_irqsave(&vin->qlock, flags); > > > > + if (vin->state == STOPPED) { > > + spin_unlock_irqrestore(&vin->qlock, flags); > > + return; > > Do I read it right that, in case a double stop is attempted, returning > here is not enough as the caller: > > { > rvin_stop_streaming(vin); > > /* Free scratch buffer. */ > dma_free_coherent(vin->dev, vin->format.sizeimage, vin->scratch, > vin->scratch_phys); > > return_unused_buffers(vin, VB2_BUF_STATE_ERROR); > } > > Are the potential double call to dma_free_coherent and the buffer > return procedure harmless ? Yes. > > Thanks > j > > > + } > > + > > vin->state = STOPPING; > > > > /* Wait until only scratch buffer is used, max 3 interrupts. */ > > -- > > 2.29.2 > >
diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c index 5a5f0e5007478c8d..eae25972ed7df2b6 100644 --- a/drivers/media/platform/rcar-vin/rcar-dma.c +++ b/drivers/media/platform/rcar-vin/rcar-dma.c @@ -1302,6 +1302,11 @@ void rvin_stop_streaming(struct rvin_dev *vin) spin_lock_irqsave(&vin->qlock, flags); + if (vin->state == STOPPED) { + spin_unlock_irqrestore(&vin->qlock, flags); + return; + } + vin->state = STOPPING; /* Wait until only scratch buffer is used, max 3 interrupts. */
Do not attempt to stop the streaming if the stream is not running. Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/media/platform/rcar-vin/rcar-dma.c | 5 +++++ 1 file changed, 5 insertions(+)