Hi Kieran
On Thu, Nov 04, 2021 at 11:04:57PM +0000, Kieran Bingham wrote:
> Quoting Jacopo Mondi (2021-11-04 11:09:23)
> > 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>
>
> This seems good to me. In your testing did you identify how many
> spins/how long it usually takes before it first detects the pixel clock?
>
> I.e. - was it cutting it close at 10ms, and we should even still extend
> this further? (as the usleep_range means we could still loop this 10 ms)
Thanks for asking, this turned out to be much quicker than expected
with the pclk_clk detected at the first or second attempts all the
times. I would not bother reducing the sleep time though.
>
> Anyway, this looks like a strong improvement.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
Thanks
j
>
> > ---
> >
> > v1->v2:
> > - Do not continue but jump to a label to respect the sleep timout after a
> > failed read
> >
> > Niklas I kept your tag anyway, hope it's ok.
> >
> > Thanks
> > j
> >
> > ---
> > drivers/media/i2c/max9271.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/media/i2c/max9271.c b/drivers/media/i2c/max9271.c
> > index ff86c8c4ea61..aa4add473716 100644
> > --- 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;
> >
> > if (ret & MAX9271_PCLKDET)
> > return 0;
> >
> > - usleep_range(50, 100);
> > +skip:
> > + usleep_range(1000, 1500);
> > }
> >
> > dev_err(&dev->client->dev, "Unable to detect valid pixel clock\n");
> > --
> > 2.33.1
> >
