[PATCH 0/2] Allwinner H616 NMI Controller

[PATCH 0/2] Allwinner H616 NMI Controller

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the Allwinner H616 NMI controller, which appears to be
common IP with previous generations of the NMI. The NMI is required to
support interrupts from the PMIC among other functions.

Chris Morgan (2):
  dt-bindings: irq: sun7i-nmi: Add binding for the H616 NMI controller
  arm64: dts: allwinner: h616: Add NMI device node

 .../interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml | 3 +++
 arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi           | 9 +++++++++
 2 files changed, 12 insertions(+)

-- 
2.34.1

[PATCH 1/2] dt-bindings: irq: sun7i-nmi: Add binding for the H616 NMI controller

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add binding for the H616 NMI controller. This is necessary to handle
interrupts from devices such as the internal PMIC.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 .../interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml       | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
index 83603180d8d9..4fc8b0320a31 100644
--- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
+++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
@@ -32,6 +32,9 @@ properties:
       - items:
           - const: allwinner,sun50i-a100-nmi
           - const: allwinner,sun9i-a80-nmi
+      - items:
+          - const: allwinner,sun50i-h616-nmi
+          - const: allwinner,sun9i-a80-nmi
 
   reg:
     maxItems: 1
-- 
2.34.1

Re: [PATCH 1/2] dt-bindings: irq: sun7i-nmi: Add binding for the H616 NMI controller

[Andre Przywara]

On Sun, 14 Apr 2024 12:04:23 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add binding for the H616 NMI controller. This is necessary to handle
> interrupts from devices such as the internal PMIC.

I think "internal PMIC" is confusing, and I guess it's not really
relevant anyway and you can remove the whole sentence.
Instead just mention that this controller turns out to be compatible
to the one described in the A80 manual.

> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  .../interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml       | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> index 83603180d8d9..4fc8b0320a31 100644
> --- a/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> +++ b/Documentation/devicetree/bindings/interrupt-controller/allwinner,sun7i-a20-sc-nmi.yaml
> @@ -32,6 +32,9 @@ properties:
>        - items:
>            - const: allwinner,sun50i-a100-nmi
>            - const: allwinner,sun9i-a80-nmi
> +      - items:
> +          - const: allwinner,sun50i-h616-nmi
> +          - const: allwinner,sun9i-a80-nmi

It looks like this is the third chip that uses an A80 compatible
version. So I'd say it's time to use an enum to group the A100, H616
and V3s together. Check allwinner,sun6i-a31-r-intc.yaml for an example.

Otherwise looks good.

Cheers,
Andre

>  
>    reg:
>      maxItems: 1

[PATCH 2/2] arm64: dts: allwinner: h616: Add NMI device node

[Chris Morgan]

From: Chris Morgan <macromorgan@hotmail.com>

Add device node for the H616 Non Maskable Interrupt (NMI) controller.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
---
 arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
index b2e85e52d1a1..1e066f3057be 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
@@ -775,6 +775,15 @@ r_ccu: clock@7010000 {
 			#reset-cells = <1>;
 		};
 
