Hi Kieran,
Thanks for your feedback.
On 2018-03-13 17:42:25 +0100, Kieran Bingham wrote:
> Hi Niklas,
>
> Thanks for the patch series :) - I've been looking forward to seeing this one !
>
> On 10/03/18 01:09, Niklas Söderlund wrote:
> > This is an error from when the driver where converted from soc-camera.
> > There is absolutely no gain to check the state variable two times to be
> > extra sure if the hardware is stopped.
>
> I'll not say this isn't a redundant check ... but isn't the check against two
> different states, and thus the remaining check doesn't actually catch the case
> now where state == STOPPED ?
Thanks for noticing this, you are correct. I think I need to refresh my
glasses subscription after missing this :-)
>
> (Perhaps state != RUNNING would be better ?, but I haven't checked the rest of
> the code)
I will respin this in a v2 and either use state != RUNNING or at least
combine the two checks to prevent future embarrassing mistakes like
this.
>
> --
> Kieran
>
>
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> > drivers/media/platform/rcar-vin/rcar-dma.c | 6 ------
> > 1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/media/platform/rcar-vin/rcar-dma.c b/drivers/media/platform/rcar-vin/rcar-dma.c
> > index 23fdff7a7370842e..b4be75d5009080f7 100644
> > --- a/drivers/media/platform/rcar-vin/rcar-dma.c
> > +++ b/drivers/media/platform/rcar-vin/rcar-dma.c
> > @@ -916,12 +916,6 @@ static irqreturn_t rvin_irq(int irq, void *data)
> > rvin_ack_interrupt(vin);
> > handled = 1;
> >
> > - /* Nothing to do if capture status is 'STOPPED' */
> > - if (vin->state == STOPPED) {
> > - vin_dbg(vin, "IRQ while state stopped\n");
> > - goto done;
> > - }
> > -
> > /* Nothing to do if capture status is 'STOPPING' */
> > if (vin->state == STOPPING) {
> > vin_dbg(vin, "IRQ while state stopping\n");
> >
--
Regards,
Niklas Söderlund
