2022-09-16 13:37:30

by Jean-Philippe Brucker

[permalink] [raw]
Subject: [PATCH] dt-bindings: timer: Add QEMU compatible strings

QEMU uses both "arm,armv8-timer" and "arm,armv7-timer" as compatible
string. Although it is unlikely that any guest relies on this, we can't
be certain of that. Therefore, add these to the schema. Clean up the
compatible list a little while at it.

Signed-off-by: Jean-Philippe Brucker <[email protected]>
---
.../devicetree/bindings/timer/arm,arch_timer.yaml | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
index df8ce87fd54b..fa03fc0fe320 100644
--- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
+++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
@@ -22,16 +22,17 @@ properties:
compatible:
oneOf:
- items:
- - enum:
- - arm,cortex-a15-timer
- - enum:
- - arm,armv7-timer
+ - const: arm,cortex-a15-timer
+ - const: arm,armv7-timer
- items:
- enum:
- arm,armv7-timer
- - items:
- - enum:
- arm,armv8-timer
+ - description: QEMU uses both compatible strings.
+ This format is deprecated.
+ items:
+ - const: arm,armv8-timer
+ - const: arm,armv7-timer

interrupts:
minItems: 1
--
2.37.3


2022-09-17 17:02:48

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: timer: Add QEMU compatible strings

On 16/09/2022 14:30, Jean-Philippe Brucker wrote:
> QEMU uses both "arm,armv8-timer" and "arm,armv7-timer" as compatible
> string. Although it is unlikely that any guest relies on this, we can't
> be certain of that. Therefore, add these to the schema. Clean up the
> compatible list a little while at it.
>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>

I guess you wanted to say QEMU uses "arm,armv8-timer" followed by
"arm,armv7-timer", because otherwise I would understand it that either
that or that. Anyway, is it a valid (virtualized) hardware? Is ARMv8
timer really, really compatible with ARMv7 one?

I don't think we should document invalid setups out-of-tree, just
because they are there, and something like this was also expressed by Rob:
https://lore.kernel.org/all/[email protected]/

Best regards,
Krzysztof

2022-09-17 20:19:13

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: timer: Add QEMU compatible strings

On Sat, 17 Sep 2022 17:51:20 +0100,
Krzysztof Kozlowski <[email protected]> wrote:
>
> On 16/09/2022 14:30, Jean-Philippe Brucker wrote:
> > QEMU uses both "arm,armv8-timer" and "arm,armv7-timer" as compatible
> > string. Although it is unlikely that any guest relies on this, we can't
> > be certain of that. Therefore, add these to the schema. Clean up the
> > compatible list a little while at it.
> >
> > Signed-off-by: Jean-Philippe Brucker <[email protected]>
>
> I guess you wanted to say QEMU uses "arm,armv8-timer" followed by
> "arm,armv7-timer", because otherwise I would understand it that either
> that or that. Anyway, is it a valid (virtualized) hardware? Is ARMv8
> timer really, really compatible with ARMv7 one?

Yes. There isn't a shred of difference between the two in the earlier
revisions of the ARMv8 architecture, and none of the differences
introduced in later revisions are exposed to DT anyway.

> I don't think we should document invalid setups out-of-tree, just
> because they are there, and something like this was also expressed by Rob:
> https://lore.kernel.org/all/[email protected]/

