[PATCH 2/4] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller

[PATCH 2/4] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller

[qinjian]

Add documentation to describe Sunplus SP7021 interrupt controller bindings.

Signed-off-by: qinjian <qinjian@cqplus1.com>
---
 .../sunplus,sp7021-intc.yaml                  | 69 +++++++++++++++++++
 MAINTAINERS                                   |  2 +
 .../interrupt-controller/sp7021-intc.h        | 24 +++++++
 3 files changed, 95 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
 create mode 100644 include/dt-bindings/interrupt-controller/sp7021-intc.h

diff --git a/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
new file mode 100644
index 000000000..73719f65b
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
@@ -0,0 +1,69 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) Sunplus Co., Ltd. 2021
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/sunplus,sp7021-intc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sunplus SP7021 SoC Interrupt Controller Device Tree Bindings
+
+maintainers:
+  - Qin Jian <qinjian@cqplus1.com>
+
+properties:
+  compatible:
+    items:
+      - const: sunplus,sp7021-intc
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 2
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupts references to primary interrupt controller
+
+  ext0-mask:
+    description:
+      cpu affinity of EXT_INT0.
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    minimum: 1
+    maximum: 16
+
+  ext1-mask:
+    description:
+      cpu affinity of EXT_INT1.
+    $ref: "/schemas/types.yaml#/definitions/uint32"
+    minimum: 1
+    maximum: 16
+
+required:
+  - compatible
+  - interrupt-controller
+  - "#interrupt-cells"
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    intc: interrupt-controller@9c000780 {
+        compatible = "sunplus,sp7021-intc";
+        interrupt-controller;
+        #interrupt-cells = <2>;
+        reg = <0x9c000780 0x80>, <0x9c000a80 0x80>;
+        interrupt-parent = <&gic>;
+        interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>, /* EXT_INT0 */
+                    <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; /* EXT_INT1 */
+        ext0-mask = <0xf>; /* core0-3 */
+        ext1-mask = <0xe>; /* core1-3 */
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 7a78b1bbe..065da0846 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2660,6 +2660,8 @@ M:	Qin Jian <qinjian@cqplus1.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for mon-subscribers)
 S:	Maintained
 F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
+F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
+F:	include/dt-bindings/interrupt-controller/sp7021-intc.h
 
 ARM/Synaptics SoC support
 M:	Jisheng Zhang <Jisheng.Zhang@synaptics.com>
diff --git a/include/dt-bindings/interrupt-controller/sp7021-intc.h b/include/dt-bindings/interrupt-controller/sp7021-intc.h
new file mode 100644
index 000000000..8f36e262e
--- /dev/null
+++ b/include/dt-bindings/interrupt-controller/sp7021-intc.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) Sunplus Technology Co., Ltd.
+ *       All rights reserved.
+ *
+ *
+ * This header provides constants for the SP7021 INTC
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_SP7021_INTC_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_SP7021_INTC_H
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/*
+ * Interrupt specifier cell 1.
+ * The flags in irq.h are valid, plus those below.
+ */
+#define SP_INTC_EXT_INT0		0x00000
+#define SP_INTC_EXT_INT1		0x01000
+#define SP_INTC_EXT_INT_MASK	0xff000
+#define SP_INTC_EXT_INT_SHFIT	12
+
+#endif
-- 
2.33.1

Re: [PATCH 2/4] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller

[Rob Herring]

On Fri, 22 Oct 2021 14:07:37 +0800, qinjian wrote:
> Add documentation to describe Sunplus SP7021 interrupt controller bindings.
> 
> Signed-off-by: qinjian <qinjian@cqplus1.com>
> ---
>  .../sunplus,sp7021-intc.yaml                  | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +
>  .../interrupt-controller/sp7021-intc.h        | 24 +++++++
>  3 files changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  create mode 100644 include/dt-bindings/interrupt-controller/sp7021-intc.h
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.example.dt.yaml: interrupt-controller@9c000780: reg: [[2617247616, 128], [2617248384, 128]] is too long
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1544791

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH 2/4] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller

[Rob Herring]

On Fri, Oct 22, 2021 at 02:07:37PM +0800, qinjian wrote:
> Add documentation to describe Sunplus SP7021 interrupt controller bindings.
> 
> Signed-off-by: qinjian <qinjian@cqplus1.com>
> ---
>  .../sunplus,sp7021-intc.yaml                  | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +
>  .../interrupt-controller/sp7021-intc.h        | 24 +++++++
>  3 files changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  create mode 100644 include/dt-bindings/interrupt-controller/sp7021-intc.h
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
> new file mode 100644
> index 000000000..73719f65b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) Sunplus Co., Ltd. 2021
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/sunplus,sp7021-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sunplus SP7021 SoC Interrupt Controller Device Tree Bindings
> +
> +maintainers:
> +  - Qin Jian <qinjian@cqplus1.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: sunplus,sp7021-intc
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2

