Hi Laurent
Thank you for your feedback
> > + * To be small jitter,
>
> Nitpicking, I would write this "to minimize the jitter".
(snip)
> > + * This code is assuming "used" from 64bit CPU only,
> > + * not from 32bit CPU. But both can compile correctly
>
> Nitpicking again, I would write this "This code only runs on 64-bit
> architectures, the unsigned long type can thus be used for 64-bit
> computation.
> It will still compile without any warning on 32-bit architectures."
I will follow your English ;)
> > + /*
> > + * fvco = fin * P * N / M
> > + * fclkout = fin * N / M / FDPLL
> > + *
> > + * To avoid duplicate calculation, let's use below
> > + *
> > + * finnm = fin * N / M
>
> This is called fout in your diagram above, I would use the same name here.
Oops indeed. I didn't notice
> > + unsigned long finnm = input * (n + 1) / (m + 1);
> > + unsigned long fvco = finnm * 2;
> > +
> > + if (fvco < 2000 || fvco > 4096 * 1000 * 1000U)
> > + continue;
>
> How about
>
> if (fvco < 1000 || fvco > 2048 * 1000 * 1000)
>
> to avoid computing the intermediate fvco variable ?
I think you want to say
- if (fvco < 1000 || fvco > 2048 * 1000 * 1000)
+ if (fout < 1000 || fout > 2048 * 1000 * 1000)
Actually I notcied about this, but I thought it makes
user confuse. Thus, I kept original number.
I'm happy if compiler can adjust it automatically,
if not, I have no objection to modify it but we want to have such comment ?
Because above comment/explain mentions about "fvco", not "fout".
> If you agree with these small changes there's no need to resubmit the patch,
> I'll modify it when applying, and
>
> Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
Thank you for your help
Best regards
---
Kuninori Morimoto
