* [PATCH v3 0/2] Add support for AST2700 INTC driver @ 2024-10-09 11:58 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-09 11:58 UTC (permalink / raw) To: linux-aspeed --- v3: aspeed,ast2700-intc.yaml: - Change reg with maxIntems:10 only. - Change interrupt with maxIntems:10 only. - Add description of INT0 and INTC1. - Fix the indentation. drivers/irqchip/irq-aspeed-intc.c: - Change to use CONFIG_ARCH_ASPEED. - Fix indentation. - Remove irq_domain name assignment. - Check all irq numbers first, and set chained. Kevin Chen (2): dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC irqchip/aspeed-intc: Add support for AST27XX INTC .../aspeed,ast2700-intc.yaml | 87 +++++++++++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-aspeed-intc.c | 139 ++++++++++++++++++ 3 files changed, 227 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml create mode 100644 drivers/irqchip/irq-aspeed-intc.c -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 0/2] Add support for AST2700 INTC driver @ 2024-10-09 11:58 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-09 11:58 UTC (permalink / raw) To: tglx, robh, krzk+dt, conor+dt, joel, andrew, kevin_chen, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed --- v3: aspeed,ast2700-intc.yaml: - Change reg with maxIntems:10 only. - Change interrupt with maxIntems:10 only. - Add description of INT0 and INTC1. - Fix the indentation. drivers/irqchip/irq-aspeed-intc.c: - Change to use CONFIG_ARCH_ASPEED. - Fix indentation. - Remove irq_domain name assignment. - Check all irq numbers first, and set chained. Kevin Chen (2): dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC irqchip/aspeed-intc: Add support for AST27XX INTC .../aspeed,ast2700-intc.yaml | 87 +++++++++++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-aspeed-intc.c | 139 ++++++++++++++++++ 3 files changed, 227 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml create mode 100644 drivers/irqchip/irq-aspeed-intc.c -- 2.34.1 ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC 2024-10-09 11:58 ` Kevin Chen @ 2024-10-09 11:58 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-09 11:58 UTC (permalink / raw) To: linux-aspeed The ASPEED AST27XX interrupt controller(INTC) contain second level and third level interrupt controller. INTC0: The second level INTC, which used to assert GIC(#192~#197) if interrupt in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0. INTC1_x: The third level INTC, which used to assert GIC(#192~#197) if interrupt in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0. Signed-off-by: Kevin Chen <kevin_chen@aspeedtech.com> --- .../aspeed,ast2700-intc.yaml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml new file mode 100644 index 000000000000..650a1f6e1177 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Aspeed AST2700 Interrupt Controller + +description: + This interrupt controller hardware is second level interrupt controller that + is hooked to a parent interrupt controller. It's useful to combine multiple + interrupt sources into 1 interrupt to parent interrupt controller. + +maintainers: + - Kevin Chen <kevin_chen@aspeedtech.com> + +properties: + compatible: + enum: + - aspeed,ast2700-intc-ic + + reg: + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + const: 2 + + interrupts: + maxItems: 10 + description: + Depend to which INTC0 or INTC1 used. + INTC0 and INTC1 are two kinds of interrupt controller with enable and raw + status registers for use. + INTC0 is used to assert GIC if interrupt in INTC1 asserted. + INTC1 is used to assert INTC0 if interrupt of modules asserted. + +-----+ +-------+ +---------+---module0 + | GIC |---| INTC0 |--+--| INTC1_0 |---module2 + | | | | | | |---... + +-----+ +-------+ | +---------+---module31 + | + | +---------+---module0 + +---| INTC1_1 |---module2 + | | |---... + | +---------+---module31 + ... + | +---------+---module0 + +---| INTC1_5 |---module2 + | |---... + +---------+---module31 + + +required: + - compatible + - reg + - interrupt-controller + - '#interrupt-cells' + - interrupts + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + + interrupt-controller at 12101b00 { + compatible = "aspeed,ast2700-intc-ic"; + reg = <0 0x12101b00 0 0x10>; + #interrupt-cells = <2>; + interrupt-controller; + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; + }; + }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC @ 2024-10-09 11:58 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-09 11:58 UTC (permalink / raw) To: tglx, robh, krzk+dt, conor+dt, joel, andrew, kevin_chen, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed The ASPEED AST27XX interrupt controller(INTC) contain second level and third level interrupt controller. INTC0: The second level INTC, which used to assert GIC(#192~#197) if interrupt in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0. INTC1_x: The third level INTC, which used to assert GIC(#192~#197) if interrupt in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0. Signed-off-by: Kevin Chen <kevin_chen@aspeedtech.com> --- .../aspeed,ast2700-intc.yaml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml new file mode 100644 index 000000000000..650a1f6e1177 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Aspeed AST2700 Interrupt Controller + +description: + This interrupt controller hardware is second level interrupt controller that + is hooked to a parent interrupt controller. It's useful to combine multiple + interrupt sources into 1 interrupt to parent interrupt controller. + +maintainers: + - Kevin Chen <kevin_chen@aspeedtech.com> + +properties: + compatible: + enum: + - aspeed,ast2700-intc-ic + + reg: + maxItems: 1 + + interrupt-controller: true + + '#interrupt-cells': + const: 2 + + interrupts: + maxItems: 10 + description: + Depend to which INTC0 or INTC1 used. + INTC0 and INTC1 are two kinds of interrupt controller with enable and raw + status registers for use. + INTC0 is used to assert GIC if interrupt in INTC1 asserted. + INTC1 is used to assert INTC0 if interrupt of modules asserted. + +-----+ +-------+ +---------+---module0 + | GIC |---| INTC0 |--+--| INTC1_0 |---module2 + | | | | | | |---... + +-----+ +-------+ | +---------+---module31 + | + | +---------+---module0 + +---| INTC1_1 |---module2 + | | |---... + | +---------+---module31 + ... + | +---------+---module0 + +---| INTC1_5 |---module2 + | |---... + +---------+---module31 + + +required: + - compatible + - reg + - interrupt-controller + - '#interrupt-cells' + - interrupts + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + bus { + #address-cells = <2>; + #size-cells = <2>; + + interrupt-controller@12101b00 { + compatible = "aspeed,ast2700-intc-ic"; + reg = <0 0x12101b00 0 0x10>; + #interrupt-cells = <2>; + interrupt-controller; + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; + }; + }; -- 2.34.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC 2024-10-09 11:58 ` Kevin Chen @ 2024-10-09 12:56 ` Markus Elfring -1 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-09 12:56 UTC (permalink / raw) To: linux-aspeed > The ASPEED AST27XX interrupt controller(INTC) contain second level and ? contains? ? in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in ? numbers? Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC @ 2024-10-09 12:56 ` Markus Elfring 0 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-09 12:56 UTC (permalink / raw) To: Kevin Chen, linux-aspeed, linux-arm-kernel, devicetree, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner Cc: LKML > The ASPEED AST27XX interrupt controller(INTC) contain second level and … contains? … in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in … numbers? Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC 2024-10-09 11:58 ` Kevin Chen @ 2024-10-09 21:11 ` Rob Herring -1 siblings, 0 replies; 36+ messages in thread From: Rob Herring @ 2024-10-09 21:11 UTC (permalink / raw) To: linux-aspeed On Wed, Oct 09, 2024 at 07:58:12PM +0800, Kevin Chen wrote: > The ASPEED AST27XX interrupt controller(INTC) contain second level and > third level interrupt controller. > > INTC0: > The second level INTC, which used to assert GIC(#192~#197) if interrupt > in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in > one INTC0. > > INTC1_x: > The third level INTC, which used to assert GIC(#192~#197) if interrupt in > INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0. > > Signed-off-by: Kevin Chen <kevin_chen@aspeedtech.com> > --- > .../aspeed,ast2700-intc.yaml | 87 +++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml > new file mode 100644 > index 000000000000..650a1f6e1177 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Aspeed AST2700 Interrupt Controller > + > +description: > + This interrupt controller hardware is second level interrupt controller that > + is hooked to a parent interrupt controller. It's useful to combine multiple > + interrupt sources into 1 interrupt to parent interrupt controller. > + > +maintainers: > + - Kevin Chen <kevin_chen@aspeedtech.com> > + > +properties: > + compatible: > + enum: > + - aspeed,ast2700-intc-ic > + > + reg: > + maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 2 Describe the meaning of the cells here. > + > + interrupts: > + maxItems: 10 > + description: You need '|' to preserve formatting. > + Depend to which INTC0 or INTC1 used. > + INTC0 and INTC1 are two kinds of interrupt controller with enable and raw > + status registers for use. > + INTC0 is used to assert GIC if interrupt in INTC1 asserted. > + INTC1 is used to assert INTC0 if interrupt of modules asserted. > + +-----+ +-------+ +---------+---module0 > + | GIC |---| INTC0 |--+--| INTC1_0 |---module2 > + | | | | | | |---... > + +-----+ +-------+ | +---------+---module31 > + | > + | +---------+---module0 > + +---| INTC1_1 |---module2 > + | | |---... > + | +---------+---module31 > + ... > + | +---------+---module0 > + +---| INTC1_5 |---module2 > + | |---... > + +---------+---module31 > + > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + interrupt-controller at 12101b00 { > + compatible = "aspeed,ast2700-intc-ic"; > + reg = <0 0x12101b00 0 0x10>; > + #interrupt-cells = <2>; > + interrupt-controller; > + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; > + }; > + }; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC @ 2024-10-09 21:11 ` Rob Herring 0 siblings, 0 replies; 36+ messages in thread From: Rob Herring @ 2024-10-09 21:11 UTC (permalink / raw) To: Kevin Chen Cc: tglx, krzk+dt, conor+dt, joel, andrew, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Wed, Oct 09, 2024 at 07:58:12PM +0800, Kevin Chen wrote: > The ASPEED AST27XX interrupt controller(INTC) contain second level and > third level interrupt controller. > > INTC0: > The second level INTC, which used to assert GIC(#192~#197) if interrupt > in INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in > one INTC0. > > INTC1_x: > The third level INTC, which used to assert GIC(#192~#197) if interrupt in > INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one INTC0. > > Signed-off-by: Kevin Chen <kevin_chen@aspeedtech.com> > --- > .../aspeed,ast2700-intc.yaml | 87 +++++++++++++++++++ > 1 file changed, 87 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml > new file mode 100644 > index 000000000000..650a1f6e1177 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700-intc.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-intc.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Aspeed AST2700 Interrupt Controller > + > +description: > + This interrupt controller hardware is second level interrupt controller that > + is hooked to a parent interrupt controller. It's useful to combine multiple > + interrupt sources into 1 interrupt to parent interrupt controller. > + > +maintainers: > + - Kevin Chen <kevin_chen@aspeedtech.com> > + > +properties: > + compatible: > + enum: > + - aspeed,ast2700-intc-ic > + > + reg: > + maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 2 Describe the meaning of the cells here. > + > + interrupts: > + maxItems: 10 > + description: You need '|' to preserve formatting. > + Depend to which INTC0 or INTC1 used. > + INTC0 and INTC1 are two kinds of interrupt controller with enable and raw > + status registers for use. > + INTC0 is used to assert GIC if interrupt in INTC1 asserted. > + INTC1 is used to assert INTC0 if interrupt of modules asserted. > + +-----+ +-------+ +---------+---module0 > + | GIC |---| INTC0 |--+--| INTC1_0 |---module2 > + | | | | | | |---... > + +-----+ +-------+ | +---------+---module31 > + | > + | +---------+---module0 > + +---| INTC1_1 |---module2 > + | | |---... > + | +---------+---module31 > + ... > + | +---------+---module0 > + +---| INTC1_5 |---module2 > + | |---... > + +---------+---module31 > + > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - interrupts > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + bus { > + #address-cells = <2>; > + #size-cells = <2>; > + > + interrupt-controller@12101b00 { > + compatible = "aspeed,ast2700-intc-ic"; > + reg = <0 0x12101b00 0 0x10>; > + #interrupt-cells = <2>; > + interrupt-controller; > + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; > + }; > + }; > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC 2024-10-09 21:11 ` Rob Herring @ 2024-10-11 10:02 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-11 10:02 UTC (permalink / raw) To: linux-aspeed > > The ASPEED AST27XX interrupt controller(INTC) contain second level and > > third level interrupt controller. > > > > INTC0: > > The second level INTC, which used to assert GIC(#192~#197) if > > interrupt in INTC1 asserted. There are 6 GIC interrupt > > number(#192~#197) used in one INTC0. > > > > INTC1_x: > > The third level INTC, which used to assert GIC(#192~#197) if interrupt > > in > > INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one > INTC0. > > > > Signed-off-by: Kevin Chen <kevin_chen@aspeedtech.com> > > --- > > .../aspeed,ast2700-intc.yaml | 87 > +++++++++++++++++++ > > 1 file changed, 87 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700- > > intc.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270 > > 0-intc.yaml > > b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270 > > 0-intc.yaml > > new file mode 100644 > > index 000000000000..650a1f6e1177 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,as > > +++ t2700-intc.yaml > > @@ -0,0 +1,87 @@ > > +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-int > > +c.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Aspeed AST2700 Interrupt Controller > > + > > +description: > > + This interrupt controller hardware is second level interrupt > > +controller that > > + is hooked to a parent interrupt controller. It's useful to combine > > +multiple > > + interrupt sources into 1 interrupt to parent interrupt controller. > > + > > +maintainers: > > + - Kevin Chen <kevin_chen@aspeedtech.com> > > + > > +properties: > > + compatible: > > + enum: > > + - aspeed,ast2700-intc-ic > > + > > + reg: > > + maxItems: 1 > > + > > + interrupt-controller: true > > + > > + '#interrupt-cells': > > + const: 2 > > Describe the meaning of the cells here. I would change to the following code + '#interrupt-cells': + const: 2 + description: + The first cell is the IRQ number, the second cell is the trigger + type as defined in interrupt.txt in this directory. > > > + > > + interrupts: > > + maxItems: 10 > > + description: > > You need '|' to preserve formatting. OK. Thanks > > > + Depend to which INTC0 or INTC1 used. > > + INTC0 and INTC1 are two kinds of interrupt controller with enable > and raw > > + status registers for use. > > + INTC0 is used to assert GIC if interrupt in INTC1 asserted. > > + INTC1 is used to assert INTC0 if interrupt of modules asserted. > > + +-----+ +-------+ +---------+---module0 > > + | GIC |---| INTC0 |--+--| INTC1_0 |---module2 > > + | | | | | | |---... > > + +-----+ +-------+ | +---------+---module31 > > + | > > + | +---------+---module0 > > + +---| INTC1_1 |---module2 > > + | | |---... > > + | +---------+---module31 > > + ... > > + | +---------+---module0 > > + +---| INTC1_5 |---module2 > > + | |---... > > + +---------+---module31 > > + > > + > > +required: > > + - compatible > > + - reg > > + - interrupt-controller > > + - '#interrupt-cells' > > + - interrupts > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + bus { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + interrupt-controller at 12101b00 { > > + compatible = "aspeed,ast2700-intc-ic"; > > + reg = <0 0x12101b00 0 0x10>; > > + #interrupt-cells = <2>; > > + interrupt-controller; > > + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC @ 2024-10-11 10:02 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-11 10:02 UTC (permalink / raw) To: Rob Herring Cc: tglx@linutronix.de, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, BMC-SW > > The ASPEED AST27XX interrupt controller(INTC) contain second level and > > third level interrupt controller. > > > > INTC0: > > The second level INTC, which used to assert GIC(#192~#197) if > > interrupt in INTC1 asserted. There are 6 GIC interrupt > > number(#192~#197) used in one INTC0. > > > > INTC1_x: > > The third level INTC, which used to assert GIC(#192~#197) if interrupt > > in > > INTC1 asserted. There are 6 GIC interrupt number(#192~#197) used in one > INTC0. > > > > Signed-off-by: Kevin Chen <kevin_chen@aspeedtech.com> > > --- > > .../aspeed,ast2700-intc.yaml | 87 > +++++++++++++++++++ > > 1 file changed, 87 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2700- > > intc.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270 > > 0-intc.yaml > > b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast270 > > 0-intc.yaml > > new file mode 100644 > > index 000000000000..650a1f6e1177 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,as > > +++ t2700-intc.yaml > > @@ -0,0 +1,87 @@ > > +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause %YAML 1.2 > > +--- > > +$id: > > +http://devicetree.org/schemas/interrupt-controller/aspeed,ast2700-int > > +c.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Aspeed AST2700 Interrupt Controller > > + > > +description: > > + This interrupt controller hardware is second level interrupt > > +controller that > > + is hooked to a parent interrupt controller. It's useful to combine > > +multiple > > + interrupt sources into 1 interrupt to parent interrupt controller. > > + > > +maintainers: > > + - Kevin Chen <kevin_chen@aspeedtech.com> > > + > > +properties: > > + compatible: > > + enum: > > + - aspeed,ast2700-intc-ic > > + > > + reg: > > + maxItems: 1 > > + > > + interrupt-controller: true > > + > > + '#interrupt-cells': > > + const: 2 > > Describe the meaning of the cells here. I would change to the following code + '#interrupt-cells': + const: 2 + description: + The first cell is the IRQ number, the second cell is the trigger + type as defined in interrupt.txt in this directory. > > > + > > + interrupts: > > + maxItems: 10 > > + description: > > You need '|' to preserve formatting. OK. Thanks > > > + Depend to which INTC0 or INTC1 used. > > + INTC0 and INTC1 are two kinds of interrupt controller with enable > and raw > > + status registers for use. > > + INTC0 is used to assert GIC if interrupt in INTC1 asserted. > > + INTC1 is used to assert INTC0 if interrupt of modules asserted. > > + +-----+ +-------+ +---------+---module0 > > + | GIC |---| INTC0 |--+--| INTC1_0 |---module2 > > + | | | | | | |---... > > + +-----+ +-------+ | +---------+---module31 > > + | > > + | +---------+---module0 > > + +---| INTC1_1 |---module2 > > + | | |---... > > + | +---------+---module31 > > + ... > > + | +---------+---module0 > > + +---| INTC1_5 |---module2 > > + | |---... > > + +---------+---module31 > > + > > + > > +required: > > + - compatible > > + - reg > > + - interrupt-controller > > + - '#interrupt-cells' > > + - interrupts > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/interrupt-controller/arm-gic.h> > > + > > + bus { > > + #address-cells = <2>; > > + #size-cells = <2>; > > + > > + interrupt-controller@12101b00 { > > + compatible = "aspeed,ast2700-intc-ic"; > > + reg = <0 0x12101b00 0 0x10>; > > + #interrupt-cells = <2>; > > + interrupt-controller; > > + interrupts = <GIC_SPI 192 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 193 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 194 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 195 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 196 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 197 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 198 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 199 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 200 IRQ_TYPE_LEVEL_HIGH>, > > + <GIC_SPI 201 IRQ_TYPE_LEVEL_HIGH>; > > + }; > > + }; > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-09 11:58 ` Kevin Chen @ 2024-10-09 11:58 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-09 11:58 UTC (permalink / raw) To: linux-aspeed Support for the Aspeed Interrupt Controller found on Aspeed Silicon SoCs, such as the AST2700, which is arm64 architecture. To support ASPEED interrupt controller(INTC) maps the internal interrupt sources of the AST27XX device to an parent interrupt controller. --- drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-aspeed-intc.c | 139 ++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 drivers/irqchip/irq-aspeed-intc.c diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index e3679ec2b9f7..086911bf4db6 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,6 +84,7 @@ obj-$(CONFIG_MVEBU_SEI) += irq-mvebu-sei.o obj-$(CONFIG_LS_EXTIRQ) += irq-ls-extirq.o obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o irq-aspeed-scu-ic.o +obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-intc.o obj-$(CONFIG_STM32MP_EXTI) += irq-stm32mp-exti.o obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c new file mode 100644 index 000000000000..ef1c095ad09e --- /dev/null +++ b/drivers/irqchip/irq-aspeed-intc.c @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Aspeed Interrupt Controller. + * + * Copyright (C) 2023 ASPEED Technology Inc. + */ + +#include <linux/bitops.h> +#include <linux/irq.h> +#include <linux/irqchip.h> +#include <linux/irqchip/chained_irq.h> +#include <linux/irqdomain.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/io.h> +#include <linux/spinlock.h> + +#define INTC_INT_ENABLE_REG 0x00 +#define INTC_INT_STATUS_REG 0x04 +#define IRQS_PER_WORD 32 + +struct aspeed_intc_ic { + void __iomem *base; + raw_spinlock_t gic_lock; + raw_spinlock_t intc_lock; + struct irq_domain *irq_domain; +}; + +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) +{ + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + unsigned long bit, status; + + chained_irq_enter(chip, desc); + + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { + status = readl(intc_ic->base + INTC_INT_STATUS_REG); + for_each_set_bit(bit, &status, IRQS_PER_WORD) { + generic_handle_domain_irq(intc_ic->irq_domain, bit); + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); + } + } + + chained_irq_exit(chip, desc); +} + +static void aspeed_intc_irq_mask(struct irq_data *data) +{ + struct aspeed_intc_ic *intc_ic = irq_data_get_irq_chip_data(data); + unsigned int mask = readl(intc_ic->base + INTC_INT_ENABLE_REG) & ~BIT(data->hwirq); + + guard(raw_spinlock)(&intc_ic->intc_lock); + writel(mask, intc_ic->base + INTC_INT_ENABLE_REG); +} + +static void aspeed_intc_irq_unmask(struct irq_data *data) +{ + struct aspeed_intc_ic *intc_ic = irq_data_get_irq_chip_data(data); + unsigned int unmask = readl(intc_ic->base + INTC_INT_ENABLE_REG) | BIT(data->hwirq); + + guard(raw_spinlock)(&intc_ic->intc_lock); + writel(unmask, intc_ic->base + INTC_INT_ENABLE_REG); +} + +static struct irq_chip aspeed_intc_chip = { + .name = "ASPEED INTC", + .irq_mask = aspeed_intc_irq_mask, + .irq_unmask = aspeed_intc_irq_unmask, +}; + +static int aspeed_intc_ic_map_irq_domain(struct irq_domain *domain, unsigned int irq, + irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &aspeed_intc_chip, handle_level_irq); + irq_set_chip_data(irq, domain->host_data); + + return 0; +} + +static const struct irq_domain_ops aspeed_intc_ic_irq_domain_ops = { + .map = aspeed_intc_ic_map_irq_domain, +}; + +static int __init aspeed_intc_ic_of_init(struct device_node *node, + struct device_node *parent) +{ + struct aspeed_intc_ic *intc_ic; + int ret = 0; + int irq, i; + + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL); + if (!intc_ic) + return -ENOMEM; + + intc_ic->base = of_iomap(node, 0); + if (!intc_ic->base) { + pr_err("Failed to iomap intc_ic base\n"); + ret = -ENOMEM; + goto err_free_ic; + } + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG); + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG); + + intc_ic->irq_domain = irq_domain_add_linear(node, 32, + &aspeed_intc_ic_irq_domain_ops, intc_ic); + if (!intc_ic->irq_domain) { + ret = -ENOMEM; + goto err_iounmap; + } + + raw_spin_lock_init(&intc_ic->gic_lock); + raw_spin_lock_init(&intc_ic->intc_lock); + + /* Check all the irq numbers valid. If not, unmaps all the base and frees the data. */ + for (i = 0; i < of_irq_count(node); i++) { + irq = irq_of_parse_and_map(node, i); + if (!irq) { + pr_err("Failed to get irq number\n"); + ret = -EINVAL; + goto err_iounmap; + } + } + + for (i = 0; i < of_irq_count(node); i++) { + irq = irq_of_parse_and_map(node, i); + irq_set_chained_handler_and_data(irq, aspeed_intc_ic_irq_handler, intc_ic); + } + + return 0; + +err_iounmap: + iounmap(intc_ic->base); +err_free_ic: + kfree(intc_ic); + return ret; +} + +IRQCHIP_DECLARE(ast2700_intc_ic, "aspeed,ast2700-intc-ic", aspeed_intc_ic_of_init); -- 2.34.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-09 11:58 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-09 11:58 UTC (permalink / raw) To: tglx, robh, krzk+dt, conor+dt, joel, andrew, kevin_chen, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed Support for the Aspeed Interrupt Controller found on Aspeed Silicon SoCs, such as the AST2700, which is arm64 architecture. To support ASPEED interrupt controller(INTC) maps the internal interrupt sources of the AST27XX device to an parent interrupt controller. --- drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-aspeed-intc.c | 139 ++++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 drivers/irqchip/irq-aspeed-intc.c diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index e3679ec2b9f7..086911bf4db6 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -84,6 +84,7 @@ obj-$(CONFIG_MVEBU_SEI) += irq-mvebu-sei.o obj-$(CONFIG_LS_EXTIRQ) += irq-ls-extirq.o obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o irq-aspeed-scu-ic.o +obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-intc.o obj-$(CONFIG_STM32MP_EXTI) += irq-stm32mp-exti.o obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c new file mode 100644 index 000000000000..ef1c095ad09e --- /dev/null +++ b/drivers/irqchip/irq-aspeed-intc.c @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Aspeed Interrupt Controller. + * + * Copyright (C) 2023 ASPEED Technology Inc. + */ + +#include <linux/bitops.h> +#include <linux/irq.h> +#include <linux/irqchip.h> +#include <linux/irqchip/chained_irq.h> +#include <linux/irqdomain.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/io.h> +#include <linux/spinlock.h> + +#define INTC_INT_ENABLE_REG 0x00 +#define INTC_INT_STATUS_REG 0x04 +#define IRQS_PER_WORD 32 + +struct aspeed_intc_ic { + void __iomem *base; + raw_spinlock_t gic_lock; + raw_spinlock_t intc_lock; + struct irq_domain *irq_domain; +}; + +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) +{ + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); + struct irq_chip *chip = irq_desc_get_chip(desc); + unsigned long bit, status; + + chained_irq_enter(chip, desc); + + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { + status = readl(intc_ic->base + INTC_INT_STATUS_REG); + for_each_set_bit(bit, &status, IRQS_PER_WORD) { + generic_handle_domain_irq(intc_ic->irq_domain, bit); + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); + } + } + + chained_irq_exit(chip, desc); +} + +static void aspeed_intc_irq_mask(struct irq_data *data) +{ + struct aspeed_intc_ic *intc_ic = irq_data_get_irq_chip_data(data); + unsigned int mask = readl(intc_ic->base + INTC_INT_ENABLE_REG) & ~BIT(data->hwirq); + + guard(raw_spinlock)(&intc_ic->intc_lock); + writel(mask, intc_ic->base + INTC_INT_ENABLE_REG); +} + +static void aspeed_intc_irq_unmask(struct irq_data *data) +{ + struct aspeed_intc_ic *intc_ic = irq_data_get_irq_chip_data(data); + unsigned int unmask = readl(intc_ic->base + INTC_INT_ENABLE_REG) | BIT(data->hwirq); + + guard(raw_spinlock)(&intc_ic->intc_lock); + writel(unmask, intc_ic->base + INTC_INT_ENABLE_REG); +} + +static struct irq_chip aspeed_intc_chip = { + .name = "ASPEED INTC", + .irq_mask = aspeed_intc_irq_mask, + .irq_unmask = aspeed_intc_irq_unmask, +}; + +static int aspeed_intc_ic_map_irq_domain(struct irq_domain *domain, unsigned int irq, + irq_hw_number_t hwirq) +{ + irq_set_chip_and_handler(irq, &aspeed_intc_chip, handle_level_irq); + irq_set_chip_data(irq, domain->host_data); + + return 0; +} + +static const struct irq_domain_ops aspeed_intc_ic_irq_domain_ops = { + .map = aspeed_intc_ic_map_irq_domain, +}; + +static int __init aspeed_intc_ic_of_init(struct device_node *node, + struct device_node *parent) +{ + struct aspeed_intc_ic *intc_ic; + int ret = 0; + int irq, i; + + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL); + if (!intc_ic) + return -ENOMEM; + + intc_ic->base = of_iomap(node, 0); + if (!intc_ic->base) { + pr_err("Failed to iomap intc_ic base\n"); + ret = -ENOMEM; + goto err_free_ic; + } + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG); + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG); + + intc_ic->irq_domain = irq_domain_add_linear(node, 32, + &aspeed_intc_ic_irq_domain_ops, intc_ic); + if (!intc_ic->irq_domain) { + ret = -ENOMEM; + goto err_iounmap; + } + + raw_spin_lock_init(&intc_ic->gic_lock); + raw_spin_lock_init(&intc_ic->intc_lock); + + /* Check all the irq numbers valid. If not, unmaps all the base and frees the data. */ + for (i = 0; i < of_irq_count(node); i++) { + irq = irq_of_parse_and_map(node, i); + if (!irq) { + pr_err("Failed to get irq number\n"); + ret = -EINVAL; + goto err_iounmap; + } + } + + for (i = 0; i < of_irq_count(node); i++) { + irq = irq_of_parse_and_map(node, i); + irq_set_chained_handler_and_data(irq, aspeed_intc_ic_irq_handler, intc_ic); + } + + return 0; + +err_iounmap: + iounmap(intc_ic->base); +err_free_ic: + kfree(intc_ic); + return ret; +} + +IRQCHIP_DECLARE(ast2700_intc_ic, "aspeed,ast2700-intc-ic", aspeed_intc_ic_of_init); -- 2.34.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-09 11:58 ` Kevin Chen @ 2024-10-09 12:32 ` Markus Elfring -1 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-09 12:32 UTC (permalink / raw) To: Kevin Chen, linux-aspeed, linux-arm-kernel, devicetree, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner Cc: LKML, kernel-janitors … > To support ASPEED interrupt controller(INTC) maps the internal interrupt > sources of the AST27XX device to an parent interrupt controller. > --- * I miss your tag “Signed-off-by”. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n396 * How do you think about to choose an additional imperative wording for an improved change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n94 … > +++ b/drivers/irqchip/irq-aspeed-intc.c > @@ -0,0 +1,139 @@ … > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) +{ > + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long bit, status; I suggest to reduce the scopes for three local variables. > + > + chained_irq_enter(chip, desc); Would you become interested to collaborate with another scoped guard for this programming interface? https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained_irq.h#L13 > + > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > + status = readl(intc_ic->base + INTC_INT_STATUS_REG); > + for_each_set_bit(bit, &status, IRQS_PER_WORD) { > + generic_handle_domain_irq(intc_ic->irq_domain, bit); > + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); > + } > + } > + > + chained_irq_exit(chip, desc); > +} Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-09 12:32 ` Markus Elfring 0 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-09 12:32 UTC (permalink / raw) To: linux-aspeed ? > To support ASPEED interrupt controller(INTC) maps the internal interrupt > sources of the AST27XX device to an parent interrupt controller. > --- * I miss your tag ?Signed-off-by?. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n396 * How do you think about to choose an additional imperative wording for an improved change description? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.12-rc2#n94 ? > +++ b/drivers/irqchip/irq-aspeed-intc.c > @@ -0,0 +1,139 @@ ? > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) +{ > + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); > + struct irq_chip *chip = irq_desc_get_chip(desc); > + unsigned long bit, status; I suggest to reduce the scopes for three local variables. > + > + chained_irq_enter(chip, desc); Would you become interested to collaborate with another scoped guard for this programming interface? https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained_irq.h#L13 > + > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > + status = readl(intc_ic->base + INTC_INT_STATUS_REG); > + for_each_set_bit(bit, &status, IRQS_PER_WORD) { > + generic_handle_domain_irq(intc_ic->irq_domain, bit); > + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); > + } > + } > + > + chained_irq_exit(chip, desc); > +} Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-09 12:32 ` Markus Elfring @ 2024-10-11 10:06 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-11 10:06 UTC (permalink / raw) To: Markus Elfring, linux-aspeed@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner, BMC-SW Cc: LKML, kernel-janitors@vger.kernel.org Hi Markus, Thank you for your review. Could you check the message below. Thanks a lot. > > … > > To support ASPEED interrupt controller(INTC) maps the internal > > interrupt sources of the AST27XX device to an parent interrupt controller. > > --- > > * I miss your tag “Signed-off-by”. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume > ntation/process/submitting-patches.rst?h=v6.12-rc2#n396 > > * How do you think about to choose an additional imperative wording > for an improved change description? > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume > ntation/process/submitting-patches.rst?h=v6.12-rc2#n94 > > > … > > +++ b/drivers/irqchip/irq-aspeed-intc.c > > @@ -0,0 +1,139 @@ > … > > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > +{ > > + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > + unsigned long bit, status; > > I suggest to reduce the scopes for three local variables. May I check the scopes of bit and status usage? Variables of bit and status are used in for_each_set_bit. How could I reduce the scopes? > > > > + > > + chained_irq_enter(chip, desc); > > Would you become interested to collaborate with another scoped guard for > this programming interface? > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained > _irq.h#L13 Maybe like the change in the following? diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c index ef1c095ad09e..54d1881c56c6 100644 --- a/drivers/irqchip/irq-aspeed-intc.c +++ b/drivers/irqchip/irq-aspeed-intc.c @@ -32,7 +32,7 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) struct irq_chip *chip = irq_desc_get_chip(desc); unsigned long bit, status; - chained_irq_enter(chip, desc); + guard(chained_irq)(desc); scoped_guard(raw_spinlock, &intc_ic->gic_lock) { status = readl(intc_ic->base + INTC_INT_STATUS_REG); @@ -41,8 +41,6 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); } } - - chained_irq_exit(chip, desc); } static void aspeed_intc_irq_mask(struct irq_data *data) diff --git a/include/linux/irqchip/chained_irq.h b/include/linux/irqchip/chained_irq.h index dd8b3c476666..399a4ae30205 100644 --- a/include/linux/irqchip/chained_irq.h +++ b/include/linux/irqchip/chained_irq.h @@ -38,4 +38,5 @@ static inline void chained_irq_exit(struct irq_chip *chip, chip->irq_unmask(&desc->irq_data); } +DEFINE_GUARD (chained_irq, struct irq_desc * , chained_irq_exit ( _T ->irq_data.chip, _T ), chained_irq_enter (_T->irq_data.chip, _T)) #endif /* __IRQCHIP_CHAINED_IRQ_H */ > > > > + > > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > + status = readl(intc_ic->base + INTC_INT_STATUS_REG); > > + for_each_set_bit(bit, &status, IRQS_PER_WORD) { > > + generic_handle_domain_irq(intc_ic->irq_domain, bit); > > + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); > > + } > > + } > > + > > + chained_irq_exit(chip, desc); > > +} > > > Regards, > Markus ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-11 10:06 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-11 10:06 UTC (permalink / raw) To: linux-aspeed Hi Markus, Thank you for your review. Could you check the message below. Thanks a lot. > > ? > > To support ASPEED interrupt controller(INTC) maps the internal > > interrupt sources of the AST27XX device to an parent interrupt controller. > > --- > > * I miss your tag ?Signed-off-by?. > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume > ntation/process/submitting-patches.rst?h=v6.12-rc2#n396 > > * How do you think about to choose an additional imperative wording > for an improved change description? > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Docume > ntation/process/submitting-patches.rst?h=v6.12-rc2#n94 > > > ? > > +++ b/drivers/irqchip/irq-aspeed-intc.c > > @@ -0,0 +1,139 @@ > ? > > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > +{ > > + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); > > + struct irq_chip *chip = irq_desc_get_chip(desc); > > + unsigned long bit, status; > > I suggest to reduce the scopes for three local variables. May I check the scopes of bit and status usage? Variables of bit and status are used in for_each_set_bit. How could I reduce the scopes? > > > > + > > + chained_irq_enter(chip, desc); > > Would you become interested to collaborate with another scoped guard for > this programming interface? > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained > _irq.h#L13 Maybe like the change in the following? diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c index ef1c095ad09e..54d1881c56c6 100644 --- a/drivers/irqchip/irq-aspeed-intc.c +++ b/drivers/irqchip/irq-aspeed-intc.c @@ -32,7 +32,7 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) struct irq_chip *chip = irq_desc_get_chip(desc); unsigned long bit, status; - chained_irq_enter(chip, desc); + guard(chained_irq)(desc); scoped_guard(raw_spinlock, &intc_ic->gic_lock) { status = readl(intc_ic->base + INTC_INT_STATUS_REG); @@ -41,8 +41,6 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); } } - - chained_irq_exit(chip, desc); } static void aspeed_intc_irq_mask(struct irq_data *data) diff --git a/include/linux/irqchip/chained_irq.h b/include/linux/irqchip/chained_irq.h index dd8b3c476666..399a4ae30205 100644 --- a/include/linux/irqchip/chained_irq.h +++ b/include/linux/irqchip/chained_irq.h @@ -38,4 +38,5 @@ static inline void chained_irq_exit(struct irq_chip *chip, chip->irq_unmask(&desc->irq_data); } +DEFINE_GUARD (chained_irq, struct irq_desc * , chained_irq_exit ( _T ->irq_data.chip, _T ), chained_irq_enter (_T->irq_data.chip, _T)) #endif /* __IRQCHIP_CHAINED_IRQ_H */ > > > > + > > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > + status = readl(intc_ic->base + INTC_INT_STATUS_REG); > > + for_each_set_bit(bit, &status, IRQS_PER_WORD) { > > + generic_handle_domain_irq(intc_ic->irq_domain, bit); > > + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); > > + } > > + } > > + > > + chained_irq_exit(chip, desc); > > +} > > > Regards, > Markus ^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-11 10:06 ` Kevin Chen @ 2024-10-11 15:34 ` Markus Elfring -1 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-11 15:34 UTC (permalink / raw) To: Kevin Chen, linux-aspeed, linux-arm-kernel, devicetree, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner, BMC-SW Cc: LKML, kernel-janitors >> … >>> +++ b/drivers/irqchip/irq-aspeed-intc.c >>> @@ -0,0 +1,139 @@ >> … >>> +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) >> +{ >>> + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); >>> + struct irq_chip *chip = irq_desc_get_chip(desc); >>> + unsigned long bit, status; >> >> I suggest to reduce the scopes for three local variables. > May I check the scopes of bit and status usage? > Variables of bit and status are used in for_each_set_bit. > How could I reduce the scopes? I propose to move selected variable definitions into corresponding compound statements (by using extra curly brackets). https://refactoring.com/catalog/reduceScopeOfVariable.html >> Would you become interested to collaborate with another scoped guard for >> this programming interface? >> https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained >> _irq.h#L13 > > Maybe like the change in the following? > > diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c > index ef1c095ad09e..54d1881c56c6 100644 > --- a/drivers/irqchip/irq-aspeed-intc.c > +++ b/drivers/irqchip/irq-aspeed-intc.c > @@ -32,7 +32,7 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > struct irq_chip *chip = irq_desc_get_chip(desc); > unsigned long bit, status; > > - chained_irq_enter(chip, desc); > + guard(chained_irq)(desc); > > scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > status = readl(intc_ic->base + INTC_INT_STATUS_REG); Perhaps. > @@ -41,8 +41,6 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); > } > } > - > - chained_irq_exit(chip, desc); > } … Probably, yes. … > +++ b/include/linux/irqchip/chained_irq.h > @@ -38,4 +38,5 @@ static inline void chained_irq_exit(struct irq_chip *chip, > chip->irq_unmask(&desc->irq_data); > } > > +DEFINE_GUARD (chained_irq, struct irq_desc * , chained_irq_exit ( _T ->irq_data.chip, _T ), chained_irq_enter (_T->irq_data.chip, _T)) … * Such a macro call looks promising. Would you like to omit any space characters before open parentheses? * Would you like to support scoped guard variants accordingly? Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-11 15:34 ` Markus Elfring 0 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-11 15:34 UTC (permalink / raw) To: linux-aspeed >> ? >>> +++ b/drivers/irqchip/irq-aspeed-intc.c >>> @@ -0,0 +1,139 @@ >> ? >>> +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) >> +{ >>> + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); >>> + struct irq_chip *chip = irq_desc_get_chip(desc); >>> + unsigned long bit, status; >> >> I suggest to reduce the scopes for three local variables. > May I check the scopes of bit and status usage? > Variables of bit and status are used in for_each_set_bit. > How could I reduce the scopes? I propose to move selected variable definitions into corresponding compound statements (by using extra curly brackets). https://refactoring.com/catalog/reduceScopeOfVariable.html >> Would you become interested to collaborate with another scoped guard for >> this programming interface? >> https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained >> _irq.h#L13 > > Maybe like the change in the following? > > diff --git a/drivers/irqchip/irq-aspeed-intc.c b/drivers/irqchip/irq-aspeed-intc.c > index ef1c095ad09e..54d1881c56c6 100644 > --- a/drivers/irqchip/irq-aspeed-intc.c > +++ b/drivers/irqchip/irq-aspeed-intc.c > @@ -32,7 +32,7 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > struct irq_chip *chip = irq_desc_get_chip(desc); > unsigned long bit, status; > > - chained_irq_enter(chip, desc); > + guard(chained_irq)(desc); > > scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > status = readl(intc_ic->base + INTC_INT_STATUS_REG); Perhaps. > @@ -41,8 +41,6 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); > } > } > - > - chained_irq_exit(chip, desc); > } ? Probably, yes. ? > +++ b/include/linux/irqchip/chained_irq.h > @@ -38,4 +38,5 @@ static inline void chained_irq_exit(struct irq_chip *chip, > chip->irq_unmask(&desc->irq_data); > } > > +DEFINE_GUARD (chained_irq, struct irq_desc * , chained_irq_exit ( _T ->irq_data.chip, _T ), chained_irq_enter (_T->irq_data.chip, _T)) ? * Such a macro call looks promising. Would you like to omit any space characters before open parentheses? * Would you like to support scoped guard variants accordingly? Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-11 15:34 ` Markus Elfring @ 2024-10-14 2:00 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-14 2:00 UTC (permalink / raw) To: Markus Elfring, linux-aspeed@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner, BMC-SW Cc: LKML, kernel-janitors@vger.kernel.org > >> … > >>> +++ b/drivers/irqchip/irq-aspeed-intc.c > >>> @@ -0,0 +1,139 @@ > >> … > >>> +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > >> +{ > >>> + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); > >>> + struct irq_chip *chip = irq_desc_get_chip(desc); > >>> + unsigned long bit, status; > >> > >> I suggest to reduce the scopes for three local variables. > > May I check the scopes of bit and status usage? > > Variables of bit and status are used in for_each_set_bit. > > How could I reduce the scopes? > > I propose to move selected variable definitions into corresponding compound > statements (by using extra curly brackets). > https://refactoring.com/catalog/reduceScopeOfVariable.html OK. I moved these two local variables into scoped_guard. +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) +{ + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); + + guard(chained_irq)(desc); + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { + unsigned long bit, status; + + status = readl(intc_ic->base + INTC_INT_STATUS_REG); + for_each_set_bit(bit, &status, IRQS_PER_WORD) { + generic_handle_domain_irq(intc_ic->irq_domain, bit); + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); + } + } +} > > > >> Would you become interested to collaborate with another scoped guard > >> for this programming interface? > >> https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqch > >> ip/chained > >> _irq.h#L13 > > > > Maybe like the change in the following? > > > > diff --git a/drivers/irqchip/irq-aspeed-intc.c > > b/drivers/irqchip/irq-aspeed-intc.c > > index ef1c095ad09e..54d1881c56c6 100644 > > --- a/drivers/irqchip/irq-aspeed-intc.c > > +++ b/drivers/irqchip/irq-aspeed-intc.c > > @@ -32,7 +32,7 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc > *desc) > > struct irq_chip *chip = irq_desc_get_chip(desc); > > unsigned long bit, status; > > > > - chained_irq_enter(chip, desc); > > + guard(chained_irq)(desc); > > > > scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > status = readl(intc_ic->base + INTC_INT_STATUS_REG); > > Perhaps. > > > > @@ -41,8 +41,6 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc > *desc) > > writel(BIT(bit), intc_ic->base + > INTC_INT_STATUS_REG); > > } > > } > > - > > - chained_irq_exit(chip, desc); > > } > … > > Probably, yes. > > > … > > +++ b/include/linux/irqchip/chained_irq.h > > @@ -38,4 +38,5 @@ static inline void chained_irq_exit(struct irq_chip *chip, > > chip->irq_unmask(&desc->irq_data); > > } > > > > +DEFINE_GUARD (chained_irq, struct irq_desc * , chained_irq_exit ( _T > > +->irq_data.chip, _T ), chained_irq_enter (_T->irq_data.chip, _T)) > … > > * Such a macro call looks promising. > Would you like to omit any space characters before open parentheses? OK. Fixed. diff --git a/include/linux/irqchip/chained_irq.h b/include/linux/irqchip/chained_irq.h index dd8b3c476666..7ee29e478152 100644 --- a/include/linux/irqchip/chained_irq.h +++ b/include/linux/irqchip/chained_irq.h @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, chip->irq_unmask(&desc->irq_data); } +DEFINE_GUARD(chained_irq, struct irq_desc *, chained_irq_exit((_T->irq_data.chip), (_T)), + chained_irq_enter((_T->irq_data.chip), (_T))) > > * Would you like to support scoped guard variants accordingly? Do you mean that I need to change the MACRO name for this usage? > > > Regards, > Markus ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-14 2:00 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-14 2:00 UTC (permalink / raw) To: linux-aspeed > >> ? > >>> +++ b/drivers/irqchip/irq-aspeed-intc.c > >>> @@ -0,0 +1,139 @@ > >> ? > >>> +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > >> +{ > >>> + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); > >>> + struct irq_chip *chip = irq_desc_get_chip(desc); > >>> + unsigned long bit, status; > >> > >> I suggest to reduce the scopes for three local variables. > > May I check the scopes of bit and status usage? > > Variables of bit and status are used in for_each_set_bit. > > How could I reduce the scopes? > > I propose to move selected variable definitions into corresponding compound > statements (by using extra curly brackets). > https://refactoring.com/catalog/reduceScopeOfVariable.html OK. I moved these two local variables into scoped_guard. +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) +{ + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); + + guard(chained_irq)(desc); + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { + unsigned long bit, status; + + status = readl(intc_ic->base + INTC_INT_STATUS_REG); + for_each_set_bit(bit, &status, IRQS_PER_WORD) { + generic_handle_domain_irq(intc_ic->irq_domain, bit); + writel(BIT(bit), intc_ic->base + INTC_INT_STATUS_REG); + } + } +} > > > >> Would you become interested to collaborate with another scoped guard > >> for this programming interface? > >> https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqch > >> ip/chained > >> _irq.h#L13 > > > > Maybe like the change in the following? > > > > diff --git a/drivers/irqchip/irq-aspeed-intc.c > > b/drivers/irqchip/irq-aspeed-intc.c > > index ef1c095ad09e..54d1881c56c6 100644 > > --- a/drivers/irqchip/irq-aspeed-intc.c > > +++ b/drivers/irqchip/irq-aspeed-intc.c > > @@ -32,7 +32,7 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc > *desc) > > struct irq_chip *chip = irq_desc_get_chip(desc); > > unsigned long bit, status; > > > > - chained_irq_enter(chip, desc); > > + guard(chained_irq)(desc); > > > > scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > status = readl(intc_ic->base + INTC_INT_STATUS_REG); > > Perhaps. > > > > @@ -41,8 +41,6 @@ static void aspeed_intc_ic_irq_handler(struct irq_desc > *desc) > > writel(BIT(bit), intc_ic->base + > INTC_INT_STATUS_REG); > > } > > } > > - > > - chained_irq_exit(chip, desc); > > } > ? > > Probably, yes. > > > ? > > +++ b/include/linux/irqchip/chained_irq.h > > @@ -38,4 +38,5 @@ static inline void chained_irq_exit(struct irq_chip *chip, > > chip->irq_unmask(&desc->irq_data); > > } > > > > +DEFINE_GUARD (chained_irq, struct irq_desc * , chained_irq_exit ( _T > > +->irq_data.chip, _T ), chained_irq_enter (_T->irq_data.chip, _T)) > ? > > * Such a macro call looks promising. > Would you like to omit any space characters before open parentheses? OK. Fixed. diff --git a/include/linux/irqchip/chained_irq.h b/include/linux/irqchip/chained_irq.h index dd8b3c476666..7ee29e478152 100644 --- a/include/linux/irqchip/chained_irq.h +++ b/include/linux/irqchip/chained_irq.h @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, chip->irq_unmask(&desc->irq_data); } +DEFINE_GUARD(chained_irq, struct irq_desc *, chained_irq_exit((_T->irq_data.chip), (_T)), + chained_irq_enter((_T->irq_data.chip), (_T))) > > * Would you like to support scoped guard variants accordingly? Do you mean that I need to change the MACRO name for this usage? > > > Regards, > Markus ^ permalink raw reply related [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-14 2:00 ` Kevin Chen @ 2024-10-14 13:10 ` Markus Elfring -1 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-14 13:10 UTC (permalink / raw) To: Kevin Chen, linux-aspeed, linux-arm-kernel, devicetree Cc: Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner, kernel-janitors, LKML, BMC-SW > > I propose to move selected variable definitions into corresponding compound > > statements (by using extra curly brackets). > > https://refactoring.com/catalog/reduceScopeOfVariable.html > OK. I moved these two local variables into scoped_guard. Will development interests grow for further refactorings? > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > +{ > + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); Another update candidate (for scope reduction)? > + > + guard(chained_irq)(desc); Using another macro call “scoped_guard(…) { … }”? > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { Would you like to reconsider the proposed macro mixture once more? > + unsigned long bit, status; … … > +++ b/include/linux/irqchip/chained_irq.h > @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, > chip->irq_unmask(&desc->irq_data); > } > > +DEFINE_GUARD(chained_irq, struct irq_desc *, chained_irq_exit((_T->irq_data.chip), (_T)), > + chained_irq_enter((_T->irq_data.chip), (_T))) Would you like to add a #include directive in this header file accordingly? Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-14 13:10 ` Markus Elfring 0 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-14 13:10 UTC (permalink / raw) To: linux-aspeed > > I propose to move selected variable definitions into corresponding compound > > statements (by using extra curly brackets). > > https://refactoring.com/catalog/reduceScopeOfVariable.html > OK. I moved these two local variables into scoped_guard. Will development interests grow for further refactorings? > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) > +{ > + struct aspeed_intc_ic *intc_ic = irq_desc_get_handler_data(desc); Another update candidate (for scope reduction)? > + > + guard(chained_irq)(desc); Using another macro call ?scoped_guard(?) { ? }?? > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { Would you like to reconsider the proposed macro mixture once more? > + unsigned long bit, status; ? ? > +++ b/include/linux/irqchip/chained_irq.h > @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, > chip->irq_unmask(&desc->irq_data); > } > > +DEFINE_GUARD(chained_irq, struct irq_desc *, chained_irq_exit((_T->irq_data.chip), (_T)), > + chained_irq_enter((_T->irq_data.chip), (_T))) Would you like to add a #include directive in this header file accordingly? Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-14 13:10 ` Markus Elfring @ 2024-10-15 10:19 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-15 10:19 UTC (permalink / raw) To: Markus Elfring, linux-aspeed@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org Cc: Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner, kernel-janitors@vger.kernel.org, LKML, BMC-SW > > > > I propose to move selected variable definitions into corresponding > > > compound statements (by using extra curly brackets). > > > https://refactoring.com/catalog/reduceScopeOfVariable.html > > OK. I moved these two local variables into scoped_guard. > > Will development interests grow for further refactorings? Do you have any example for this refactorings? > > > > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) { > > + struct aspeed_intc_ic *intc_ic = > > +irq_desc_get_handler_data(desc); > > Another update candidate (for scope reduction)? Variable of intc_ic used in "scoped_guard(raw_spinlock, &intc_ic->gic_lock) {". Or, how can I reduce the scope of intc_ic? > > > > + > > + guard(chained_irq)(desc); > > Using another macro call “scoped_guard(…) { … }”? Is it necessary to use scoped_guard(...) {...}? In the end of aspeed_intc_ic_irq_handler, chained_irq_exit would be called as destructor. Only one reason I thought is that the chained_irq_exit is needed to be called in the middle of aspeed_intc_ic_irq_handler. > > > > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > Would you like to reconsider the proposed macro mixture once more? Could I check the reason for once more? > > > > + unsigned long bit, status; > … > > … > > +++ b/include/linux/irqchip/chained_irq.h > > @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, > > chip->irq_unmask(&desc->irq_data); > > } > > > > +DEFINE_GUARD(chained_irq, struct irq_desc *, > chained_irq_exit((_T->irq_data.chip), (_T)), > > + chained_irq_enter((_T->irq_data.chip), (_T))) > > Would you like to add a #include directive in this header file accordingly? Can you give me an example? > > Regards, > Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-15 10:19 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-15 10:19 UTC (permalink / raw) To: linux-aspeed > > > > I propose to move selected variable definitions into corresponding > > > compound statements (by using extra curly brackets). > > > https://refactoring.com/catalog/reduceScopeOfVariable.html > > OK. I moved these two local variables into scoped_guard. > > Will development interests grow for further refactorings? Do you have any example for this refactorings? > > > > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) { > > + struct aspeed_intc_ic *intc_ic = > > +irq_desc_get_handler_data(desc); > > Another update candidate (for scope reduction)? Variable of intc_ic used in "scoped_guard(raw_spinlock, &intc_ic->gic_lock) {". Or, how can I reduce the scope of intc_ic? > > > > + > > + guard(chained_irq)(desc); > > Using another macro call ?scoped_guard(?) { ? }?? Is it necessary to use scoped_guard(...) {...}? In the end of aspeed_intc_ic_irq_handler, chained_irq_exit would be called as destructor. Only one reason I thought is that the chained_irq_exit is needed to be called in the middle of aspeed_intc_ic_irq_handler. > > > > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > Would you like to reconsider the proposed macro mixture once more? Could I check the reason for once more? > > > > + unsigned long bit, status; > ? > > ? > > +++ b/include/linux/irqchip/chained_irq.h > > @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, > > chip->irq_unmask(&desc->irq_data); > > } > > > > +DEFINE_GUARD(chained_irq, struct irq_desc *, > chained_irq_exit((_T->irq_data.chip), (_T)), > > + chained_irq_enter((_T->irq_data.chip), (_T))) > > Would you like to add a #include directive in this header file accordingly? Can you give me an example? > > Regards, > Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-15 10:19 ` Kevin Chen @ 2024-10-15 15:07 ` Markus Elfring -1 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-15 15:07 UTC (permalink / raw) To: Kevin Chen, linux-arm-kernel, devicetree, linux-aspeed Cc: Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring, Thomas Gleixner, LKML, BMC-SW, kernel-janitors … > > > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) { … > > > + guard(chained_irq)(desc); > > > > Using another macro call “scoped_guard(…) { … }”? > Is it necessary to use scoped_guard(...) {...}? It depends on corresponding case disintions. > > > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > > > Would you like to reconsider the proposed macro mixture once more? > Could I check the reason for once more? Coding style concerns …? > > > +++ b/include/linux/irqchip/chained_irq.h > > > @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, > > > chip->irq_unmask(&desc->irq_data); > > > } > > > > > > +DEFINE_GUARD(chained_irq, struct irq_desc *, > > chained_irq_exit((_T->irq_data.chip), (_T)), > > > + chained_irq_enter((_T->irq_data.chip), (_T))) > > > > Would you like to add a #include directive in this header file accordingly? > Can you give me an example? See also: https://elixir.bootlin.com/linux/v6.12-rc3/source/include/linux/device.h#L33 Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-15 15:07 ` Markus Elfring 0 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-15 15:07 UTC (permalink / raw) To: linux-aspeed ? > > > +static void aspeed_intc_ic_irq_handler(struct irq_desc *desc) { ? > > > + guard(chained_irq)(desc); > > > > Using another macro call ?scoped_guard(?) { ? }?? > Is it necessary to use scoped_guard(...) {...}? It depends on corresponding case disintions. > > > + scoped_guard(raw_spinlock, &intc_ic->gic_lock) { > > > > Would you like to reconsider the proposed macro mixture once more? > Could I check the reason for once more? Coding style concerns ?? > > > +++ b/include/linux/irqchip/chained_irq.h > > > @@ -38,4 +38,6 @@ static inline void chained_irq_exit(struct irq_chip *chip, > > > chip->irq_unmask(&desc->irq_data); > > > } > > > > > > +DEFINE_GUARD(chained_irq, struct irq_desc *, > > chained_irq_exit((_T->irq_data.chip), (_T)), > > > + chained_irq_enter((_T->irq_data.chip), (_T))) > > > > Would you like to add a #include directive in this header file accordingly? > Can you give me an example? See also: https://elixir.bootlin.com/linux/v6.12-rc3/source/include/linux/device.h#L33 Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-09 12:32 ` Markus Elfring @ 2024-10-15 21:42 ` Thomas Gleixner -1 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2024-10-15 21:42 UTC (permalink / raw) To: Markus Elfring, Kevin Chen, linux-aspeed, linux-arm-kernel, devicetree, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring Cc: LKML, kernel-janitors On Wed, Oct 09 2024 at 14:32, Markus Elfring wrote: >> + >> + chained_irq_enter(chip, desc); > > Would you become interested to collaborate with another scoped guard > for this programming interface? Collaborate in which way? What are you collaborating on? You are merely asking people to do work which you think is useful. You can do that, but that does not make it useful. Making a guard variant of chained_irq_enter/exit needs some thought and a general plan for cleaning the whole chained irq usage up. It's on the cleanup list already with quite some other items. We are not adhoc adding a guard variant because guards are hip right now. And no this does not need a scoped variant ever. guards are not the panacea for everything. > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained_irq.h#L13 Please refrain from these silly links. People know to find the functions on their own. Kevin, please update the change log, add your SOB and move the local variables (unsigned long bit, status;) into the scoped_guard() zone. Leave chained_irq_enter/exit() alone and resubmit. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-15 21:42 ` Thomas Gleixner 0 siblings, 0 replies; 36+ messages in thread From: Thomas Gleixner @ 2024-10-15 21:42 UTC (permalink / raw) To: linux-aspeed On Wed, Oct 09 2024 at 14:32, Markus Elfring wrote: >> + >> + chained_irq_enter(chip, desc); > > Would you become interested to collaborate with another scoped guard > for this programming interface? Collaborate in which way? What are you collaborating on? You are merely asking people to do work which you think is useful. You can do that, but that does not make it useful. Making a guard variant of chained_irq_enter/exit needs some thought and a general plan for cleaning the whole chained irq usage up. It's on the cleanup list already with quite some other items. We are not adhoc adding a guard variant because guards are hip right now. And no this does not need a scoped variant ever. guards are not the panacea for everything. > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchip/chained_irq.h#L13 Please refrain from these silly links. People know to find the functions on their own. Kevin, please update the change log, add your SOB and move the local variables (unsigned long bit, status;) into the scoped_guard() zone. Leave chained_irq_enter/exit() alone and resubmit. Thanks, tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-15 21:42 ` Thomas Gleixner @ 2024-10-16 0:31 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-16 0:31 UTC (permalink / raw) To: Thomas Gleixner, Markus Elfring, linux-aspeed@lists.ozlabs.org, linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring Cc: LKML, kernel-janitors@vger.kernel.org > >> + > >> + chained_irq_enter(chip, desc); > > > > Would you become interested to collaborate with another scoped guard > > for this programming interface? > > Collaborate in which way? What are you collaborating on? > > You are merely asking people to do work which you think is useful. You can do > that, but that does not make it useful. > > Making a guard variant of chained_irq_enter/exit needs some thought and a > general plan for cleaning the whole chained irq usage up. It's on the cleanup > list already with quite some other items. > > We are not adhoc adding a guard variant because guards are hip right now. > And no this does not need a scoped variant ever. > > guards are not the panacea for everything. > > > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchi > > p/chained_irq.h#L13 > > Please refrain from these silly links. People know to find the functions on their > own. > > Kevin, please update the change log, add your SOB and move the local > variables (unsigned long bit, status;) into the scoped_guard() zone. OK. I will do this change. > > Leave chained_irq_enter/exit() alone and resubmit. OK. I will send the v4 patch with chained_irq_enter/exit() alone. Thanks a lot. > > Thanks, > > tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-16 0:31 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-16 0:31 UTC (permalink / raw) To: linux-aspeed > >> + > >> + chained_irq_enter(chip, desc); > > > > Would you become interested to collaborate with another scoped guard > > for this programming interface? > > Collaborate in which way? What are you collaborating on? > > You are merely asking people to do work which you think is useful. You can do > that, but that does not make it useful. > > Making a guard variant of chained_irq_enter/exit needs some thought and a > general plan for cleaning the whole chained irq usage up. It's on the cleanup > list already with quite some other items. > > We are not adhoc adding a guard variant because guards are hip right now. > And no this does not need a scoped variant ever. > > guards are not the panacea for everything. > > > https://elixir.bootlin.com/linux/v6.12-rc2/source/include/linux/irqchi > > p/chained_irq.h#L13 > > Please refrain from these silly links. People know to find the functions on their > own. > > Kevin, please update the change log, add your SOB and move the local > variables (unsigned long bit, status;) into the scoped_guard() zone. OK. I will do this change. > > Leave chained_irq_enter/exit() alone and resubmit. OK. I will send the v4 patch with chained_irq_enter/exit() alone. Thanks a lot. > > Thanks, > > tglx ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-15 21:42 ` Thomas Gleixner @ 2024-10-16 9:54 ` Markus Elfring -1 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-16 9:54 UTC (permalink / raw) To: Thomas Gleixner, Kevin Chen, linux-aspeed, linux-arm-kernel, devicetree, Andrew Jeffery, Conor Dooley, Joel Stanley, Krzysztof Kozlowski, Rob Herring Cc: LKML, kernel-janitors > Making a guard variant of chained_irq_enter/exit needs some thought and > a general plan for cleaning the whole chained irq usage up. It's on the > cleanup list already with quite some other items. I became also curious how API usage will evolve further here. > We are not adhoc adding a guard variant because guards are hip right now. Application interests are growing, aren't they? > And no this does not need a scoped variant ever. There are subsystems which seem to prefer such a programming interface occasionally. > guards are not the panacea for everything. Their usage might become more popular. Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-16 9:54 ` Markus Elfring 0 siblings, 0 replies; 36+ messages in thread From: Markus Elfring @ 2024-10-16 9:54 UTC (permalink / raw) To: linux-aspeed > Making a guard variant of chained_irq_enter/exit needs some thought and > a general plan for cleaning the whole chained irq usage up. It's on the > cleanup list already with quite some other items. I became also curious how API usage will evolve further here. > We are not adhoc adding a guard variant because guards are hip right now. Application interests are growing, aren't they? > And no this does not need a scoped variant ever. There are subsystems which seem to prefer such a programming interface occasionally. > guards are not the panacea for everything. Their usage might become more popular. Regards, Markus ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-09 11:58 ` Kevin Chen @ 2024-10-11 13:06 ` Dan Carpenter -1 siblings, 0 replies; 36+ messages in thread From: Dan Carpenter @ 2024-10-11 13:06 UTC (permalink / raw) To: linux-aspeed On Wed, Oct 09, 2024 at 07:58:13PM +0800, Kevin Chen wrote: > +static int __init aspeed_intc_ic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct aspeed_intc_ic *intc_ic; > + int ret = 0; > + int irq, i; > + > + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL); > + if (!intc_ic) > + return -ENOMEM; > + > + intc_ic->base = of_iomap(node, 0); > + if (!intc_ic->base) { > + pr_err("Failed to iomap intc_ic base\n"); > + ret = -ENOMEM; > + goto err_free_ic; > + } > + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG); > + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG); > + > + intc_ic->irq_domain = irq_domain_add_linear(node, 32, > + &aspeed_intc_ic_irq_domain_ops, intc_ic); > + if (!intc_ic->irq_domain) { > + ret = -ENOMEM; > + goto err_iounmap; > + } > + > + raw_spin_lock_init(&intc_ic->gic_lock); > + raw_spin_lock_init(&intc_ic->intc_lock); > + > + /* Check all the irq numbers valid. If not, unmaps all the base and frees the data. */ > + for (i = 0; i < of_irq_count(node); i++) { > + irq = irq_of_parse_and_map(node, i); > + if (!irq) { > + pr_err("Failed to get irq number\n"); > + ret = -EINVAL; > + goto err_iounmap; > + } > + } > + > + for (i = 0; i < of_irq_count(node); i++) { > + irq = irq_of_parse_and_map(node, i); > + irq_set_chained_handler_and_data(irq, aspeed_intc_ic_irq_handler, intc_ic); There is an extra tab on this line. regards, dan carpenter > + } > + > + return 0; ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-11 13:06 ` Dan Carpenter 0 siblings, 0 replies; 36+ messages in thread From: Dan Carpenter @ 2024-10-11 13:06 UTC (permalink / raw) To: Kevin Chen Cc: tglx, robh, krzk+dt, conor+dt, joel, andrew, linux-kernel, devicetree, linux-arm-kernel, linux-aspeed On Wed, Oct 09, 2024 at 07:58:13PM +0800, Kevin Chen wrote: > +static int __init aspeed_intc_ic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + struct aspeed_intc_ic *intc_ic; > + int ret = 0; > + int irq, i; > + > + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL); > + if (!intc_ic) > + return -ENOMEM; > + > + intc_ic->base = of_iomap(node, 0); > + if (!intc_ic->base) { > + pr_err("Failed to iomap intc_ic base\n"); > + ret = -ENOMEM; > + goto err_free_ic; > + } > + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG); > + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG); > + > + intc_ic->irq_domain = irq_domain_add_linear(node, 32, > + &aspeed_intc_ic_irq_domain_ops, intc_ic); > + if (!intc_ic->irq_domain) { > + ret = -ENOMEM; > + goto err_iounmap; > + } > + > + raw_spin_lock_init(&intc_ic->gic_lock); > + raw_spin_lock_init(&intc_ic->intc_lock); > + > + /* Check all the irq numbers valid. If not, unmaps all the base and frees the data. */ > + for (i = 0; i < of_irq_count(node); i++) { > + irq = irq_of_parse_and_map(node, i); > + if (!irq) { > + pr_err("Failed to get irq number\n"); > + ret = -EINVAL; > + goto err_iounmap; > + } > + } > + > + for (i = 0; i < of_irq_count(node); i++) { > + irq = irq_of_parse_and_map(node, i); > + irq_set_chained_handler_and_data(irq, aspeed_intc_ic_irq_handler, intc_ic); There is an extra tab on this line. regards, dan carpenter > + } > + > + return 0; ^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC 2024-10-11 13:06 ` Dan Carpenter @ 2024-10-14 0:17 ` Kevin Chen -1 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-14 0:17 UTC (permalink / raw) To: linux-aspeed > > +static int __init aspeed_intc_ic_of_init(struct device_node *node, > > + struct device_node *parent) > > +{ > > + struct aspeed_intc_ic *intc_ic; > > + int ret = 0; > > + int irq, i; > > + > > + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL); > > + if (!intc_ic) > > + return -ENOMEM; > > + > > + intc_ic->base = of_iomap(node, 0); > > + if (!intc_ic->base) { > > + pr_err("Failed to iomap intc_ic base\n"); > > + ret = -ENOMEM; > > + goto err_free_ic; > > + } > > + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG); > > + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG); > > + > > + intc_ic->irq_domain = irq_domain_add_linear(node, 32, > > + &aspeed_intc_ic_irq_domain_ops, intc_ic); > > + if (!intc_ic->irq_domain) { > > + ret = -ENOMEM; > > + goto err_iounmap; > > + } > > + > > + raw_spin_lock_init(&intc_ic->gic_lock); > > + raw_spin_lock_init(&intc_ic->intc_lock); > > + > > + /* Check all the irq numbers valid. If not, unmaps all the base and frees > the data. */ > > + for (i = 0; i < of_irq_count(node); i++) { > > + irq = irq_of_parse_and_map(node, i); > > + if (!irq) { > > + pr_err("Failed to get irq number\n"); > > + ret = -EINVAL; > > + goto err_iounmap; > > + } > > + } > > + > > + for (i = 0; i < of_irq_count(node); i++) { > > + irq = irq_of_parse_and_map(node, i); > > + irq_set_chained_handler_and_data(irq, > aspeed_intc_ic_irq_handler, intc_ic); > > There is an extra tab on this line. OK. Fixed. Thanks a lot. > > regards, > dan carpenter > > > + } > > + > > + return 0; > ^ permalink raw reply [flat|nested] 36+ messages in thread
* RE: [PATCH v3 2/2] irqchip/aspeed-intc: Add support for AST27XX INTC @ 2024-10-14 0:17 ` Kevin Chen 0 siblings, 0 replies; 36+ messages in thread From: Kevin Chen @ 2024-10-14 0:17 UTC (permalink / raw) To: Dan Carpenter Cc: tglx@linutronix.de, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, joel@jms.id.au, andrew@codeconstruct.com.au, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, BMC-SW > > +static int __init aspeed_intc_ic_of_init(struct device_node *node, > > + struct device_node *parent) > > +{ > > + struct aspeed_intc_ic *intc_ic; > > + int ret = 0; > > + int irq, i; > > + > > + intc_ic = kzalloc(sizeof(*intc_ic), GFP_KERNEL); > > + if (!intc_ic) > > + return -ENOMEM; > > + > > + intc_ic->base = of_iomap(node, 0); > > + if (!intc_ic->base) { > > + pr_err("Failed to iomap intc_ic base\n"); > > + ret = -ENOMEM; > > + goto err_free_ic; > > + } > > + writel(0xffffffff, intc_ic->base + INTC_INT_STATUS_REG); > > + writel(0x0, intc_ic->base + INTC_INT_ENABLE_REG); > > + > > + intc_ic->irq_domain = irq_domain_add_linear(node, 32, > > + &aspeed_intc_ic_irq_domain_ops, intc_ic); > > + if (!intc_ic->irq_domain) { > > + ret = -ENOMEM; > > + goto err_iounmap; > > + } > > + > > + raw_spin_lock_init(&intc_ic->gic_lock); > > + raw_spin_lock_init(&intc_ic->intc_lock); > > + > > + /* Check all the irq numbers valid. If not, unmaps all the base and frees > the data. */ > > + for (i = 0; i < of_irq_count(node); i++) { > > + irq = irq_of_parse_and_map(node, i); > > + if (!irq) { > > + pr_err("Failed to get irq number\n"); > > + ret = -EINVAL; > > + goto err_iounmap; > > + } > > + } > > + > > + for (i = 0; i < of_irq_count(node); i++) { > > + irq = irq_of_parse_and_map(node, i); > > + irq_set_chained_handler_and_data(irq, > aspeed_intc_ic_irq_handler, intc_ic); > > There is an extra tab on this line. OK. Fixed. Thanks a lot. > > regards, > dan carpenter > > > + } > > + > > + return 0; > ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2024-10-16 9:55 UTC | newest] Thread overview: 36+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-09 11:58 [PATCH v3 0/2] Add support for AST2700 INTC driver Kevin Chen 2024-10-09 11:58 ` Kevin Chen 2024-10-09 11:58 ` [PATCH v3 1/2] dt-bindings: interrupt-controller: Add support for ASPEED AST27XX INTC Kevin Chen 2024-10-09 11:58 ` Kevin Chen 2024-10-09 12:56 ` Markus Elfring 2024-10-09 12:56 ` Markus Elfring 2024-10-09 21:11 ` Rob Herring 2024-10-09 21:11 ` Rob Herring 2024-10-11 10:02 ` Kevin Chen 2024-10-11 10:02 ` Kevin Chen 2024-10-09 11:58 ` [PATCH v3 2/2] irqchip/aspeed-intc: Add support for " Kevin Chen 2024-10-09 11:58 ` Kevin Chen 2024-10-09 12:32 ` Markus Elfring 2024-10-09 12:32 ` Markus Elfring 2024-10-11 10:06 ` Kevin Chen 2024-10-11 10:06 ` Kevin Chen 2024-10-11 15:34 ` Markus Elfring 2024-10-11 15:34 ` Markus Elfring 2024-10-14 2:00 ` Kevin Chen 2024-10-14 2:00 ` Kevin Chen 2024-10-14 13:10 ` Markus Elfring 2024-10-14 13:10 ` Markus Elfring 2024-10-15 10:19 ` Kevin Chen 2024-10-15 10:19 ` Kevin Chen 2024-10-15 15:07 ` Markus Elfring 2024-10-15 15:07 ` Markus Elfring 2024-10-15 21:42 ` Thomas Gleixner 2024-10-15 21:42 ` Thomas Gleixner 2024-10-16 0:31 ` Kevin Chen 2024-10-16 0:31 ` Kevin Chen 2024-10-16 9:54 ` [v3 " Markus Elfring 2024-10-16 9:54 ` Markus Elfring 2024-10-11 13:06 ` [PATCH v3 " Dan Carpenter 2024-10-11 13:06 ` Dan Carpenter 2024-10-14 0:17 ` Kevin Chen 2024-10-14 0:17 ` Kevin Chen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.