Hi, Daniel > Subject: Re: [PATCH V3 1/5] clocksource: timer-of: Support getting clock > frequency from DT > > > Hi Anson, > > thanks for taking care of adding the clock-frequency handling in the timer-of.
Sure. > > On 28/06/2019 05:30, [email protected] wrote: > > From: Anson Huang <[email protected]> > > > > More and more platforms use platform driver model for clock driver, so > > the clock driver is NOT ready during timer initialization phase, it > > will cause timer initialization failed. > > > > To support those platforms with upper scenario, introducing a new flag > > TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with > > TIMER_OF_CLOCK flag to support getting timer clock frequency from DT, > > then of_clk operations can be skipped. > > > > User needs to select either TIMER_OF_CLOCK_FREQUENCY or > TIMER_OF_CLOCK > > flag if want to use timer-of driver to initialize the clock rate, and > > the corresponding clock name or property name needs to be specified. > > > > Signed-off-by: Anson Huang <[email protected]> > > --- > > New patch: > > - Add new flag of TIMER_OF_CLOCK_FREQUENCY, mutually exclusive > with TIMER_OF_CLOCK, to support > > getting clock frequency from DT directly; > > - Add prop_name to of_timer_clk structure, if using > TIMER_OF_CLOCK_FREQUENCY flag, user needs > > to pass the property name for timer-of driver to get clock frequency > from DT, this is to avoid > > the couple of timer-of driver and DT, so timer-of driver does NOT > use a fixed property name. > > --- > > drivers/clocksource/timer-of.c | 23 +++++++++++++++++++++++ > > drivers/clocksource/timer-of.h | 8 +++++--- > > 2 files changed, 28 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/clocksource/timer-of.c > > b/drivers/clocksource/timer-of.c index 8054228..c91a8b6 100644 > > --- a/drivers/clocksource/timer-of.c > > +++ b/drivers/clocksource/timer-of.c > > @@ -161,6 +161,21 @@ static __init int timer_of_base_init(struct > device_node *np, > > return 0; > > } > > > > +static __init int timer_of_clk_frequency_init(struct device_node *np, > > + struct of_timer_clk *of_clk) { > > + int ret; > > + u32 rate; > > + > > + ret = of_property_read_u32(np, of_clk->prop_name, &rate); > > + if (!ret) { > > + of_clk->rate = rate; > > + of_clk->period = DIV_ROUND_UP(rate, HZ); > > + } > > + > > + return ret; > > +} > > + > > int __init timer_of_init(struct device_node *np, struct timer_of *to) > > { > > int ret = -EINVAL; > > @@ -178,6 +193,11 @@ int __init timer_of_init(struct device_node *np, > struct timer_of *to) > > if (ret) > > goto out_fail; > > flags |= TIMER_OF_CLOCK; > > + } else if (to->flags & TIMER_OF_CLOCK_FREQUENCY) { > > + ret = timer_of_clk_frequency_init(np, &to->of_clk); > > + if (ret) > > + goto out_fail; > > + flags |= TIMER_OF_CLOCK_FREQUENCY; > > } > > /* Pre-condition */ > > if (to->flags & (TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY)) > return -EINVAL; > > [ ... ] > > if (to->flags & TIMER_OF_CLOCK) { > } > > if (to->flags & TIMER_OF_CLOCK_FREQUENCY) { } > Ah, make sense, they are exclusive, will add it in next version. > > if (to->flags & TIMER_OF_IRQ) { > > @@ -201,6 +221,9 @@ int __init timer_of_init(struct device_node *np, > struct timer_of *to) > > if (flags & TIMER_OF_CLOCK) > > timer_of_clk_exit(&to->of_clk); > > > > + if (flags & TIMER_OF_CLOCK_FREQUENCY) > > + to->of_clk.rate = 0; > > + > > if (flags & TIMER_OF_BASE) > > timer_of_base_exit(&to->of_base); > > return ret; > > diff --git a/drivers/clocksource/timer-of.h > > b/drivers/clocksource/timer-of.h index a5478f3..f1a083e 100644 > > --- a/drivers/clocksource/timer-of.h > > +++ b/drivers/clocksource/timer-of.h > > @@ -4,9 +4,10 @@ > > > > #include <linux/clockchips.h> > > > > -#define TIMER_OF_BASE 0x1 > > -#define TIMER_OF_CLOCK 0x2 > > -#define TIMER_OF_IRQ 0x4 > > +#define TIMER_OF_BASE 0x1 > > +#define TIMER_OF_CLOCK 0x2 > > +#define TIMER_OF_IRQ 0x4 > > +#define TIMER_OF_CLOCK_FREQUENCY 0x8 > > > > struct of_timer_irq { > > int irq; > > @@ -26,6 +27,7 @@ struct of_timer_base { struct of_timer_clk { > > struct clk *clk; > > const char *name; > > + const char *prop_name; > > For the moment, keep it hardcoded with "clock-frequency" directly in the > function. OK, then I will NOT add any dt-binding for this property. The reason to use prop_name instead of hardcode is I don't want to create a binding doc just for this property. Thanks, Anson.
