Re: [PATCH v2 13/16] timer: Add loongarch_timer driver

[Yao Zi <me@ziyao.cc>]

On Thu, Jul 02, 2026 at 11:27:04AM +0100, Simon Glass wrote:
> Hi Yao,
> 
> On 2026-07-01T11:17:53, Yao Zi <me@ziyao.cc> wrote:
> > timer: Add loongarch_timer driver
> >
> > Implement a timer driver for LoongArch architecture driver.
> >
> > It's synced in hardware for every core in a system, and frequency
> > information can be gathered from CPUCFG instruction.
> >
> > It is not described in fdt, thus I have to declare a DRVINFO
> > for it.
> >
> > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Signed-off-by: Yao Zi <me@ziyao.cc>
> >
> > drivers/timer/Kconfig           |   8 +++
> >  drivers/timer/Makefile          |   1 +
> >  drivers/timer/loongarch_timer.c | 112 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 121 insertions(+)
> 
> > diff --git a/drivers/timer/loongarch_timer.c b/drivers/timer/loongarch_timer.c
> > @@ -0,0 +1,112 @@
> > +#include <dm.h>
> > +#include <errno.h>
> > +#include <timer.h>
> > +#include <asm/loongarch.h>
> 
> lldiv() is used below but div64.h is not included. Please add it
> (see riscv_timer.c).
> 
> > diff --git a/drivers/timer/loongarch_timer.c b/drivers/timer/loongarch_timer.c
> > @@ -0,0 +1,112 @@
> > +static int loongarch_timer_bind(struct udevice *dev)
> > +{
> > +     struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
> > +     u32 rate;
> > +
> > +     rate = loongarch_timer_get_freq_cpucfg();
> > +     uc_priv->clock_rate = rate;
> > +
> > +     return 0;
> > +}
> 
> This is wired up as .probe below, so please rename to
> loongarch_timer_probe(). Also, if loongarch_timer_get_freq_cpucfg()
> returns 0 (LLFTP not present, or CPUCFG5 reads back zero) the driver
> still probes with clock_rate=0, which then trips a divide-by-zero
> in timer_get_boot_us(). Perhaps return -EINVAL in that case?

In case of missing LLFTP, I think bailing out would be correct. I'd
prefer to return -ENODEV since LLFTP represents support for constant
frequency counter and timer, when it's missing we don't even have a
stable timer to poke at all.

And CPUCFG5 represents "the corresponding multiplication factor of the
clock used by the timer", thus I don't think CPUCFG5 = 0 is a reasonable
hardware implementation, and prefer to ignore this case.

...

> Regards,
> Simon

Regards,
Yao Zi