This is, on the contrary, something that is perfectly valid. For
example, a system running a 32bit OS on a 64bit system is perfectly
entitled to expose both (v8 because that's what the HW is, v7 because
that's what the OS is the most likely to understand).

You may find it odd, but that:

- expresses something that is actually required

- is what I, as the original author of this binding, have always
considered valid

- has been valid for a long time (10+ years) before you decided it
suddenly wasn't

I understand that the "DT police" has high standards, but this has
been around for much longer, and it isn't because the conversion to
schema is imperfect that you can rewrite history.

As for the patch, I'd remove the QEMU reference and the deprecation.
This format is perfectly allowed, and is in use in most VMMs out
there. Yes, DT is an ABI.

Thanks,

M.

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

2022-09-18 09:07:16

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: timer: Add QEMU compatible strings

On 17/09/2022 19:47, Marc Zyngier wrote:
> On Sat, 17 Sep 2022 17:51:20 +0100,
> Krzysztof Kozlowski <[email protected]> wrote:
>>
>> On 16/09/2022 14:30, Jean-Philippe Brucker wrote:
>>> QEMU uses both "arm,armv8-timer" and "arm,armv7-timer" as compatible
>>> string. Although it is unlikely that any guest relies on this, we can't
>>> be certain of that. Therefore, add these to the schema. Clean up the
>>> compatible list a little while at it.
>>>
>>> Signed-off-by: Jean-Philippe Brucker <[email protected]>
>>
>> I guess you wanted to say QEMU uses "arm,armv8-timer" followed by
>> "arm,armv7-timer", because otherwise I would understand it that either
>> that or that. Anyway, is it a valid (virtualized) hardware? Is ARMv8
>> timer really, really compatible with ARMv7 one?
>
> Yes. There isn't a shred of difference between the two in the earlier
> revisions of the ARMv8 architecture, and none of the differences
> introduced in later revisions are exposed to DT anyway.
>
>> I don't think we should document invalid setups out-of-tree, just
>> because they are there, and something like this was also expressed by Rob:
>> https://lore.kernel.org/all/[email protected]/
>
> This is, on the contrary, something that is perfectly valid. For
> example, a system running a 32bit OS on a 64bit system is perfectly
> entitled to expose both (v8 because that's what the HW is, v7 because
> that's what the OS is the most likely to understand).
>
> You may find it odd, but that:
>
> - expresses something that is actually required
>
> - is what I, as the original author of this binding, have always
> considered valid
>
> - has been valid for a long time (10+ years) before you decided it
> suddenly wasn't
>
> I understand that the "DT police" has high standards, but this has
> been around for much longer, and it isn't because the conversion to
> schema is imperfect that you can rewrite history.
>
> As for the patch, I'd remove the QEMU reference and the deprecation.
> This format is perfectly allowed, and is in use in most VMMs out
> there. Yes, DT is an ABI.

Thanks for the explanation, actually enough was to say that it is
perfectly valid combination describing hardware. :)


Acked-by: Krzysztof Kozlowski <[email protected]>


Best regards,
Krzysztof

2022-09-18 09:51:17

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: timer: Add QEMU compatible strings

On Sun, 18 Sep 2022 09:50:58 +0100,
Krzysztof Kozlowski <[email protected]> wrote:
>
> On 17/09/2022 19:47, Marc Zyngier wrote:
> > On Sat, 17 Sep 2022 17:51:20 +0100,
> > Krzysztof Kozlowski <[email protected]> wrote:
> >>
> >> On 16/09/2022 14:30, Jean-Philippe Brucker wrote:
> >>> QEMU uses both "arm,armv8-timer" and "arm,armv7-timer" as compatible
> >>> string. Although it is unlikely that any guest relies on this, we can't
> >>> be certain of that. Therefore, add these to the schema. Clean up the
> >>> compatible list a little while at it.
> >>>
> >>> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> >>
> >> I guess you wanted to say QEMU uses "arm,armv8-timer" followed by
> >> "arm,armv7-timer", because otherwise I would understand it that either
> >> that or that. Anyway, is it a valid (virtualized) hardware? Is ARMv8
> >> timer really, really compatible with ARMv7 one?
> >
> > Yes. There isn't a shred of difference between the two in the earlier
> > revisions of the ARMv8 architecture, and none of the differences
> > introduced in later revisions are exposed to DT anyway.
> >
> >> I don't think we should document invalid setups out-of-tree, just
> >> because they are there, and something like this was also expressed by Rob:
> >> https://lore.kernel.org/all/[email protected]/
> >
> > This is, on the contrary, something that is perfectly valid. For
> > example, a system running a 32bit OS on a 64bit system is perfectly
> > entitled to expose both (v8 because that's what the HW is, v7 because
> > that's what the OS is the most likely to understand).
> >
> > You may find it odd, but that:
> >
> > - expresses something that is actually required
> >
> > - is what I, as the original author of this binding, have always
> > considered valid
> >
> > - has been valid for a long time (10+ years) before you decided it
> > suddenly wasn't
> >
> > I understand that the "DT police" has high standards, but this has
> > been around for much longer, and it isn't because the conversion to
> > schema is imperfect that you can rewrite history.
> >
> > As for the patch, I'd remove the QEMU reference and the deprecation.
> > This format is perfectly allowed, and is in use in most VMMs out
> > there. Yes, DT is an ABI.
>
> Thanks for the explanation, actually enough was to say that it is
> perfectly valid combination describing hardware. :)

Assuming that the original posting was wrong without understanding the
context is what triggered it. It is absolutely fine not to know these
things, but in this case *do ask questions* rather than dismiss the
patch off the bat based of third hand conclusions.

Now at least you have the full picture.

M.

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

2022-09-22 16:32:37

by Jean-Philippe Brucker

[permalink] [raw]
Subject: Re: [PATCH] dt-bindings: timer: Add QEMU compatible strings

On Sun, Sep 18, 2022 at 09:50:58AM +0100, Krzysztof Kozlowski wrote:
> > As for the patch, I'd remove the QEMU reference and the deprecation.
> > This format is perfectly allowed, and is in use in most VMMs out
> > there. Yes, DT is an ABI.
>
> Thanks for the explanation, actually enough was to say that it is
> perfectly valid combination describing hardware. :)
>
>
> Acked-by: Krzysztof Kozlowski <[email protected]>

Thanks Krzysztof, I'll rework the commit message and resend

Thanks,
Jean