Add Actions Semi Owl family sirq support

PATCH Superseded Cristian Ciocaltea 4 participants 16 messages Aug 19, 2020 – Aug 27, 2020

Subsystems (3)

Device tree bindings irqchip drivers Arm/actions semi architecture

lore

Share & tools

lore

Cristian Ciocaltea

author

2020-08-19 16:38 UTC #0 lore raw

v5 cover Add Actions Semi Owl family sirq support

This patch series adds support for the external interrupt controller (SIRQ) found in the Actions Semi Owl family of SoC's (S500, S700 and S900). The controller handles up to 3 external interrupt lines through dedicated SIRQ pins.

This is a rework of the patch series submitted some time ago by Parthiban Nallathambi:
https://lore.kernel.org/lkml/[email protected]/

Please note I have dropped, for the moment, the S700 related patches since I do not own a compatible hardware for testing. I'm using instead an S500 SoC based board for which I have already provided the initial support:
https://lore.kernel.org/lkml/[email protected]/

The SIRQ controller support is a prerequisite of the soon to be submitted MFD driver for the Actions Semi ATC260x PMICs.

Thanks and regards,
Cristi

Changes in v5:
- Integrated Marc's review (more details in the driver patch changelog)
- Rebased patch series on v5.9-rc1

Changes in v4:
- Simplified the DTS structure:
* dropped 'actions,sirq-shared-reg' node, now the differentiation between SoC variants is handled now via the compatible property
* dropped 'actions,sirq-reg-offset', now controller base address in DTS points to SIRQ0 register, so no additional information is required for S500 and S700, while for S900 SoC the offsets of SIRQ1 and SIRQ2 regs are provided by the driver
* 'actions,ext-irq-range' was replaced with 'actions,ext-interrupts', an array of the GIC interrupts triggered by the controller
- Fixed wrong INTC_EXTCTL_TYPE_MASK definition
- Removed redundant irq_fwspec checks in owl_sirq_domain_alloc()
- Improved error handling in owl_sirq_of_init()
- Added yaml binding document
- Dropped S700 related DTS patches for lack of testing hardware:
* arm64: dts: actions: Add sirq node for Actions Semi S700
* arm64: dts: actions: s700-cubieboard7: Enable SIRQ
- Updated MAINTAINERS
- Rebased patchset on kernel v5.8
- Cosmetic changes
* Ordered include statements alphabetically
* Added comment to owl_sirq_set_type() describing conversion of falling edge or active low signals
* Replaced IRQF_TRIGGER_* with corresponding IRQ_TYPE_* variants
* Ensured data types and function naming are consistent regarding the 'owl_sirq' prefix

Changes in v3 (Parthiban Nallathambi):
- Set default operating frequency to 24MHz
- Falling edge and Low Level interrupts translated to rising edge and high level
- Introduced common function with lock handling for register read and write
- Used direct GIC interrupt number for interrupt local hwirq and finding offset using DT entry (range) when registers are shared
- Changed irq_ack to irq_eoi
- Added translation method for irq_domain_ops
- Clearing interrupt pending based on bitmask for edge triggered
- Added pinctrl definition for sirq for cubieboard7. This depends on,
https://lore.kernel.org/patchwork/patch/1012859/

Changes in v2 (Parthiban Nallathambi):
- Added SIRQ as hierarchical chip

        GIC <----> SIRQ <----> External interrupt controller/Child devices

- Device binding updates with vendor prefix
- Register sharing handled globally and common init sequence/data for all actions SoC family

Cristian Ciocaltea (3):
dt-bindings: interrupt-controller: Add Actions SIRQ controller binding
irqchip: Add Actions Semi Owl SIRQ controller
MAINTAINERS: Add entries for Actions Semi Owl SIRQ controller

 .../actions,owl-sirq.yaml                     |  68 ++++
 MAINTAINERS                                   |   2 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-owl-sirq.c                | 347 ++++++++++++++++++
 4 files changed, 418 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
 create mode 100644 drivers/irqchip/irq-owl-sirq.c

Signature · 1 line

2.28.0

Cristian Ciocaltea

author

2020-08-19 16:38 UTC #1 lore raw

PATCH v5 1/3 dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 and S900 SoCs and provides support for handling up to 3 external interrupt lines.

Signed-off-by Cristian Ciocaltea <[email protected]>

---
Changes in v5:
- Updated controller description statements both in the commit message and the binding doc

 .../actions,owl-sirq.yaml                     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +68 -0

new file mode 100644
index 000000000000..cf9b7a514e4e
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoCs SIRQ interrupt controller
+
+maintainers:
+  - Manivannan Sadhasivam <[email protected]>
+  - Cristian Ciocaltea <[email protected]>
+
+description: |
+  This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700
+  and S900) and provides support for handling up to 3 external interrupt lines.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - actions,s500-sirq
+          - actions,s700-sirq
+          - actions,s900-sirq
+        - const: actions,owl-sirq
+      - const: actions,owl-sirq
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
+      The first cell is the input IRQ number, between 0 and 2, while the second
+      cell is the trigger type as defined in interrupt.txt in this directory.
+
+  'actions,ext-interrupts':
+    description: |
+      Contains the GIC SPI IRQ numbers mapped to the external interrupt
+      lines. They shall be specified sequentially from output 0 to 2.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 3
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+  - 'actions,ext-interrupts'
+
+additionalProperties: false
+
+examples:
+  - |
+    sirq: interrupt-controller@b01b0200 {
+      compatible = "actions,s500-sirq", "actions,owl-sirq";
+      reg = <0xb01b0200 0x4>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      actions,ext-interrupts = <13>, /* SIRQ0 */
+                               <14>, /* SIRQ1 */
+                               <15>; /* SIRQ2 */
+    };
+
+...

Signature · 1 line

2.28.0

Rob Herring

maintainer

