Re: [PATCH 05/10] hw/arm/virt: Fix devicetree warning about the timer node

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Wed, Aug 24, 2022 at 08:40:21PM +0100, Peter Maydell wrote:
> On Wed, 24 Aug 2022 at 16:51, Jean-Philippe Brucker
> <jean-philippe@linaro.org> wrote:
> >
> > The compatible property of the Arm timer should contain either
> > "arm,armv7-timer" or "arm,armv8-timer", not both.
> >
> >   timer: compatible: 'oneOf' conditional failed, one must be fixed:
> >         ['arm,armv8-timer', 'arm,armv7-timer'] is too long
> >   From schema: linux/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
> >
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > ---
> >  hw/arm/virt.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index ca5d213895..5935f32a44 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -344,7 +344,7 @@ static void fdt_add_timer_nodes(const VirtMachineState *vms)
> >
> >      armcpu = ARM_CPU(qemu_get_cpu(0));
> >      if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
> > -        const char compat[] = "arm,armv8-timer\0arm,armv7-timer";
> > +        const char compat[] = "arm,armv8-timer";
> >          qemu_fdt_setprop(ms->fdt, "/timer", "compatible",
> >                           compat, sizeof(compat));
> >      } else {
> 
> Are we really sure there are no existing guests out there that are
> looking for this device under "armv7-timer" ?

It's highly unlikely. It would take for example a 32-bit Linux from before
2013 or a 32-bit FreeBSD from before 2015, running on a machine with
ARM_FEATURE_V8. But I can't say for sure that no one is running such a
config, so I'll ask about relaxing the binding.

> 
> This used to be valid DT before Linux kernel commit 4d2bb3e65035954,
> which changed from "should at least contain one of" to requiring
> exactly one-of, and that was only in 2018.

Yes the text bindings weren't always exact.

Thanks,
Jean