2022-02-24 14:04:28

by Hector Martin

[permalink] [raw]
Subject: [PATCH v2 2/7] dt-bindings: interrupt-controller: apple,aic2: New binding for AICv2

This new incompatible revision of the AIC peripheral introduces
multi-die support. This binding is based on apple,aic, but
changes interrupt-cells to add a new die argument.

Also adds an apple,event-reg property to specify the offset of the event
register. Inexplicably, the capability registers allow us to compute
other register offsets, but not this one. This allows us to keep
forward-compatibility with future SoCs that will likely implement
different die counts, thus shifting the event register. Apple do the
same thing in their device tree...

Signed-off-by: Hector Martin <[email protected]>
---
.../interrupt-controller/apple,aic2.yaml | 99 +++++++++++++++++++
MAINTAINERS | 2 +-
2 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml

diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
new file mode 100644
index 000000000000..311900613b9e
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/apple,aic2.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Apple Interrupt Controller 2
+
+maintainers:
+ - Hector Martin <[email protected]>
+
+description: |
+ The Apple Interrupt Controller 2 is a simple interrupt controller present on
+ Apple ARM SoC platforms starting with t600x (M1 Pro and Max).
+
+ It provides the following features:
+
+ - Level-triggered hardware IRQs wired to SoC blocks
+ - Single mask bit per IRQ
+ - Automatic masking on event delivery (auto-ack)
+ - Software triggering (ORed with hw line)
+ - Automatic prioritization (single event/ack register per CPU, lower IRQs =
+ higher priority)
+ - Automatic masking on ack
+ - Support for multiple dies
+
+ This device also represents the FIQ interrupt sources on platforms using AIC,
+ which do not go through a discrete interrupt controller. It also handles
+ FIQ-based Fast IPIs.
+
+properties:
+ compatible:
+ items:
+ - const: apple,t6000-aic
+ - const: apple,aic2
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 4
+ description: |
+ The 1st cell contains the interrupt type:
+ - 0: Hardware IRQ
+ - 1: FIQ
+
+ The 2nd cell contains the die ID.
+
+ The next cell contains the interrupt number.
+ - HW IRQs: interrupt number
+ - FIQs:
+ - 0: physical HV timer
+ - 1: virtual HV timer
+ - 2: physical guest timer
+ - 3: virtual guest timer
+
+ The last cell contains the interrupt flags. This is normally
+ IRQ_TYPE_LEVEL_HIGH (4).
+
+ reg:
+ description: |
+ Specifies base physical address and size of the AIC registers.
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ apple,event-reg:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Specifies the offset of the event register, which lies after all the
+ implemented die register sets, page aligned. This is not computable from
+ capability register values, so we have to specify it explicitly.
+
+required:
+ - compatible
+ - '#interrupt-cells'
+ - interrupt-controller
+ - reg
+ - apple,event-reg
+
+additionalProperties: false
+
+allOf:
+ - $ref: /schemas/interrupt-controller.yaml#
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ aic: interrupt-controller@28e100000 {
+ compatible = "apple,t6000-aic", "apple,aic2";
+ #interrupt-cells = <4>;
+ interrupt-controller;
+ reg = <0x2 0x8e100000 0x0 0x10000>;
+ apple,event-reg = <0xc000>;
+ };
+ };
diff --git a/MAINTAINERS b/MAINTAINERS
index aa0f6cbb634e..ad887ec7e96b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1806,7 +1806,7 @@ T: git https://github.com/AsahiLinux/linux.git
F: Documentation/devicetree/bindings/arm/apple.yaml
F: Documentation/devicetree/bindings/arm/apple/*
F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
-F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
+F: Documentation/devicetree/bindings/interrupt-controller/apple,*
F: Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
F: Documentation/devicetree/bindings/pci/apple,pcie.yaml
F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
--
2.33.0


2022-02-26 01:46:41

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] dt-bindings: interrupt-controller: apple,aic2: New binding for AICv2

On Thu, Feb 24, 2022 at 10:07:36PM +0900, Hector Martin wrote:
> This new incompatible revision of the AIC peripheral introduces
> multi-die support. This binding is based on apple,aic, but
> changes interrupt-cells to add a new die argument.
>
> Also adds an apple,event-reg property to specify the offset of the event
> register. Inexplicably, the capability registers allow us to compute
> other register offsets, but not this one. This allows us to keep
> forward-compatibility with future SoCs that will likely implement
> different die counts, thus shifting the event register. Apple do the
> same thing in their device tree...
>
> Signed-off-by: Hector Martin <[email protected]>
> ---
> .../interrupt-controller/apple,aic2.yaml | 99 +++++++++++++++++++
> MAINTAINERS | 2 +-
> 2 files changed, 100 insertions(+), 1 deletion(-)
> create mode 100644 Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
>
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml b/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
> new file mode 100644
> index 000000000000..311900613b9e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/apple,aic2.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/apple,aic2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple Interrupt Controller 2
> +
> +maintainers:
> + - Hector Martin <[email protected]>
> +
> +description: |
> + The Apple Interrupt Controller 2 is a simple interrupt controller present on
> + Apple ARM SoC platforms starting with t600x (M1 Pro and Max).
> +
> + It provides the following features:
> +
> + - Level-triggered hardware IRQs wired to SoC blocks
> + - Single mask bit per IRQ
> + - Automatic masking on event delivery (auto-ack)
> + - Software triggering (ORed with hw line)
> + - Automatic prioritization (single event/ack register per CPU, lower IRQs =
> + higher priority)
> + - Automatic masking on ack
> + - Support for multiple dies
> +
> + This device also represents the FIQ interrupt sources on platforms using AIC,
> + which do not go through a discrete interrupt controller. It also handles
> + FIQ-based Fast IPIs.
> +
> +properties:
> + compatible:
> + items:
> + - const: apple,t6000-aic
> + - const: apple,aic2

I feel I was sold on Apple doesn't change h/w and we're the 2nd chip in
and the h/w changed. Just my musings, but aic3 will be rejected. :(

> +
> + interrupt-controller: true
> +
> + '#interrupt-cells':
> + const: 4
> + description: |
> + The 1st cell contains the interrupt type:
> + - 0: Hardware IRQ
> + - 1: FIQ
> +
> + The 2nd cell contains the die ID.
> +
> + The next cell contains the interrupt number.
> + - HW IRQs: interrupt number
> + - FIQs:
> + - 0: physical HV timer
> + - 1: virtual HV timer
> + - 2: physical guest timer
> + - 3: virtual guest timer
> +
> + The last cell contains the interrupt flags. This is normally
> + IRQ_TYPE_LEVEL_HIGH (4).
> +
> + reg:
> + description: |
> + Specifies base physical address and size of the AIC registers.
> + maxItems: 1
> +
> + power-domains:
> + maxItems: 1
> +
> + apple,event-reg:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Specifies the offset of the event register, which lies after all the
> + implemented die register sets, page aligned. This is not computable from
> + capability register values, so we have to specify it explicitly.

If this is last, then couldn't it be a 2nd 'reg' entry?

'page aligned' is ambiguous. I assume that means 16K since that's what
Apple uses, but I might assume 4K not knowing that.

> +
> +required:
> + - compatible
> + - '#interrupt-cells'
> + - interrupt-controller
> + - reg
> + - apple,event-reg
> +
> +additionalProperties: false
> +
> +allOf:
> + - $ref: /schemas/interrupt-controller.yaml#
> +
> +examples:
> + - |
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + aic: interrupt-controller@28e100000 {
> + compatible = "apple,t6000-aic", "apple,aic2";
> + #interrupt-cells = <4>;
> + interrupt-controller;
> + reg = <0x2 0x8e100000 0x0 0x10000>;
> + apple,event-reg = <0xc000>;
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index aa0f6cbb634e..ad887ec7e96b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1806,7 +1806,7 @@ T: git https://github.com/AsahiLinux/linux.git
> F: Documentation/devicetree/bindings/arm/apple.yaml
> F: Documentation/devicetree/bindings/arm/apple/*
> F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
> -F: Documentation/devicetree/bindings/interrupt-controller/apple,aic.yaml
> +F: Documentation/devicetree/bindings/interrupt-controller/apple,*
> F: Documentation/devicetree/bindings/mailbox/apple,mailbox.yaml
> F: Documentation/devicetree/bindings/pci/apple,pcie.yaml
> F: Documentation/devicetree/bindings/pinctrl/apple,pinctrl.yaml
> --
> 2.33.0
>
>

2022-02-26 01:56:52

by Hector Martin

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] dt-bindings: interrupt-controller: apple,aic2: New binding for AICv2

On 26/02/2022 05.19, Rob Herring wrote:
>> +properties:
>> + compatible:
>> + items:
>> + - const: apple,t6000-aic
>> + - const: apple,aic2
>
> I feel I was sold on Apple doesn't change h/w and we're the 2nd chip in
> and the h/w changed. Just my musings, but aic3 will be rejected. :(

Well yes, after not changing hardware for N phone/tablet generations,
they figured out they *finally* had to make some changes for real
desktop chips... (t8103 was a tablet chip they shoehorned into laptops;
t6000 is the first real laptop/desktop chip). This isn't the 2nd chip
in, this is the 26th chip in or so, and yet it's called AIC2 (by Apple
even)... We aren't starting from chip #1, just the first chip they
decided to *let* us put Linux on.

It's pretty clear that the t6000 changes were made with future-proofing
in mind. I guess we'll find out in a couple weeks, since the rumor mill
says M2 is coming. If I'm right and we end up needing *zero* kernel
changes to boot on M2, will you be happy? ;-)

>> + apple,event-reg:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description:
>> + Specifies the offset of the event register, which lies after all the
>> + implemented die register sets, page aligned. This is not computable from
>> + capability register values, so we have to specify it explicitly.
>
> If this is last, then couldn't it be a 2nd 'reg' entry?
>
> 'page aligned' is ambiguous. I assume that means 16K since that's what
> Apple uses, but I might assume 4K not knowing that.

16K, and yeah, it could be a 2nd reg entry if you think that works
better. Makes sense.

--
Hector Martin ([email protected])
Public Key: https://mrcn.st/pub

2022-03-07 12:23:33

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH v2 2/7] dt-bindings: interrupt-controller: apple,aic2: New binding for AICv2

On Fri, 25 Feb 2022 21:58:34 +0000,
Hector Martin <[email protected]> wrote:
>
> On 26/02/2022 05.19, Rob Herring wrote:
> >> +properties:
> >> + compatible:
> >> + items:
> >> + - const: apple,t6000-aic
> >> + - const: apple,aic2
> >
> > I feel I was sold on Apple doesn't change h/w and we're the 2nd chip in
> > and the h/w changed. Just my musings, but aic3 will be rejected. :(
>
> Well yes, after not changing hardware for N phone/tablet generations,
> they figured out they *finally* had to make some changes for real
> desktop chips... (t8103 was a tablet chip they shoehorned into laptops;
> t6000 is the first real laptop/desktop chip). This isn't the 2nd chip
> in, this is the 26th chip in or so, and yet it's called AIC2 (by Apple
> even)... We aren't starting from chip #1, just the first chip they
> decided to *let* us put Linux on.
>
> It's pretty clear that the t6000 changes were made with future-proofing
> in mind. I guess we'll find out in a couple weeks, since the rumor mill
> says M2 is coming. If I'm right and we end up needing *zero* kernel
> changes to boot on M2, will you be happy? ;-)
>
> >> + apple,event-reg:
> >> + $ref: /schemas/types.yaml#/definitions/uint32
> >> + description:
> >> + Specifies the offset of the event register, which lies after all the
> >> + implemented die register sets, page aligned. This is not computable from
> >> + capability register values, so we have to specify it explicitly.
> >
> > If this is last, then couldn't it be a 2nd 'reg' entry?
> >
> > 'page aligned' is ambiguous. I assume that means 16K since that's what
> > Apple uses, but I might assume 4K not knowing that.
>
> 16K, and yeah, it could be a 2nd reg entry if you think that works
> better. Makes sense.

Do you plan to respin this? If I'm going to that this series for 5.18,
it needs to be this week.

Thanks,

M.

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