2020-08-25 22:09 UTC #2 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Quoted 36 lines · Cristian Ciocaltea

Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 and S900 SoCs and provides support for handling up to 3 external interrupt lines.
Signed-off-by Cristian Ciocaltea <[email protected]>
---
Changes in v5:
- Updated controller description statements both in the commit message and the binding doc
 .../actions,owl-sirq.yaml                     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +16 -0
new file mode 100644
index 000000000000..cf9b7a514e4e
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoCs SIRQ interrupt controller
+
+maintainers:
+  - Manivannan Sadhasivam <[email protected]>
+  - Cristian Ciocaltea <[email protected]>
+
+description: |
+  This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700
+  and S900) and provides support for handling up to 3 external interrupt lines.
+

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 26

Quoted 10 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +10 -0

+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
+          - actions,s500-sirq
+          - actions,s700-sirq
+          - actions,s900-sirq
+        - const: actions,owl-sirq
+      - const: actions,owl-sirq

This should be dropped. You should always have the SoC specific compatible.

Quoted 9 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +9 -0
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 45

Quoted 10 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +10 -0

+      The first cell is the input IRQ number, between 0 and 2, while the second
+      cell is the trigger type as defined in interrupt.txt in this directory.
+
+  'actions,ext-interrupts':
+    description: |
+      Contains the GIC SPI IRQ numbers mapped to the external interrupt
+      lines. They shall be specified sequentially from output 0 to 2.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 3

Can't you use 'interrupts' here?

Quoted 26 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +23 -0

+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+  - 'actions,ext-interrupts'
+
+additionalProperties: false
+
+examples:
+  - |
+    sirq: interrupt-controller@b01b0200 {
+      compatible = "actions,s500-sirq", "actions,owl-sirq";
+      reg = <0xb01b0200 0x4>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      actions,ext-interrupts = <13>, /* SIRQ0 */
+                               <14>, /* SIRQ1 */
+                               <15>; /* SIRQ2 */
+    };
+
+...

Signature · 1 line

2.28.0

Cristian Ciocaltea

author

2020-08-26 21:42 UTC #3 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Hi Rob,

Thanks for the review!

Quoted 42 lines · Rob Herring

Quoted 41 lines · Cristian Ciocaltea
Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 and S900 SoCs and provides support for handling up to 3 external interrupt lines.
Signed-off-by Cristian Ciocaltea <[email protected]>
---
Changes in v5:
- Updated controller description statements both in the commit message and the binding doc
 .../actions,owl-sirq.yaml                     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +21 -0
new file mode 100644
index 000000000000..cf9b7a514e4e
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoCs SIRQ interrupt controller
+
+maintainers:
+  - Manivannan Sadhasivam <[email protected]>
+  - Cristian Ciocaltea <[email protected]>
+
+description: |
+  This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700
+  and S900) and provides support for handling up to 3 external interrupt lines.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:

Quoted 8 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 26
Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +5 -0
+          - actions,s500-sirq
+          - actions,s700-sirq
+          - actions,s900-sirq
+        - const: actions,owl-sirq
+      - const: actions,owl-sirq
This should be dropped. You should always have the SoC specific compatible.

Sure, I will get rid of the 'owl-sirq' compatible.

Quoted 13 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +13 -0

+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
+      The first cell is the input IRQ number, between 0 and 2, while the second
+      cell is the trigger type as defined in interrupt.txt in this directory.
+
+  'actions,ext-interrupts':

Quoted 8 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 45
Quoted 6 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +6 -0
+    description: |
+      Contains the GIC SPI IRQ numbers mapped to the external interrupt
+      lines. They shall be specified sequentially from output 0 to 2.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 3
Can't you use 'interrupts' here?

This was actually my initial idea, but it might confuse the users since this is not following the parent controller IRQ specs, i.e. the trigger type is set internally by the SIRQ driver, it's not taken from DT.

Please see the DTS sample bellow where both devices are on the same level and have GIC as interrupt parent. The 'interrupts' property in the sirq node looks incomplete now. That is why I decided to use a custom name for it, although I'm not sure it's the most relevant one, I am open to any other suggestion.

i2c0: i2c@b0170000 {
[...]
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
[...]
};

sirq: interrupt-controller@b01b0200 {
[...]
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <13>, /* SIRQ0 */

               <14>, /* SIRQ1 */
               <15>; /* SIRQ2 */
};

Regards,
Cristi

Quoted 26 lines

Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +23 -0

+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - '#interrupt-cells'
+  - 'actions,ext-interrupts'
+
+additionalProperties: false
+
+examples:
+  - |
+    sirq: interrupt-controller@b01b0200 {
+      compatible = "actions,s500-sirq", "actions,owl-sirq";
+      reg = <0xb01b0200 0x4>;
+      interrupt-controller;
+      #interrupt-cells = <2>;
+      actions,ext-interrupts = <13>, /* SIRQ0 */
+                               <14>, /* SIRQ1 */
+                               <15>; /* SIRQ2 */
+    };
+
+...

Signature · 1 line

2.28.0

Rob Herring

maintainer

2020-08-26 22:48 UTC #4 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Quoted 81 lines · Cristian Ciocaltea

Hi Rob,
Thanks for the review!
Quoted 42 lines · Rob Herring
Quoted 41 lines · Cristian Ciocaltea
Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 and S900 SoCs and provides support for handling up to 3 external interrupt lines.
Signed-off-by Cristian Ciocaltea <[email protected]>
---
Changes in v5:
- Updated controller description statements both in the commit message and the binding doc
 .../actions,owl-sirq.yaml                     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +21 -0
