Re: [PATCH v2 04/17] dt-bindings: timer: arm,arch_timer: Fix requirements for interrupt description

[Marc Zyngier <maz@kernel.org>]

On Thu, 14 May 2026 22:37:07 +0100,
sashiko-bot@kernel.org wrote:
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Critical] The schema permits omitting the EL3 secure timer but fails to make `interrupt-names` a required property, which allows configurations that fatally misroute interrupts and hang the system.
> - [Medium] The static `items` array with `minItems: 2` structurally fails to enforce the stated architectural rules and causes positional misalignment for valid configurations.
> - [Medium] The commit incorrectly asserts that the EL1 virtual timer IRQ is 'always required', which misrepresents ARMv7 hardware and contradicts existing device trees.
> --
> 
> commit 01733bbba71a7bef01e5c0820740d8ab21f684f8
> Author: Marc Zyngier <maz@kernel.org>
> 
> dt-bindings: timer: arm,arch_timer: Fix requirements for interrupt description
> 
> This commit updates the arm,arch_timer binding to clarify the requirements
> for timer interrupts based on the implemented architecture. It raises the
> minimum number of interrupts to two and uses architectural terminology.
> 
> > Follow the architecture by making it explicit that:
> > - the EL1 secure timer irq is required if EL3 is implemented
> > - the EL1 physical timer irq is always required
> > - the EL1 virtual timer irq is always required
> 
> Is this description architecturally accurate for ARMv7?
> 
> In ARMv7, Virtualization Extensions are optional, so hardware lacking these
> extensions does not implement a virtual timer. Existing device trees, such
> as arch/arm/boot/dts/rockchip/rv1108.dtsi, accurately reflect this by
> providing exactly two physical interrupts (secure and non-secure) and
> completely omitting the virtual timer.

This *is* valid for all known implementations of ARMv7, as the
architected timer is present only on implementations that have the
virtualisation extensions.

> Establishing the EL1 virtual timer as always required seems to contradict
> these valid hardware configurations.

That's because that DT was broken the first place.

> 
> > diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
> > --- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
> > +++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
> 
> [ ... ]
> 
> > @@ -33,13 +28,13 @@ properties:
> >            - const: arm,armv7-timer
> >  
> >    interrupts:
> > -    minItems: 1
> > +    minItems: 2
> >      items:
> > -      - description: secure timer irq
> > -      - description: non-secure timer irq
> > -      - description: virtual timer irq
> > -      - description: hypervisor timer irq
> > -      - description: hypervisor virtual timer irq
> > +      - description: EL1 secure physical timer irq, if EL3 is implemented
> > +      - description: EL1 non-secure physical timer irq
> > +      - description: EL1 virtual timer irq
> 
> Will this static array structure cause positional misalignment and fail to
> enforce the stated requirements?

It always has been a problem, and this is worked around on the sole
platform affected by this (APM XGene) by having a dummy interrupt for
the secure timer.

We could add a separate requirement that interrupt names must be
provided if secure is not implemented, but I have no idea how to
specify this.

	M.

-- 
Without deviation from the norm, progress is not possible.