Looks like cell 1 is more than just 0-N hw irq numbers, so please 
describe the format of the cell.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Interrupts references to primary interrupt controller

How many?

> +
> +  ext0-mask:
> +    description:
> +      cpu affinity of EXT_INT0.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 1
> +    maximum: 16
> +
> +  ext1-mask:
> +    description:
> +      cpu affinity of EXT_INT1.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 1
> +    maximum: 16

These need a vendor prefix.

This assumes the DT cpu numbering is 0-N which isn't the case on all 
architectures (Arm specifically).

> +
> +required:
> +  - compatible
> +  - interrupt-controller
> +  - "#interrupt-cells"
> +  - reg
> +  - interrupts
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    intc: interrupt-controller@9c000780 {
> +        compatible = "sunplus,sp7021-intc";
> +        interrupt-controller;
> +        #interrupt-cells = <2>;
> +        reg = <0x9c000780 0x80>, <0x9c000a80 0x80>;
> +        interrupt-parent = <&gic>;
> +        interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>, /* EXT_INT0 */
> +                    <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>; /* EXT_INT1 */
> +        ext0-mask = <0xf>; /* core0-3 */
> +        ext1-mask = <0xe>; /* core1-3 */
> +    };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7a78b1bbe..065da0846 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2660,6 +2660,8 @@ M:	Qin Jian <qinjian@cqplus1.com>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for mon-subscribers)
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/arm/sunplus,sp7021.yaml
> +F:	Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
> +F:	include/dt-bindings/interrupt-controller/sp7021-intc.h
>  
>  ARM/Synaptics SoC support
>  M:	Jisheng Zhang <Jisheng.Zhang@synaptics.com>
> diff --git a/include/dt-bindings/interrupt-controller/sp7021-intc.h b/include/dt-bindings/interrupt-controller/sp7021-intc.h
> new file mode 100644
> index 000000000..8f36e262e
> --- /dev/null
> +++ b/include/dt-bindings/interrupt-controller/sp7021-intc.h
> @@ -0,0 +1,24 @@
> +/* SPDX-License-Identifier: GPL-2.0 */

Dual license please.

> +/*
> + * Copyright (C) Sunplus Technology Co., Ltd.
> + *       All rights reserved.
> + *
> + *
> + * This header provides constants for the SP7021 INTC
> + */
> +
> +#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_SP7021_INTC_H
> +#define _DT_BINDINGS_INTERRUPT_CONTROLLER_SP7021_INTC_H
> +
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/*
> + * Interrupt specifier cell 1.
> + * The flags in irq.h are valid, plus those below.
> + */
> +#define SP_INTC_EXT_INT0		0x00000
> +#define SP_INTC_EXT_INT1		0x01000
> +#define SP_INTC_EXT_INT_MASK	0xff000
> +#define SP_INTC_EXT_INT_SHFIT	12
> +
> +#endif
> -- 
> 2.33.1
> 
> 

Re: [PATCH 2/4] dt-bindings: interrupt-controller: Add bindings for SP7021 interrupt controller

[Marc Zyngier]

On Fri, 22 Oct 2021 07:07:37 +0100,
qinjian <qinjian@cqplus1.com> wrote:
> 
> Add documentation to describe Sunplus SP7021 interrupt controller bindings.
> 
> Signed-off-by: qinjian <qinjian@cqplus1.com>
> ---
>  .../sunplus,sp7021-intc.yaml                  | 69 +++++++++++++++++++
>  MAINTAINERS                                   |  2 +
>  .../interrupt-controller/sp7021-intc.h        | 24 +++++++
>  3 files changed, 95 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
>  create mode 100644 include/dt-bindings/interrupt-controller/sp7021-intc.h
> 
> diff --git a/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
> new file mode 100644
> index 000000000..73719f65b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/interrupt-controller/sunplus,sp7021-intc.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) Sunplus Co., Ltd. 2021
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/interrupt-controller/sunplus,sp7021-intc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Sunplus SP7021 SoC Interrupt Controller Device Tree Bindings
> +
> +maintainers:
> +  - Qin Jian <qinjian@cqplus1.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - const: sunplus,sp7021-intc
> +
> +  interrupt-controller: true
> +
> +  "#interrupt-cells":
> +    const: 2
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      Interrupts references to primary interrupt controller
> +
> +  ext0-mask:
> +    description:
> +      cpu affinity of EXT_INT0.
> +    $ref: "/schemas/types.yaml#/definitions/uint32"
> +    minimum: 1
> +    maximum: 16

That's not a description of the HW. This is just a SW policy that
trickles into the DT. The output interrupt is simply connected to the
parent interrupt controller, and how it is routed is out of the scope
of the DT. This (and its ext1 sibling) should be dropped.

	M.

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