Hi Matsuoka-san,
Thank you for the patch,
On 11/01/2019 15:43, Kieran Bingham wrote:>
> From: Koji Matsuoka <koji.matsuoka.xm@xxxxxxxxxxx>
>
> By Documentation/timers/timers-howto.txt, when waiting 20ms from 10us,
> it is correct to use usleep_range. this patch corrects it.
>
> Signed-off-by: Koji Matsuoka <koji.matsuoka.xm@xxxxxxxxxxx>
> (cherry picked from horms/renesas-bsp commit
af0cdba377bc8a784cdae6a77fb7a822cebc7083)
> Signed-off-by: Steve Longerbeam <steve_longerbeam@xxxxxxxxxx>
> ---
> drivers/media/i2c/adv748x/adv748x-core.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/adv748x/adv748x-core.c
b/drivers/media/i2c/adv748x/adv748x-core.c
> index 64eb1bfda581..097e5c3a8e7e 100644
> --- a/drivers/media/i2c/adv748x/adv748x-core.c
> +++ b/drivers/media/i2c/adv748x/adv748x-core.c
> @@ -273,7 +273,8 @@ static int adv748x_write_regs(struct adv748x_state
*state,
>
> while (regs->page != ADV748X_PAGE_EOR) {
> if (regs->page == ADV748X_PAGE_WAIT) {
> - msleep(regs->value);
> + usleep_range(regs->value * 1000,
> + (regs->value * 1000) + 1000);
I might propose simplifying this by moving the duplicated multiplication
to a local variable:
if (regs->page == ADV748X_PAGE_WAIT) {
unsigned int usec = regs->value * 1000;
usleep_range(usec, usec + 1000);
} ...
There are three other usages of usleep_range in the driver which utilise
a 500 uSec range rather than 1000. However I don't see any real reason
to object to a range of 1000 and it will help the scheduling to be a bit
more relaxed.
In fact, It seems there is only a single usage of ADV748X_PAGE_WAIT ...
to perform a sw-reset, and that itself only has 3 register writes.
As part of the ongoing aim to remove the register tables from this code
- I think I will drop this patch and submit an alternative which
implements a function to handle the sw_reset, and removes all references
to ADV748X_PAGE_WAIT.
The PAGE_WAIT feels like a bit of an ugly hack due to the tables and I
think we can let it go.
> } else {
> ret = adv748x_write(state, regs->page, regs->reg,
> regs->value);
>
Regards
Kieran