new file mode 100644
index 000000000000..cf9b7a514e4e
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoCs SIRQ interrupt controller
+
+maintainers:
+  - Manivannan Sadhasivam <[email protected]>
+  - Cristian Ciocaltea <[email protected]>
+
+description: |
+  This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700
+  and S900) and provides support for handling up to 3 external interrupt lines.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 26
Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +5 -0
+          - actions,s500-sirq
+          - actions,s700-sirq
+          - actions,s900-sirq
+        - const: actions,owl-sirq
+      - const: actions,owl-sirq
This should be dropped. You should always have the SoC specific compatible.
Sure, I will get rid of the 'owl-sirq' compatible.
Quoted 13 lines
Quoted 13 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +13 -0
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
+      The first cell is the input IRQ number, between 0 and 2, while the second
+      cell is the trigger type as defined in interrupt.txt in this directory.
+
+  'actions,ext-interrupts':
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 45
Quoted 6 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +6 -0
+    description: |
+      Contains the GIC SPI IRQ numbers mapped to the external interrupt
+      lines. They shall be specified sequentially from output 0 to 2.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 3
Can't you use 'interrupts' here?

Quoted 3 lines

This was actually my initial idea, but it might confuse the users since this is not following the parent controller IRQ specs, i.e. the trigger type is set internally by the SIRQ driver, it's not taken from DT.

Then what's the 2nd cell for?

Quoted 12 lines

Please see the DTS sample bellow where both devices are on the same level and have GIC as interrupt parent. The 'interrupts' property in the sirq node looks incomplete now. That is why I decided to use a custom name for it, although I'm not sure it's the most relevant one, I am open to any other suggestion.
i2c0: i2c@b0170000 {
[...]
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
[...]
};

Quoted 7 lines

sirq: interrupt-controller@b01b0200 {
[...]
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <13>, /* SIRQ0 */
               <14>, /* SIRQ1 */
               <15>; /* SIRQ2 */

This isn't valid if the GIC is the parent as you have to have 3 cells for each interrupt. Ultimately the GIC trigger type has to be something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common.

Rob

Cristian Ciocaltea

author

2020-08-27 10:06 UTC #5 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Quoted 83 lines · Rob Herring

Quoted 81 lines · Cristian Ciocaltea
Hi Rob,
Thanks for the review!
Quoted 42 lines · Rob Herring
Quoted 41 lines · Cristian Ciocaltea
Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 and S900 SoCs and provides support for handling up to 3 external interrupt lines.
Signed-off-by Cristian Ciocaltea <[email protected]>
---
Changes in v5:
- Updated controller description statements both in the commit message and the binding doc
 .../actions,owl-sirq.yaml                     | 68 +++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +21 -0
new file mode 100644
index 000000000000..cf9b7a514e4e
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoCs SIRQ interrupt controller
+
+maintainers:
+  - Manivannan Sadhasivam <[email protected]>
+  - Cristian Ciocaltea <[email protected]>
+
+description: |
+  This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700
+  and S900) and provides support for handling up to 3 external interrupt lines.
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+        - enum:
Quoted 8 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 26
Quoted 5 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +5 -0
+          - actions,s500-sirq
+          - actions,s700-sirq
+          - actions,s900-sirq
+        - const: actions,owl-sirq
+      - const: actions,owl-sirq
This should be dropped. You should always have the SoC specific compatible.
Sure, I will get rid of the 'owl-sirq' compatible.
Quoted 21 lines
Quoted 9 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +9 -0
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  '#interrupt-cells':
+    const: 2
+    description:
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml line 45
Quoted 10 lines
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml new file +10 -0
+      The first cell is the input IRQ number, between 0 and 2, while the second
+      cell is the trigger type as defined in interrupt.txt in this directory.
+
+  'actions,ext-interrupts':
+    description: |
+      Contains the GIC SPI IRQ numbers mapped to the external interrupt
+      lines. They shall be specified sequentially from output 0 to 2.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 3
+    maxItems: 3
Can't you use 'interrupts' here?

Quoted 5 lines

Quoted 3 lines
This was actually my initial idea, but it might confuse the users since this is not following the parent controller IRQ specs, i.e. the trigger type is set internally by the SIRQ driver, it's not taken from DT.
Then what's the 2nd cell for?

I should have added also a child device sample to make it more clear how this is supposed to work:

&i2c0 {

  atc260x: pmic@65 {
    [...]
	interrupt-parent = <&sirq>;
	interrupts = <2 IRQ_TYPE_LEVEL_HIGH>;

};
};

The PMIC above uses the SIRQ2 pin of the SIRQ controller to trigger interrupts, while the controller is responsible for proper translation before sending to GIC, i.e. converting falling edge to rising edge signal and active low to active high signal.

Quoted 20 lines

