Re: [PATCH v2 1/2] media: max9271: Ignore busy loop read errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert

On Mon, Nov 08, 2021 at 11:45:58AM +0100, Geert Uytterhoeven wrote:
> Hi Jacopo,
>
> On Thu, Nov 4, 2021 at 12:10 PM Jacopo Mondi <jacopo+renesas@xxxxxxxxxx> wrote:
> > Valid pixel clock detection is performed by spinning on a register read
> > which if repeated too frequently might fail. As the error is not fatal
> > ignore it instead of bailing out to continue spinning until the timeout
> > completion.
> >
> > Also relax the time between bus transactions and slightly increase the
> > wait interval to mitigate the failure risk.
> >
> > Signed-off-by: Jacopo Mondi <jacopo+renesas@xxxxxxxxxx>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> > ---
> >
> > v1->v2:
> > - Do not continue but jump to a label to respect the sleep timout after a
> >   failed read
>
> Thanks for the update!
>
> > --- a/drivers/media/i2c/max9271.c
> > +++ b/drivers/media/i2c/max9271.c
> > @@ -55,7 +55,7 @@ static int max9271_write(struct max9271_device *dev, u8 reg, u8 val)
> >  /*
> >   * max9271_pclk_detect() - Detect valid pixel clock from image sensor
> >   *
> > - * Wait up to 10ms for a valid pixel clock.
> > + * Wait up to 15ms for a valid pixel clock.
> >   *
> >   * Returns 0 for success, < 0 for pixel clock not properly detected
> >   */
> > @@ -64,15 +64,16 @@ static int max9271_pclk_detect(struct max9271_device *dev)
> >         unsigned int i;
> >         int ret;
> >
> > -       for (i = 0; i < 100; i++) {
> > +       for (i = 0; i < 10; i++) {
> >                 ret = max9271_read(dev, 0x15);
> >                 if (ret < 0)
> > -                       return ret;
> > +                       goto skip;
>
> Edgar Dijkstra: Go To Statement Considered Harmful?
>

Dijkstra would be very unpleased reading the kernel error path
handling implementations. But I got your point, we're not in a cleanup
path here =)

> >
> >                 if (ret & MAX9271_PCLKDET)
>
> "if (ret > 0 && (ret & MAX9271_PCLKDET))"?
>

Much better, thanks. I'll resend!

Thanks
   j

> >                         return 0;
> >
> > -               usleep_range(50, 100);
> > +skip:
> > +               usleep_range(1000, 1500);
> >         }
> >
> >         dev_err(&dev->client->dev, "Unable to detect valid pixel clock\n");
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds



[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux