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.pinchart@xxxxxxxxxxxxxxxx> Thank you for your help Best regards --- Kuninori Morimoto
