On Wed, Nov 11, 2020 at 09:28:34AM +0000, Wan Mohamad, Wan Ahmad Zainie wrote: > > From: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> > > Sent: Monday, November 9, 2020 7:41 PM > > To: Wan Mohamad, Wan Ahmad Zainie > > On Mon, Nov 09, 2020 at 11:16:54AM +0800, Wan Ahmad Zainie wrote: ... > > > + usleep_range(30, 50); > > > > Why 30-50? > > I take this value from boot firmware. > There is a delay of 30us after clearing IDDQ_enable bit. > I believe the purpose is to ensure all analog blocks are powered up. Then put it into comment. ... > > > + usleep_range(20, 50); > > > > Why these numbers? > > In Keem Bay data book, under USB initialization section, > there is step that there must be a minimum 20us wait > after clock enable, before bringing PHYs out of reset. > > 50 is the value that I picked randomly. Is usleep_range(20, 20) > Better? No, the better as I told you already few times is to comment "why?" these numbers. Above can be like: "According to datasheet this step requires 20us wait..." ... > > > + usleep_range(2, 10); > > > > Ditto. > > Under the same section above, there is a step for 2us wait. > I believe it is for register write to go through. Ditto. > > > > ... > > > > > + usleep_range(20, 50); > > > > Ditto. > > Under the same section above, there is a step to wait 20us > after setting SRAM load bit, before release the controller > reset. > > I will add comment for those 4 delay above. Yes, please. ... > Before I proceed with v3, I would like to know if I should > use udelay(), instead of usleep_range()? > I refer to https://www.kernel.org/doc/Documentation/timers/timers-howto.txt. You should know your code better than me. That howto is clear about when of which API calls can be used. -- With Best Regards, Andy Shevchenko