Quoted 12 lines
Please see the DTS sample bellow where both devices are on the same level and have GIC as interrupt parent. The 'interrupts' property in the sirq node looks incomplete now. That is why I decided to use a custom name for it, although I'm not sure it's the most relevant one, I am open to any other suggestion.
i2c0: i2c@b0170000 {
[...]
interrupts = <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
[...]
};
Quoted 7 lines
sirq: interrupt-controller@b01b0200 {
[...]
interrupt-controller;
#interrupt-cells = <2>;
interrupts = <13>, /* SIRQ0 */
               <14>, /* SIRQ1 */
               <15>; /* SIRQ2 */

Quoted 2 lines

This isn't valid if the GIC is the parent as you have to have 3 cells for each interrupt.

Right, that's the reason of replacing 'interrupts' with 'actions,ext-interrupts'.

Quoted 4 lines

Ultimately the GIC trigger type has to be
something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common.

Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node:

interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */

             [...]

Quoted 1 line

Rob

Thanks,
Cristi

Marc Zyngier

2020-08-27 10:35 UTC #6 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Quoted 3 lines · Cristian Ciocaltea

Quoted 2 lines · Rob Herring
On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea <[email protected]> wrote:

[...]

Quoted 11 lines

Quoted 4 lines
Ultimately the GIC trigger type has to be
something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common.
Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node:
interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */
             [...]

Please don't. If you are describing a GIC interrupt, use a trigger that actually exists. Given that you have a 1:1 mapping between input and output, just encode the output trigger that matches the input.

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Cristian Ciocaltea

author

2020-08-27 15:24 UTC #7 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Hi Marc,

Quoted 16 lines · Marc Zyngier

Quoted 3 lines · Cristian Ciocaltea
Quoted 2 lines · Rob Herring
On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea <[email protected]> wrote:
[...]
Quoted 9 lines
Quoted 4 lines
Ultimately the GIC trigger type has to be
something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common.
Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node:

Quoted 7 lines

Quoted 2 lines
interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */
             [...]
Please don't. If you are describing a GIC interrupt, use a trigger that actually exists. Given that you have a 1:1 mapping between input and output, just encode the output trigger that matches the input.

Understood, the only remark here is that internally, the driver will not use this information and instead will continue to rely on the input to properly set the trigger type for the output.

The question is if the driver should also emit a warning (or error?) when the trigger type supplied via DT doesn't match the expected value.

If yes, we should also clarify what the user is supposed to provide in the controller node: the trigger type before the conversion (the input) or the one after the conversion (the output).

Quoted 3 lines

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Thanks,
Cristi

Marc Zyngier

2020-08-27 15:42 UTC #8 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Cristian,

Quoted 22 lines · Cristian Ciocaltea

Hi Marc,
Quoted 19 lines · Marc Zyngier
Quoted 3 lines · Cristian Ciocaltea
Quoted 2 lines · Rob Herring
On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea <[email protected]> wrote:
[...]
Quoted 11 lines
Quoted 4 lines
Ultimately the GIC trigger type has to be
something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common.
Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node:
interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */
             [...]

Quoted 8 lines

Quoted 4 lines
Please don't. If you are describing a GIC interrupt, use a trigger that actually exists. Given that you have a 1:1 mapping between input and output, just encode the output trigger that matches the input.
Understood, the only remark here is that internally, the driver will not use this information and instead will continue to rely on the input to properly set the trigger type for the output.

It's fine. The binding has to be consistent on its own, but doesn't dictate the way the driver does thing.

Quoted 2 lines

The question is if the driver should also emit a warning (or error?) when the trigger type supplied via DT doesn't match the expected value.

Rob will tell you that the kernel isn't a validation tool for broken DTs. Shout if you want, but you are allowed to simply ignore the output trigger for example

Quoted 3 lines

If yes, we should also clarify what the user is supposed to provide in the controller node: the trigger type before the conversion (the input) or the one after the conversion (the output).

The output of a SIRQ should be compatible with the GIC input it is attached to. You can have:

        device (LEVEL_LOW) -> SIRQ (LEVEL_HIGH) -> GIC

but you can't have:

        device (LEVEL_LOW) -> SIRQ (EDGE_RISING) -> GIC

because that's not an acceptable transformation for the SIRQ, nor can you have:

        device (EDGE_FALLING) -> SIRQ (EDGE_FALLING) -> GIC

because EDGE_FALLING isn't a valid input for the GIC.

In both of the invalid cases, you would be free to apply which ever transformation actually makes sense, and shout at the user if you want to help them debugging their turf. The later part is definitely optional.

Hope this helps,

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Cristian Ciocaltea

author

2020-08-27 18:55 UTC #9 lore raw

Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding

Quoted 62 lines · Marc Zyngier

Cristian,
Quoted 22 lines · Cristian Ciocaltea
Hi Marc,
Quoted 19 lines · Marc Zyngier
Quoted 3 lines · Cristian Ciocaltea
Quoted 2 lines · Rob Herring
On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea <[email protected]> wrote:
[...]
Quoted 11 lines
Quoted 4 lines
Ultimately the GIC trigger type has to be
something. Is it fixed or passed thru? If the latter, just use 0 (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort of translation of the trigger is pretty common.
Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node:
interrupts = <GIC_SPI 13 IRQ_TYPE_NONE>, /* SIRQ0 */
             [...]
Quoted 8 lines
Quoted 4 lines
Please don't. If you are describing a GIC interrupt, use a trigger that actually exists. Given that you have a 1:1 mapping between input and output, just encode the output trigger that matches the input.
Understood, the only remark here is that internally, the driver will not use this information and instead will continue to rely on the input to properly set the trigger type for the output.
It's fine. The binding has to be consistent on its own, but doesn't dictate the way the driver does thing.
Quoted 2 lines
The question is if the driver should also emit a warning (or error?) when the trigger type supplied via DT doesn't match the expected value.
Rob will tell you that the kernel isn't a validation tool for broken DTs. Shout if you want, but you are allowed to simply ignore the output trigger for example
Quoted 3 lines
If yes, we should also clarify what the user is supposed to provide in the controller node: the trigger type before the conversion (the input) or the one after the conversion (the output).
The output of a SIRQ should be compatible with the GIC input it is attached to. You can have:
        device (LEVEL_LOW) -> SIRQ (LEVEL_HIGH) -> GIC
but you can't have:
        device (LEVEL_LOW) -> SIRQ (EDGE_RISING) -> GIC
because that's not an acceptable transformation for the SIRQ, nor can you have:
        device (EDGE_FALLING) -> SIRQ (EDGE_FALLING) -> GIC

Quoted 8 lines

because EDGE_FALLING isn't a valid input for the GIC.
In both of the invalid cases, you would be free to apply which ever transformation actually makes sense, and shout at the user if you want to help them debugging their turf. The later part is definitely optional.
Hope this helps,

This certainly helps a lot, now I have a clear understanding of what is to be done next.

Quoted 3 lines

M.

Signature · 1 line

Jazz is not dead. It just smells funny...

Many thanks for the detailed explanations,
Cristi

Cristian Ciocaltea

author

2020-08-19 16:38 UTC #10 lore raw

PATCH v5 2/3 irqchip: Add Actions Semi Owl SIRQ controller

In reply to Cristian Ciocaltea

This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 and S900) and provides support for handling up to 3 external interrupt lines.

Each line can be independently configured as interrupt and triggers on either of the edges (raising or falling) or either of the levels (high or low). Additionally, each line can also be masked individually.

This is based on the patch series submitted by Parthiban Nallathambi:
https://lore.kernel.org/lkml/[email protected]/

Signed-off-by Parthiban Nallathambi <[email protected]>

Signed-off-by Saravanan Sekar <[email protected]>

[cristi: optimized DT, various fixes/cleanups/improvements]

Signed-off-by Cristian Ciocaltea <[email protected]>

---
Changes in v5 - according to Marc's review:
* Updated commit message
* Aligned members in struct owl_sirq_chip_data
* Added naming for SIRQ0 control register offset
* Improved code readability by using FIELD_PREP and FIELD_GET
* Dropped redundant handling of the IRQ trigger information
* Added missing irq_set_affinity to owl_sirq_chip descriptor

 drivers/irqchip/Makefile       |   1 +
 drivers/irqchip/irq-owl-sirq.c | 347 +++++++++++++++++++++++++++++++++
 2 files changed, 348 insertions(+)
 create mode 100644 drivers/irqchip/irq-owl-sirq.c

drivers/irqchip/Makefile +1 -0

index 133f9c45744a..b8eb5b8b766d 100644
@@ -7,6 +7,7 @@ obj-$(CONFIG_ATH79)			+= irq-ath79-cpu.o
 obj-$(CONFIG_ATH79)			+= irq-ath79-misc.o
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2835.o
 obj-$(CONFIG_ARCH_BCM2835)		+= irq-bcm2836.o
+obj-$(CONFIG_ARCH_ACTIONS)		+= irq-owl-sirq.o
 obj-$(CONFIG_DAVINCI_AINTC)		+= irq-davinci-aintc.o
 obj-$(CONFIG_DAVINCI_CP_INTC)		+= irq-davinci-cp-intc.o
 obj-$(CONFIG_EXYNOS_IRQ_COMBINER)	+= exynos-combiner.o

drivers/irqchip/irq-owl-sirq.c new file +347 -0

347 lines not on this page · raw message

Signature · 1 line

2.28.0

Cristian Ciocaltea

author

2020-08-19 16:38 UTC #11 lore raw

PATCH v5 3/3 MAINTAINERS: Add entries for Actions Semi Owl SIRQ controller

In reply to Cristian Ciocaltea

Add entries for Actions Semi Owl SIRQ controller binding and driver.

Signed-off-by Cristian Ciocaltea <[email protected]>

---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

MAINTAINERS +2 -0

index db97d048a92e..e28d8ad47d03 100644
@@ -1525,6 +1525,7 @@ F:	Documentation/devicetree/bindings/arm/actions.yaml
 F:	Documentation/devicetree/bindings/clock/actions,owl-cmu.txt
 F:	Documentation/devicetree/bindings/dma/owl-dma.txt
 F:	Documentation/devicetree/bindings/i2c/i2c-owl.txt
+F:	Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
 F:	Documentation/devicetree/bindings/mmc/owl-mmc.yaml
 F:	Documentation/devicetree/bindings/pinctrl/actions,*
 F:	Documentation/devicetree/bindings/power/actions,owl-sps.txt

@@ -1536,6 +1537,7 @@ F:	drivers/clk/actions/
 F:	drivers/clocksource/timer-owl*
 F:	drivers/dma/owl-dma.c
 F:	drivers/i2c/busses/i2c-owl.c
+F:	drivers/irqchip/irq-owl-sirq.c
 F:	drivers/mmc/host/owl-mmc.c
 F:	drivers/pinctrl/actions/*
 F:	drivers/soc/actions/

Signature · 1 line

2.28.0

Manivannan Sadhasivam

2020-08-22 13:17 UTC #12 lore raw

In reply to Cristian Ciocaltea

Hi Cristi,

Quoted 9 lines · Cristian Ciocaltea

This patch series adds support for the external interrupt controller (SIRQ) found in the Actions Semi Owl family of SoC's (S500, S700 and S900). The controller handles up to 3 external interrupt lines through dedicated SIRQ pins.
This is a rework of the patch series submitted some time ago by Parthiban Nallathambi:
https://lore.kernel.org/lkml/[email protected]/

You need to preserve the authorship while reposting the patches. If you'd like to take the authorship intentionally then please explain the reason in cover letter.

Thanks,
Mani

Quoted 78 lines

Please note I have dropped, for the moment, the S700 related patches since I do not own a compatible hardware for testing. I'm using instead an S500 SoC based board for which I have already provided the initial support:
https://lore.kernel.org/lkml/[email protected]/
The SIRQ controller support is a prerequisite of the soon to be submitted MFD driver for the Actions Semi ATC260x PMICs.
Thanks and regards,
Cristi
Changes in v5:
- Integrated Marc's review (more details in the driver patch changelog)
- Rebased patch series on v5.9-rc1
Changes in v4:
- Simplified the DTS structure:
* dropped 'actions,sirq-shared-reg' node, now the differentiation between SoC variants is handled now via the compatible property
* dropped 'actions,sirq-reg-offset', now controller base address in DTS points to SIRQ0 register, so no additional information is required for S500 and S700, while for S900 SoC the offsets of SIRQ1 and SIRQ2 regs are provided by the driver
* 'actions,ext-irq-range' was replaced with 'actions,ext-interrupts', an array of the GIC interrupts triggered by the controller
- Fixed wrong INTC_EXTCTL_TYPE_MASK definition
- Removed redundant irq_fwspec checks in owl_sirq_domain_alloc()
- Improved error handling in owl_sirq_of_init()
- Added yaml binding document
- Dropped S700 related DTS patches for lack of testing hardware:
* arm64: dts: actions: Add sirq node for Actions Semi S700
* arm64: dts: actions: s700-cubieboard7: Enable SIRQ
- Updated MAINTAINERS
- Rebased patchset on kernel v5.8
- Cosmetic changes
* Ordered include statements alphabetically
* Added comment to owl_sirq_set_type() describing conversion of falling edge or active low signals
* Replaced IRQF_TRIGGER_* with corresponding IRQ_TYPE_* variants
* Ensured data types and function naming are consistent regarding the 'owl_sirq' prefix
Changes in v3 (Parthiban Nallathambi):
- Set default operating frequency to 24MHz
- Falling edge and Low Level interrupts translated to rising edge and high level
- Introduced common function with lock handling for register read and write
- Used direct GIC interrupt number for interrupt local hwirq and finding offset using DT entry (range) when registers are shared
- Changed irq_ack to irq_eoi
- Added translation method for irq_domain_ops
- Clearing interrupt pending based on bitmask for edge triggered
- Added pinctrl definition for sirq for cubieboard7. This depends on,
https://lore.kernel.org/patchwork/patch/1012859/
Changes in v2 (Parthiban Nallathambi):
- Added SIRQ as hierarchical chip
        GIC <----> SIRQ <----> External interrupt controller/Child devices
- Device binding updates with vendor prefix
- Register sharing handled globally and common init sequence/data for all actions SoC family
Cristian Ciocaltea (3):
dt-bindings: interrupt-controller: Add Actions SIRQ controller binding
irqchip: Add Actions Semi Owl SIRQ controller
MAINTAINERS: Add entries for Actions Semi Owl SIRQ controller
 .../actions,owl-sirq.yaml                     |  68 ++++
 MAINTAINERS                                   |   2 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-owl-sirq.c                | 347 ++++++++++++++++++
 4 files changed, 418 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
 create mode 100644 drivers/irqchip/irq-owl-sirq.c
Signature · 1 line
2.28.0

Cristian Ciocaltea

author

2020-08-22 23:05 UTC #13 lore raw

Hi Mani,

Quoted 13 lines · Manivannan Sadhasivam

Hi Cristi,
Quoted 9 lines · Cristian Ciocaltea
This patch series adds support for the external interrupt controller (SIRQ) found in the Actions Semi Owl family of SoC's (S500, S700 and S900). The controller handles up to 3 external interrupt lines through dedicated SIRQ pins.
This is a rework of the patch series submitted some time ago by Parthiban Nallathambi:
https://lore.kernel.org/lkml/[email protected]/

Quoted 6 lines

You need to preserve the authorship while reposting the patches. If you'd like to take the authorship intentionally then please explain the reason in cover letter.
Thanks,
Mani

Thanks for pointing this out, I was not aware of the procedure - this is actually my very first repost. Could you please indicate how should I proceed to fix this? I had absolutely no intention to take the authorship..

Sorry for the mistake,
Cristi

Quoted 78 lines

Quoted 78 lines
Please note I have dropped, for the moment, the S700 related patches since I do not own a compatible hardware for testing. I'm using instead an S500 SoC based board for which I have already provided the initial support:
https://lore.kernel.org/lkml/[email protected]/
The SIRQ controller support is a prerequisite of the soon to be submitted MFD driver for the Actions Semi ATC260x PMICs.
Thanks and regards,
Cristi
Changes in v5:
- Integrated Marc's review (more details in the driver patch changelog)
- Rebased patch series on v5.9-rc1
Changes in v4:
- Simplified the DTS structure:
* dropped 'actions,sirq-shared-reg' node, now the differentiation between SoC variants is handled now via the compatible property
* dropped 'actions,sirq-reg-offset', now controller base address in DTS points to SIRQ0 register, so no additional information is required for S500 and S700, while for S900 SoC the offsets of SIRQ1 and SIRQ2 regs are provided by the driver
* 'actions,ext-irq-range' was replaced with 'actions,ext-interrupts', an array of the GIC interrupts triggered by the controller
- Fixed wrong INTC_EXTCTL_TYPE_MASK definition
- Removed redundant irq_fwspec checks in owl_sirq_domain_alloc()
- Improved error handling in owl_sirq_of_init()
- Added yaml binding document
- Dropped S700 related DTS patches for lack of testing hardware:
* arm64: dts: actions: Add sirq node for Actions Semi S700
* arm64: dts: actions: s700-cubieboard7: Enable SIRQ
- Updated MAINTAINERS
- Rebased patchset on kernel v5.8
- Cosmetic changes
* Ordered include statements alphabetically
* Added comment to owl_sirq_set_type() describing conversion of falling edge or active low signals
* Replaced IRQF_TRIGGER_* with corresponding IRQ_TYPE_* variants
* Ensured data types and function naming are consistent regarding the 'owl_sirq' prefix
Changes in v3 (Parthiban Nallathambi):
- Set default operating frequency to 24MHz
- Falling edge and Low Level interrupts translated to rising edge and high level
- Introduced common function with lock handling for register read and write
- Used direct GIC interrupt number for interrupt local hwirq and finding offset using DT entry (range) when registers are shared
- Changed irq_ack to irq_eoi
- Added translation method for irq_domain_ops
- Clearing interrupt pending based on bitmask for edge triggered
- Added pinctrl definition for sirq for cubieboard7. This depends on,
https://lore.kernel.org/patchwork/patch/1012859/
Changes in v2 (Parthiban Nallathambi):
- Added SIRQ as hierarchical chip
        GIC <----> SIRQ <----> External interrupt controller/Child devices
- Device binding updates with vendor prefix
- Register sharing handled globally and common init sequence/data for all actions SoC family
Cristian Ciocaltea (3):
dt-bindings: interrupt-controller: Add Actions SIRQ controller binding
irqchip: Add Actions Semi Owl SIRQ controller
MAINTAINERS: Add entries for Actions Semi Owl SIRQ controller
 .../actions,owl-sirq.yaml                     |  68 ++++
 MAINTAINERS                                   |   2 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-owl-sirq.c                | 347 ++++++++++++++++++
 4 files changed, 418 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
 create mode 100644 drivers/irqchip/irq-owl-sirq.c
Signature · 1 line
2.28.0

Manivannan Sadhasivam

2020-08-25 02:09 UTC #14 lore raw

Quoted 25 lines · Cristian Ciocaltea

Hi Mani,
Quoted 4 lines · Manivannan Sadhasivam
Hi Cristi,
Quoted 1 line · Cristian Ciocaltea
This patch series adds support for the external interrupt
controller
Quoted 1 line
Quoted 1 line
(SIRQ) found in the Actions Semi Owl family of SoC's (S500, S700
and
Quoted 1 line
Quoted 1 line
S900). The controller handles up to 3 external interrupt lines
through
Quoted 8 lines
Quoted 6 lines
dedicated SIRQ pins.
This is a rework of the patch series submitted some time ago by Parthiban Nallathambi:
https://lore.kernel.org/lkml/[email protected]/
You need to preserve the authorship while reposting the patches. If
you'd
Quoted 1 line
like to take the authorship intentionally then please explain the
reason in
Quoted 2 lines
cover letter.

Quoted 9 lines

Quoted 2 lines
Thanks,
Mani
Thanks for pointing this out, I was not aware of the procedure - this is
actually my very first repost. Could you please indicate how should I proceed to fix this? I had absolutely no intention to take the authorship..

Below command would change the author of last commit:

git commit --amend --author="Manivannan Sadhasivam <[email protected]>"

Quoted 1 line

Sorry for the mistake,

No issues!

Thanks,
Mani

Quoted 100 lines

Cristi
Quoted 1 line
Quoted 1 line
Please note I have dropped, for the moment, the S700 related
patches
Quoted 1 line
Quoted 1 line
since I do not own a compatible hardware for testing. I'm using
instead
Quoted 1 line
Quoted 1 line
an S500 SoC based board for which I have already provided the
initial
Quoted 2 lines
Quoted 2 lines
support:
https://lore.kernel.org/lkml/[email protected]/
Quoted 2 lines
Quoted 2 lines
The SIRQ controller support is a prerequisite of the soon to be
submitted
Quoted 7 lines
Quoted 7 lines
MFD driver for the Actions Semi ATC260x PMICs.
Thanks and regards,
Cristi
Changes in v5:
- Integrated Marc's review (more details in the driver patch
changelog)
Quoted 7 lines
Quoted 7 lines
- Rebased patch series on v5.9-rc1
Changes in v4:
- Simplified the DTS structure:
* dropped 'actions,sirq-shared-reg' node, now the differentiation between SoC variants is handled now via the compatible property
* dropped 'actions,sirq-reg-offset', now controller base address
in
Quoted 2 lines
Quoted 2 lines
DTS points to SIRQ0 register, so no additional information is required for S500 and S700, while for S900 SoC the offsets of
SIRQ1
Quoted 2 lines
Quoted 2 lines
and SIRQ2 regs are provided by the driver
* 'actions,ext-irq-range' was replaced with
'actions,ext-interrupts',
Quoted 5 lines
Quoted 5 lines
an array of the GIC interrupts triggered by the controller
- Fixed wrong INTC_EXTCTL_TYPE_MASK definition
- Removed redundant irq_fwspec checks in owl_sirq_domain_alloc()
- Improved error handling in owl_sirq_of_init()
- Added yaml binding document
Quoted 8 lines
Quoted 8 lines
- Dropped S700 related DTS patches for lack of testing hardware:
* arm64: dts: actions: Add sirq node for Actions Semi S700
* arm64: dts: actions: s700-cubieboard7: Enable SIRQ
- Updated MAINTAINERS
- Rebased patchset on kernel v5.8
- Cosmetic changes
* Ordered include statements alphabetically
* Added comment to owl_sirq_set_type() describing conversion of
falling
Quoted 3 lines
Quoted 3 lines
edge or active low signals
* Replaced IRQF_TRIGGER_* with corresponding IRQ_TYPE_* variants
* Ensured data types and function naming are consistent regarding
the
Quoted 5 lines
Quoted 5 lines
'owl_sirq' prefix
Changes in v3 (Parthiban Nallathambi):
- Set default operating frequency to 24MHz
- Falling edge and Low Level interrupts translated to rising edge
and high level
Quoted 1 line
Quoted 1 line
- Introduced common function with lock handling for register read
and write
Quoted 1 line
Quoted 1 line
- Used direct GIC interrupt number for interrupt local hwirq and
finding offset
Quoted 5 lines
Quoted 5 lines
using DT entry (range) when registers are shared
- Changed irq_ack to irq_eoi
- Added translation method for irq_domain_ops
- Clearing interrupt pending based on bitmask for edge triggered
- Added pinctrl definition for sirq for cubieboard7. This depends
on,
Quoted 5 lines
Quoted 5 lines
https://lore.kernel.org/patchwork/patch/1012859/
Changes in v2 (Parthiban Nallathambi):
- Added SIRQ as hierarchical chip
        GIC <----> SIRQ <----> External interrupt controller/Child
devices
Quoted 2 lines
Quoted 2 lines
- Device binding updates with vendor prefix
- Register sharing handled globally and common init sequence/data
for all
Quoted 4 lines
Quoted 4 lines
actions SoC family
Cristian Ciocaltea (3):
dt-bindings: interrupt-controller: Add Actions SIRQ controller
binding
Quoted 7 lines
Quoted 7 lines
irqchip: Add Actions Semi Owl SIRQ controller
MAINTAINERS: Add entries for Actions Semi Owl SIRQ controller
 .../actions,owl-sirq.yaml                     |  68 ++++
 MAINTAINERS                                   |   2 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-owl-sirq.c                | 347
++++++++++++++++++
Quoted 2 lines
Quoted 2 lines
 4 files changed, 418 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
Quoted 5 lines
Quoted 5 lines
 create mode 100644 drivers/irqchip/irq-owl-sirq.c
Signature · 1 line
2.28.0

Signature · 1 line

Sent from my Android device with K-9 Mail. Please excuse my brevity.

Cristian Ciocaltea

author

2020-08-25 09:44 UTC #15 lore raw

Quoted 38 lines · Manivannan Sadhasivam

Quoted 25 lines · Cristian Ciocaltea
Hi Mani,
Quoted 4 lines · Manivannan Sadhasivam
Hi Cristi,
Quoted 1 line · Cristian Ciocaltea
This patch series adds support for the external interrupt
controller
Quoted 1 line
Quoted 1 line
(SIRQ) found in the Actions Semi Owl family of SoC's (S500, S700
and
Quoted 1 line
Quoted 1 line
S900). The controller handles up to 3 external interrupt lines
through
Quoted 8 lines
Quoted 6 lines
dedicated SIRQ pins.
This is a rework of the patch series submitted some time ago by Parthiban Nallathambi:
https://lore.kernel.org/lkml/[email protected]/
You need to preserve the authorship while reposting the patches. If
you'd
Quoted 1 line
like to take the authorship intentionally then please explain the
reason in
Quoted 2 lines
cover letter.
Quoted 9 lines
Quoted 2 lines
Thanks,
Mani
Thanks for pointing this out, I was not aware of the procedure - this is
actually my very first repost. Could you please indicate how should I proceed to fix this? I had absolutely no intention to take the authorship..

Quoted 3 lines

Below command would change the author of last commit:
git commit --amend --author="Manivannan Sadhasivam <[email protected]>"

Got it now, for some reason I have lost original author information in my local repository - I'll pay more attention to this next time. I will submit the fix after receiving Rob's review for the binding doc, if that would be ok.

Thanks,
Cristi

Quoted 110 lines

Quoted 1 line
Sorry for the mistake,
No issues!
Thanks,
Mani
Quoted 100 lines
Cristi
Quoted 1 line
Quoted 1 line
Please note I have dropped, for the moment, the S700 related
patches
Quoted 1 line
Quoted 1 line
since I do not own a compatible hardware for testing. I'm using
instead
Quoted 1 line
Quoted 1 line
an S500 SoC based board for which I have already provided the
initial
Quoted 2 lines
Quoted 2 lines
support:
https://lore.kernel.org/lkml/[email protected]/
Quoted 2 lines
Quoted 2 lines
The SIRQ controller support is a prerequisite of the soon to be
submitted
Quoted 7 lines
Quoted 7 lines
MFD driver for the Actions Semi ATC260x PMICs.
Thanks and regards,
Cristi
Changes in v5:
- Integrated Marc's review (more details in the driver patch
changelog)
Quoted 7 lines
Quoted 7 lines
- Rebased patch series on v5.9-rc1
Changes in v4:
- Simplified the DTS structure:
* dropped 'actions,sirq-shared-reg' node, now the differentiation between SoC variants is handled now via the compatible property
* dropped 'actions,sirq-reg-offset', now controller base address
in
Quoted 2 lines
Quoted 2 lines
DTS points to SIRQ0 register, so no additional information is required for S500 and S700, while for S900 SoC the offsets of
SIRQ1
Quoted 2 lines
Quoted 2 lines
and SIRQ2 regs are provided by the driver
* 'actions,ext-irq-range' was replaced with
'actions,ext-interrupts',
Quoted 5 lines
Quoted 5 lines
an array of the GIC interrupts triggered by the controller
- Fixed wrong INTC_EXTCTL_TYPE_MASK definition
- Removed redundant irq_fwspec checks in owl_sirq_domain_alloc()
- Improved error handling in owl_sirq_of_init()
- Added yaml binding document
Quoted 8 lines
Quoted 8 lines
- Dropped S700 related DTS patches for lack of testing hardware:
* arm64: dts: actions: Add sirq node for Actions Semi S700
* arm64: dts: actions: s700-cubieboard7: Enable SIRQ
- Updated MAINTAINERS
- Rebased patchset on kernel v5.8
- Cosmetic changes
* Ordered include statements alphabetically
* Added comment to owl_sirq_set_type() describing conversion of
falling
Quoted 3 lines
Quoted 3 lines
edge or active low signals
* Replaced IRQF_TRIGGER_* with corresponding IRQ_TYPE_* variants
* Ensured data types and function naming are consistent regarding
the
Quoted 5 lines
Quoted 5 lines
'owl_sirq' prefix
Changes in v3 (Parthiban Nallathambi):
- Set default operating frequency to 24MHz
- Falling edge and Low Level interrupts translated to rising edge
and high level
Quoted 1 line
Quoted 1 line
- Introduced common function with lock handling for register read
and write
Quoted 1 line
Quoted 1 line
- Used direct GIC interrupt number for interrupt local hwirq and
finding offset
Quoted 5 lines
Quoted 5 lines
using DT entry (range) when registers are shared
- Changed irq_ack to irq_eoi
- Added translation method for irq_domain_ops
- Clearing interrupt pending based on bitmask for edge triggered
- Added pinctrl definition for sirq for cubieboard7. This depends
on,
Quoted 5 lines
Quoted 5 lines
https://lore.kernel.org/patchwork/patch/1012859/
Changes in v2 (Parthiban Nallathambi):
- Added SIRQ as hierarchical chip
        GIC <----> SIRQ <----> External interrupt controller/Child
devices
Quoted 2 lines
Quoted 2 lines
- Device binding updates with vendor prefix
- Register sharing handled globally and common init sequence/data
for all
Quoted 4 lines
Quoted 4 lines
actions SoC family
Cristian Ciocaltea (3):
dt-bindings: interrupt-controller: Add Actions SIRQ controller
binding
Quoted 7 lines
Quoted 7 lines
irqchip: Add Actions Semi Owl SIRQ controller
MAINTAINERS: Add entries for Actions Semi Owl SIRQ controller
 .../actions,owl-sirq.yaml                     |  68 ++++
 MAINTAINERS                                   |   2 +
 drivers/irqchip/Makefile                      |   1 +
 drivers/irqchip/irq-owl-sirq.c                | 347
++++++++++++++++++
Quoted 2 lines
Quoted 2 lines
 4 files changed, 418 insertions(+)
 create mode 100644
Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml
Quoted 5 lines
Quoted 5 lines
 create mode 100644 drivers/irqchip/irq-owl-sirq.c
Signature · 1 line
2.28.0
Signature · 1 line
Sent from my Android device with K-9 Mail. Please excuse my brevity.