+		nmi_intc: interrupt-controller@7010320 {
+			compatible = "allwinner,sun50i-h616-nmi",
+				     "allwinner,sun9i-a80-nmi";
+			reg = <0x07010320 0xc>;
+			interrupt-controller;
+			#interrupt-cells = <2>;
+			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+		};
+
 		r_pio: pinctrl@7022000 {
 			compatible = "allwinner,sun50i-h616-r-pinctrl";
 			reg = <0x07022000 0x400>;
-- 
2.34.1

Re: [PATCH 2/2] arm64: dts: allwinner: h616: Add NMI device node

[Andre Przywara]

On Sun, 14 Apr 2024 12:04:24 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:

Hi Chris,

> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add device node for the H616 Non Maskable Interrupt (NMI) controller.

You might want to mention that the NMI pad is not exposed on the H616 variants, but on
the T507 and H700 packages.

> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> ---
>  arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> index b2e85e52d1a1..1e066f3057be 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> @@ -775,6 +775,15 @@ r_ccu: clock@7010000 {
>  			#reset-cells = <1>;
>  		};
>  
> +		nmi_intc: interrupt-controller@7010320 {
> +			compatible = "allwinner,sun50i-h616-nmi",
> +				     "allwinner,sun9i-a80-nmi";
> +			reg = <0x07010320 0xc>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> +		};
> +

I can confirm that this matches the manual, and the registers behave as
described in the A80 manual. I don't have access to a chip with the NMI
pad exposed or used, so I cannot test this fully, but Chris'
experiments with the AXP717 PMIC connected to that pin on on H700
board seem to confirm that it indeed works.

So with that small amendment to the commit message please take my:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre

>  		r_pio: pinctrl@7022000 {
>  			compatible = "allwinner,sun50i-h616-r-pinctrl";
>  			reg = <0x07022000 0x400>;

Re: [PATCH 2/2] arm64: dts: allwinner: h616: Add NMI device node

[Chris Morgan]

On Mon, Apr 15, 2024 at 12:37:40AM +0100, Andre Przywara wrote:
> On Sun, 14 Apr 2024 12:04:24 -0500
> Chris Morgan <macroalpha82@gmail.com> wrote:
> 
> Hi Chris,
> 
> > From: Chris Morgan <macromorgan@hotmail.com>
> > 
> > Add device node for the H616 Non Maskable Interrupt (NMI) controller.
> 
> You might want to mention that the NMI pad is not exposed on the H616 variants, but on
> the T507 and H700 packages.
> 
> > 
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > ---
> >  arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > index b2e85e52d1a1..1e066f3057be 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > @@ -775,6 +775,15 @@ r_ccu: clock@7010000 {
> >  			#reset-cells = <1>;
> >  		};
> >  
> > +		nmi_intc: interrupt-controller@7010320 {
> > +			compatible = "allwinner,sun50i-h616-nmi",
> > +				     "allwinner,sun9i-a80-nmi";
> > +			reg = <0x07010320 0xc>;
> > +			interrupt-controller;
> > +			#interrupt-cells = <2>;
> > +			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > +		};
> > +
> 
> I can confirm that this matches the manual, and the registers behave as
> described in the A80 manual. I don't have access to a chip with the NMI
> pad exposed or used, so I cannot test this fully, but Chris'
> experiments with the AXP717 PMIC connected to that pin on on H700
> board seem to confirm that it indeed works.
> 
> So with that small amendment to the commit message please take my:
> 
> Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> 
> Cheers,
> Andre
> 
> >  		r_pio: pinctrl@7022000 {
> >  			compatible = "allwinner,sun50i-h616-r-pinctrl";
> >  			reg = <0x07022000 0x400>;
> 

Since the H616 doesn't have this functionality but the T507 and H700
does, should I change the compatible string? It's all the same
silicon die with just a different part number printed on it, but
still...

Thank you.
Chris

Re: [PATCH 2/2] arm64: dts: allwinner: h616: Add NMI device node

[Andre Przywara]

On Thu, 18 Apr 2024 10:59:13 -0500
Chris Morgan <macromorgan@hotmail.com> wrote:

Hi,

> On Mon, Apr 15, 2024 at 12:37:40AM +0100, Andre Przywara wrote:
> > On Sun, 14 Apr 2024 12:04:24 -0500
> > Chris Morgan <macroalpha82@gmail.com> wrote:
> > 
> > Hi Chris,
> >   
> > > From: Chris Morgan <macromorgan@hotmail.com>
> > > 
> > > Add device node for the H616 Non Maskable Interrupt (NMI) controller.  
> > 
> > You might want to mention that the NMI pad is not exposed on the H616 variants, but on
> > the T507 and H700 packages.
> >   
> > > 
> > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > ---
> > >  arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > > index b2e85e52d1a1..1e066f3057be 100644
> > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > > @@ -775,6 +775,15 @@ r_ccu: clock@7010000 {
> > >  			#reset-cells = <1>;
> > >  		};
> > >  
> > > +		nmi_intc: interrupt-controller@7010320 {
> > > +			compatible = "allwinner,sun50i-h616-nmi",
> > > +				     "allwinner,sun9i-a80-nmi";
> > > +			reg = <0x07010320 0xc>;
> > > +			interrupt-controller;
> > > +			#interrupt-cells = <2>;
> > > +			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > > +		};
> > > +  
> > 
> > I can confirm that this matches the manual, and the registers behave as
> > described in the A80 manual. I don't have access to a chip with the NMI
> > pad exposed or used, so I cannot test this fully, but Chris'
> > experiments with the AXP717 PMIC connected to that pin on on H700
> > board seem to confirm that it indeed works.
> > 
> > So with that small amendment to the commit message please take my:
> > 
> > Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> > 
> > Cheers,
> > Andre
> >   
> > >  		r_pio: pinctrl@7022000 {
> > >  			compatible = "allwinner,sun50i-h616-r-pinctrl";
> > >  			reg = <0x07022000 0x400>;  
> >   
> 
> Since the H616 doesn't have this functionality but the T507 and H700
> does, should I change the compatible string? It's all the same
> silicon die with just a different part number printed on it, but
> still...

I would stick to h616, since we use that for all the other devices. Also
the H616 *has* that NMI controller: I can confirm that the registers exist,
and I can trigger and acknowledge interrupts. So in the interest of
consistency: keep using "allwinner,sun50i-h616-nmi".

Cheers,
Andre

Re: [PATCH 2/2] arm64: dts: allwinner: h616: Add NMI device node

[Chris Morgan]

On Thu, Apr 18, 2024 at 05:19:07PM +0100, Andre Przywara wrote:
> On Thu, 18 Apr 2024 10:59:13 -0500
> Chris Morgan <macromorgan@hotmail.com> wrote:
> 
> Hi,
> 
> > On Mon, Apr 15, 2024 at 12:37:40AM +0100, Andre Przywara wrote:
> > > On Sun, 14 Apr 2024 12:04:24 -0500
> > > Chris Morgan <macroalpha82@gmail.com> wrote:
> > > 
> > > Hi Chris,
> > >   
> > > > From: Chris Morgan <macromorgan@hotmail.com>
> > > > 
> > > > Add device node for the H616 Non Maskable Interrupt (NMI) controller.  
> > > 
> > > You might want to mention that the NMI pad is not exposed on the H616 variants, but on
> > > the T507 and H700 packages.
> > >   
> > > > 
> > > > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > > > ---
> > > >  arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi | 9 +++++++++
> > > >  1 file changed, 9 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > > > index b2e85e52d1a1..1e066f3057be 100644
> > > > --- a/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h616.dtsi
> > > > @@ -775,6 +775,15 @@ r_ccu: clock@7010000 {
> > > >  			#reset-cells = <1>;
> > > >  		};
> > > >  
> > > > +		nmi_intc: interrupt-controller@7010320 {
> > > > +			compatible = "allwinner,sun50i-h616-nmi",
> > > > +				     "allwinner,sun9i-a80-nmi";
> > > > +			reg = <0x07010320 0xc>;
> > > > +			interrupt-controller;
> > > > +			#interrupt-cells = <2>;
> > > > +			interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > > > +		};
> > > > +  
> > > 
> > > I can confirm that this matches the manual, and the registers behave as
> > > described in the A80 manual. I don't have access to a chip with the NMI
> > > pad exposed or used, so I cannot test this fully, but Chris'
> > > experiments with the AXP717 PMIC connected to that pin on on H700
> > > board seem to confirm that it indeed works.
> > > 
> > > So with that small amendment to the commit message please take my:
> > > 
> > > Reviewed-by: Andre Przywara <andre.przywara@arm.com>
> > > 
> > > Cheers,
> > > Andre
> > >   
> > > >  		r_pio: pinctrl@7022000 {
> > > >  			compatible = "allwinner,sun50i-h616-r-pinctrl";
> > > >  			reg = <0x07022000 0x400>;  
> > >   
> > 
> > Since the H616 doesn't have this functionality but the T507 and H700
> > does, should I change the compatible string? It's all the same
> > silicon die with just a different part number printed on it, but
> > still...
> 
> I would stick to h616, since we use that for all the other devices. Also
> the H616 *has* that NMI controller: I can confirm that the registers exist,
> and I can trigger and acknowledge interrupts. So in the interest of
> consistency: keep using "allwinner,sun50i-h616-nmi".
> 
> Cheers,
> Andre

Okay, and I'll just resubmit then with the notes you mentioned and
your tags.

Thank you.