Re: [PATCH v2 2/2] lib: utils/timer: mtimer: add MIPS P8700 compatible

["Benoît Monin" <benoit.monin@bootlin.com>]

Hi Anup,

On Wednesday, 5 November 2025 at 17:11:11 CET, Anup Patel wrote:
> On Mon, Oct 27, 2025 at 6:42 PM Benoît Monin <benoit.monin@bootlin.com> wrote:
> >
> > The MTIMER of the MIPS P8700 is compliant with the ACLINT specification,
> > so add a compatible string for it.
> >
> > In a multi-cluster configuration, there is one MTIMER per cluster, each
> > associated with the HARTS of the cluster. So we do not have a MTIMER
> > with no associated HARTs to use as our reference.
> >
> > To be able to select a reference MTIMER in that case, add a quirk to look
> > up an optional device tree property that indicate which MTIMER to use.
> > For the MIPS P8700, the property is "mips,reference-mtimer".
> >
> > Signed-off-by: Benoît Monin <benoit.monin@bootlin.com>
> > ---
> >  lib/utils/timer/fdt_timer_mtimer.c | 25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/utils/timer/fdt_timer_mtimer.c b/lib/utils/timer/fdt_timer_mtimer.c
> > index e1fd051..e98c99f 100644
> > --- a/lib/utils/timer/fdt_timer_mtimer.c
> > +++ b/lib/utils/timer/fdt_timer_mtimer.c
> > @@ -20,6 +20,7 @@ struct timer_mtimer_quirks {
> >         unsigned int    clint_mtime_offset;
> >         bool            clint_without_mtime;
> >         bool            has_64bit_mmio;
> > +       const char      *dt_ref_prop;
> >  };
> >
> >  struct timer_mtimer_node {
> > @@ -39,6 +40,7 @@ static int timer_mtimer_cold_init(const void *fdt, int nodeoff,
> >         struct aclint_mtimer_data *mt;
> >         const struct timer_mtimer_quirks *quirks = match->data;
> >         bool is_clint = quirks && quirks->is_clint;
> > +       bool is_ref = false;
> >
> >         mtn = sbi_zalloc(sizeof(*mtn));
> >         if (!mtn)
> > @@ -110,13 +112,18 @@ static int timer_mtimer_cold_init(const void *fdt, int nodeoff,
> >         }
> >
> >         /*
> > -        * Select first MTIMER device with no associated HARTs as our
> > -        * reference MTIMER device. This is only a temporary strategy
> > -        * of selecting reference MTIMER device. In future, we might
> > -        * define an optional DT property or some other mechanism to
> > -        * help us select the reference MTIMER device.
> > +        * If we have a DT property to indicate which MTIMER is the reference,
> > +        * select the first MTIMER device that has it. Otherwise, select the
> > +        * first MTIMER device with no associated HARTs as our reference.
> >          */
> > -       if (!mt->hart_count && !mt_reference) {
> > +       if (quirks && quirks->dt_ref_prop) {
> > +               if (fdt_getprop(fdt, nodeoff, quirks->dt_ref_prop, NULL))
> > +                       is_ref = true;
> > +       } else if (!mt->hart_count) {
> > +               is_ref = true;
> > +       }
> > +
> > +       if (is_ref && !mt_reference) {
> >                 mt_reference = mt;
> >                 /*
> >                  * Set reference for already propbed MTIMER devices
> > @@ -152,6 +159,10 @@ static const struct timer_mtimer_quirks thead_aclint_quirks = {
> >         .has_64bit_mmio         = false,
> >  };
> >
> > +static const struct timer_mtimer_quirks mips_aclint_quirks = {
> > +       .dt_ref_prop            = "mips,reference-mtimer",
> > +};
> > +
> 
> I think this DT property can be useful to other platforms as well.
> How about "riscv,reference-mtimer" as a DT property ? This way
> you don't need to make it as MIPS quirk.
> 
ACLINT is not a ratified specification, is it okay to use the "riscv,"
prefix? That's the reason I went with a vendor prefix.

[...]

Best regards,
-- 
Benoît Monin, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com




-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi