On 24/02/16 11:32AM, Vinod Koul wrote: > On 15-02-24, 17:43, Andy Shevchenko wrote: > > On Thu, Feb 15, 2024 at 04:17:53PM +0100, Thomas Richard wrote: > > > The wiz_clock_init() function mixes probe and hardware configuration. > > > Rename the wiz_clock_init() to wiz_clock_probe() and move the hardware > > > configuration part in a new function named wiz_clock_init(). > > > > > > This hardware configuration sequence must be called during the resume > > > stage of the driver. > > > > ... > > > > (Side note, as this can be done later) > > > > > if (rate >= 100000000) > > > > > + if (rate >= 100000000) > > > > > + if (rate >= 100000000) > > > > I would make local definition and use it, we may get the global one as there > > are users. > > > > #define HZ_PER_GHZ 1000000000UL > > Better to define as: > #define HZ_PER_GHZ 1 * GIGA The variable "rate" is being compared against 100 MHz and not 1 GHz. The driver already has the following macros defined: #define REF_CLK_19_2MHZ 19200000 #define REF_CLK_25MHZ 25000000 #define REF_CLK_100MHZ 100000000 #define REF_CLK_156_25MHZ 156250000 So would it be acceptable to change it to: if (rate >= REF_CLK_100MHZ) instead? Regards, Siddharth.