diff mbox series

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

Message ID 20220824155113.286730-6-jean-philippe@linaro.org (mailing list archive)
State New, archived
Headers show
Series hw/arm/virt: Fix dt-schema warnings | expand

Commit Message

Jean-Philippe Brucker Aug. 24, 2022, 3:51 p.m. UTC
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(-)

Comments

Peter Maydell Aug. 24, 2022, 7:40 p.m. UTC | #1
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" ?

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.

thanks
-- PMM
Jean-Philippe Brucker Sept. 1, 2022, 2:58 p.m. UTC | #2
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
diff mbox series

Patch

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 {