* [PATCH v2 0/2] Add JH8100 external interrupt controller support @ 2024-01-30 5:58 Changhuang Liang 2024-01-30 5:58 ` [PATCH v2 1/2] dt-bindings: interrupt-controller: Add starfive,jh8100-intc Changhuang Liang 2024-01-30 5:58 ` [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller Changhuang Liang 0 siblings, 2 replies; 7+ messages in thread From: Changhuang Liang @ 2024-01-30 5:58 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Ley Foon Tan, Jack Zhu, Changhuang Liang, linux-kernel, devicetree This patchset adds external interrupt controller driver for the StarFive JH81000 SoC. It can be used to handle high-level input interrupt signals. It also send the output interrupt signal to RISC-V PLIC. changes since v1: - Rebased on tag v6.8-rc1. - Dropped store reset_contorl. - Replaced "of_reset_control_get_by_index" with of_reset_control_get_exclusive - Printed the error code via %pe v1: https://lore.kernel.org/all/20240111023201.6187-1-changhuang.liang@starfivetech.com/ Changhuang Liang (2): dt-bindings: interrupt-controller: Add starfive,jh8100-intc irqchip: Add StarFive external interrupt controller .../starfive,jh8100-intc.yaml | 61 ++++++ MAINTAINERS | 6 + drivers/irqchip/Kconfig | 11 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-starfive-jh8100-intc.c | 180 ++++++++++++++++++ 5 files changed, 259 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml create mode 100644 drivers/irqchip/irq-starfive-jh8100-intc.c -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] dt-bindings: interrupt-controller: Add starfive,jh8100-intc 2024-01-30 5:58 [PATCH v2 0/2] Add JH8100 external interrupt controller support Changhuang Liang @ 2024-01-30 5:58 ` Changhuang Liang 2024-01-30 5:58 ` [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller Changhuang Liang 1 sibling, 0 replies; 7+ messages in thread From: Changhuang Liang @ 2024-01-30 5:58 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Ley Foon Tan, Jack Zhu, Changhuang Liang, linux-kernel, devicetree StarFive SoCs like the JH8100 use a interrupt controller. Add a binding for it. Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> --- .../starfive,jh8100-intc.yaml | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml diff --git a/Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml b/Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml new file mode 100644 index 000000000000..ada5788602d6 --- /dev/null +++ b/Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/interrupt-controller/starfive,jh8100-intc.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: StarFive External Interrupt Controller + +description: + StarFive SoC JH8100 contain a external interrupt controller. It can be used + to handle high-level input interrupt signals. It also send the output + interrupt signal to RISC-V PLIC. + +maintainers: + - Changhuang Liang <changhuang.liang@starfivetech.com> + +properties: + compatible: + const: starfive,jh8100-intc + + reg: + maxItems: 1 + + clocks: + description: APB clock for the interrupt controller + maxItems: 1 + + resets: + description: APB reset for the interrupt controller + maxItems: 1 + + interrupts: + maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": + const: 1 + +required: + - compatible + - reg + - clocks + - resets + - interrupts + - interrupt-controller + - "#interrupt-cells" + +additionalProperties: false + +examples: + - | + interrupt-controller@12260000 { + compatible = "starfive,jh8100-intc"; + reg = <0x12260000 0x10000>; + clocks = <&syscrg_ne 76>; + resets = <&syscrg_ne 13>; + interrupts = <45>; + interrupt-controller; + #interrupt-cells = <1>; + }; -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller 2024-01-30 5:58 [PATCH v2 0/2] Add JH8100 external interrupt controller support Changhuang Liang 2024-01-30 5:58 ` [PATCH v2 1/2] dt-bindings: interrupt-controller: Add starfive,jh8100-intc Changhuang Liang @ 2024-01-30 5:58 ` Changhuang Liang 2024-02-13 9:03 ` Thomas Gleixner 1 sibling, 1 reply; 7+ messages in thread From: Changhuang Liang @ 2024-01-30 5:58 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Ley Foon Tan, Jack Zhu, Changhuang Liang, linux-kernel, devicetree Add StarFive external interrupt controller for JH8100 SoC. Signed-off-by: Changhuang Liang <changhuang.liang@starfivetech.com> Reviewed-by: Ley Foon Tan <leyfoon.tan@starfivetech.com> --- MAINTAINERS | 6 + drivers/irqchip/Kconfig | 11 ++ drivers/irqchip/Makefile | 1 + drivers/irqchip/irq-starfive-jh8100-intc.c | 180 +++++++++++++++++++++ 4 files changed, 198 insertions(+) create mode 100644 drivers/irqchip/irq-starfive-jh8100-intc.c diff --git a/MAINTAINERS b/MAINTAINERS index 8d1052fa6a69..ef678f04c830 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -20956,6 +20956,12 @@ F: Documentation/devicetree/bindings/phy/starfive,jh7110-usb-phy.yaml F: drivers/phy/starfive/phy-jh7110-pcie.c F: drivers/phy/starfive/phy-jh7110-usb.c +STARFIVE JH8100 EXTERNAL INTERRUPT CONTROLLER DRIVER +M: Changhuang Liang <changhuang.liang@starfivetech.com> +S: Supported +F: Documentation/devicetree/bindings/interrupt-controller/starfive,jh8100-intc.yaml +F: drivers/irqchip/irq-starfive-jh8100-intc.c + STATIC BRANCH/CALL M: Peter Zijlstra <peterz@infradead.org> M: Josh Poimboeuf <jpoimboe@kernel.org> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig index f7149d0f3d45..72c07a12f5e1 100644 --- a/drivers/irqchip/Kconfig +++ b/drivers/irqchip/Kconfig @@ -546,6 +546,17 @@ config SIFIVE_PLIC select IRQ_DOMAIN_HIERARCHY select GENERIC_IRQ_EFFECTIVE_AFF_MASK if SMP +config STARFIVE_JH8100_INTC + bool "StarFive JH8100 External Interrupt Controller" + depends on ARCH_STARFIVE || COMPILE_TEST + default ARCH_STARFIVE + select IRQ_DOMAIN_HIERARCHY + help + This enables support for the INTC chip found in StarFive JH8100 + SoC. + + If you don't know what to do here, say Y. + config EXYNOS_IRQ_COMBINER bool "Samsung Exynos IRQ combiner support" if COMPILE_TEST depends on (ARCH_EXYNOS && ARM) || COMPILE_TEST diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile index ffd945fe71aa..ec4a18380998 100644 --- a/drivers/irqchip/Makefile +++ b/drivers/irqchip/Makefile @@ -96,6 +96,7 @@ obj-$(CONFIG_CSKY_MPINTC) += irq-csky-mpintc.o obj-$(CONFIG_CSKY_APB_INTC) += irq-csky-apb-intc.o obj-$(CONFIG_RISCV_INTC) += irq-riscv-intc.o obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o +obj-$(CONFIG_STARFIVE_JH8100_INTC) += irq-starfive-jh8100-intc.o obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o obj-$(CONFIG_IMX_INTMUX) += irq-imx-intmux.o obj-$(CONFIG_IMX_MU_MSI) += irq-imx-mu-msi.o diff --git a/drivers/irqchip/irq-starfive-jh8100-intc.c b/drivers/irqchip/irq-starfive-jh8100-intc.c new file mode 100644 index 000000000000..344f7d871518 --- /dev/null +++ b/drivers/irqchip/irq-starfive-jh8100-intc.c @@ -0,0 +1,180 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * StarFive JH8100 External Interrupt Controller driver + * + * Copyright (C) 2023 StarFive Technology Co., Ltd. + * + * Author: Changhuang Liang <changhuang.liang@starfivetech.com> + */ + +#define pr_fmt(fmt) "irq-starfive-jh8100: " fmt + +#include <linux/bitops.h> +#include <linux/clk.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/reset.h> + +#define STARFIVE_INTC_SRC0_CLEAR 0x10 +#define STARFIVE_INTC_SRC0_MASK 0x14 +#define STARFIVE_INTC_SRC0_INT 0x1c + +#define STARFIVE_INTC_SRC_IRQ_NUM 32 + +struct starfive_irq_chip { + void __iomem *base; + struct irq_domain *root_domain; + struct clk *clk; +}; + +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 reg, u32 mask, u32 data) +{ + u32 value; + + value = ioread32(irqc->base + reg) & ~mask; + data &= mask; + data |= value; + iowrite32(data, irqc->base + reg); +} + +static void starfive_intc_unmask(struct irq_data *d) +{ + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d); + + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0); +} + +static void starfive_intc_mask(struct irq_data *d) +{ + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d); + + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), BIT(d->hwirq)); +} + +static struct irq_chip intc_dev = { + .name = "starfive jh8100 intc", + .irq_unmask = starfive_intc_unmask, + .irq_mask = starfive_intc_mask, +}; + +static int starfive_intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) +{ + irq_domain_set_info(d, irq, hwirq, &intc_dev, d->host_data, + handle_level_irq, NULL, NULL); + + return 0; +} + +static const struct irq_domain_ops starfive_intc_domain_ops = { + .xlate = irq_domain_xlate_onecell, + .map = starfive_intc_map, +}; + +static void starfive_intc_irq_handler(struct irq_desc *desc) +{ + struct irq_chip *chip = irq_desc_get_chip(desc); + struct starfive_irq_chip *irqc = irq_data_get_irq_handler_data(&desc->irq_data); + unsigned long value = 0; + int hwirq; + + chained_irq_enter(chip, desc); + + value = ioread32(irqc->base + STARFIVE_INTC_SRC0_INT); + while (value) { + hwirq = ffs(value) - 1; + + generic_handle_domain_irq(irqc->root_domain, hwirq); + + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), BIT(hwirq)); + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), 0); + + clear_bit(hwirq, &value); + } + + chained_irq_exit(chip, desc); +} + +static int __init starfive_intc_init(struct device_node *intc, + struct device_node *parent) +{ + struct starfive_irq_chip *irqc; + struct reset_control *rst; + int ret; + int parent_irq; + + irqc = kzalloc(sizeof(*irqc), GFP_KERNEL); + if (!irqc) + return -ENOMEM; + + irqc->base = of_iomap(intc, 0); + if (!irqc->base) { + pr_err("Unable to map IC registers\n"); + ret = -ENXIO; + goto err_free; + } + + rst = of_reset_control_get_exclusive(intc, NULL); + if (IS_ERR(rst)) { + pr_err("Unable to get reset control %pe\n", rst); + ret = PTR_ERR(rst); + goto err_unmap; + } + + irqc->clk = of_clk_get(intc, 0); + if (IS_ERR(irqc->clk)) { + pr_err("Unable to get clock\n"); + ret = PTR_ERR(irqc->clk); + goto err_rst; + } + + ret = reset_control_deassert(rst); + if (ret) + goto err_clk; + + ret = clk_prepare_enable(irqc->clk); + if (ret) + goto err_clk; + + irqc->root_domain = irq_domain_add_linear(intc, STARFIVE_INTC_SRC_IRQ_NUM, + &starfive_intc_domain_ops, irqc); + if (!irqc->root_domain) { + pr_err("Unable to create IRQ domain\n"); + ret = -EINVAL; + goto err_clk; + } + + parent_irq = of_irq_get(intc, 0); + if (parent_irq < 0) { + pr_err("Failed to get main IRQ: %d\n", parent_irq); + ret = parent_irq; + goto err_clk; + } + + irq_set_chained_handler_and_data(parent_irq, starfive_intc_irq_handler, irqc); + + pr_info("Interrupt controller register, nr_irqs %d\n", STARFIVE_INTC_SRC_IRQ_NUM); + + return 0; + +err_clk: + clk_put(irqc->clk); +err_rst: + reset_control_put(rst); +err_unmap: + iounmap(irqc->base); +err_free: + kfree(irqc); + return ret; +} + +IRQCHIP_PLATFORM_DRIVER_BEGIN(starfive_intc) +IRQCHIP_MATCH("starfive,jh8100-intc", starfive_intc_init) +IRQCHIP_PLATFORM_DRIVER_END(starfive_intc) + +MODULE_DESCRIPTION("StarFive JH8100 External Interrupt Controller"); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Changhuang Liang <changhuang.liang@starfivetech.com>"); -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller 2024-01-30 5:58 ` [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller Changhuang Liang @ 2024-02-13 9:03 ` Thomas Gleixner 2024-02-18 2:36 ` 回复: " Changhuang Liang 0 siblings, 1 reply; 7+ messages in thread From: Thomas Gleixner @ 2024-02-13 9:03 UTC (permalink / raw) To: Changhuang Liang, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Ley Foon Tan, Jack Zhu, Changhuang Liang, linux-kernel, devicetree On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote: > + > +struct starfive_irq_chip { > + void __iomem *base; > + struct irq_domain *root_domain; > + struct clk *clk; > +}; https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#struct-declarations-and-initializers Please. > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 reg, u32 mask, u32 data) > +{ > + u32 value; > + > + value = ioread32(irqc->base + reg) & ~mask; > + data &= mask; Why? > + data |= value; > + iowrite32(data, irqc->base + reg); How is this serialized against concurrent invocations of this code on different CPUs for different interrupts? It's not and this requires a raw_spinlock for protection. > +} > + > +static void starfive_intc_unmask(struct irq_data *d) > +{ > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d); > + > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0); > +} > + > +static void starfive_intc_mask(struct irq_data *d) > +{ > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d); > + > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), BIT(d->hwirq)); > +} > + > +static struct irq_chip intc_dev = { > + .name = "starfive jh8100 intc", > + .irq_unmask = starfive_intc_unmask, > + .irq_mask = starfive_intc_mask, > +}; See documentation please. > +static int starfive_intc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hwirq) > +{ > + irq_domain_set_info(d, irq, hwirq, &intc_dev, d->host_data, > + handle_level_irq, NULL, NULL); > + > + return 0; > +} > + > +static const struct irq_domain_ops starfive_intc_domain_ops = { > + .xlate = irq_domain_xlate_onecell, > + .map = starfive_intc_map, > +}; Ditto. > +static void starfive_intc_irq_handler(struct irq_desc *desc) > +{ > + struct irq_chip *chip = irq_desc_get_chip(desc); > + struct starfive_irq_chip *irqc = irq_data_get_irq_handler_data(&desc->irq_data); https://www.kernel.org/doc/html/latest/process/maintainer-tip.html#variable-declarations > + unsigned long value = 0; Pointless initialization > + int hwirq; > + > + chained_irq_enter(chip, desc); > + > + value = ioread32(irqc->base + STARFIVE_INTC_SRC0_INT); > + while (value) { > + hwirq = ffs(value) - 1; > + > + generic_handle_domain_irq(irqc->root_domain, hwirq); > + > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), BIT(hwirq)); > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_CLEAR, BIT(hwirq), 0); > + > + clear_bit(hwirq, &value); > + } > + > + chained_irq_exit(chip, desc); > +} > + > +static int __init starfive_intc_init(struct device_node *intc, > + struct device_node *parent) > +{ > + struct starfive_irq_chip *irqc; > + struct reset_control *rst; > + int ret; > + int parent_irq; See Documentation > + irqc = kzalloc(sizeof(*irqc), GFP_KERNEL); > + if (!irqc) > + return -ENOMEM; > + > + irqc->base = of_iomap(intc, 0); > + if (!irqc->base) { > + pr_err("Unable to map IC registers\n"); > + ret = -ENXIO; > + goto err_free; > + } > + > + rst = of_reset_control_get_exclusive(intc, NULL); > + if (IS_ERR(rst)) { > + pr_err("Unable to get reset control %pe\n", rst); > + ret = PTR_ERR(rst); > + goto err_unmap; > + } > + > + irqc->clk = of_clk_get(intc, 0); > + if (IS_ERR(irqc->clk)) { > + pr_err("Unable to get clock\n"); > + ret = PTR_ERR(irqc->clk); > + goto err_rst; > + } > + > + ret = reset_control_deassert(rst); > + if (ret) > + goto err_clk; > + > + ret = clk_prepare_enable(irqc->clk); > + if (ret) > + goto err_clk; > + > + irqc->root_domain = irq_domain_add_linear(intc, STARFIVE_INTC_SRC_IRQ_NUM, > + &starfive_intc_domain_ops, irqc); > + if (!irqc->root_domain) { > + pr_err("Unable to create IRQ domain\n"); > + ret = -EINVAL; > + goto err_clk; > + } > + > + parent_irq = of_irq_get(intc, 0); > + if (parent_irq < 0) { > + pr_err("Failed to get main IRQ: %d\n", parent_irq); > + ret = parent_irq; > + goto err_clk; Leaks the interrupt domain, no? Thanks, tglx ^ permalink raw reply [flat|nested] 7+ messages in thread
* 回复: [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller 2024-02-13 9:03 ` Thomas Gleixner @ 2024-02-18 2:36 ` Changhuang Liang 2024-02-20 9:28 ` Thomas Gleixner 0 siblings, 1 reply; 7+ messages in thread From: Changhuang Liang @ 2024-02-18 2:36 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Leyfoon Tan, Jack Zhu, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Hi, Thomas Thanks for your comment. > On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote: [...] > > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 > > +reg, u32 mask, u32 data) { > > + u32 value; > > + > > + value = ioread32(irqc->base + reg) & ~mask; > > + data &= mask; > > Why? > If I want to update the reg GENMASK(7, 4) to value 5, the data I will pass in 5 << 4 > > + data |= value; > > + iowrite32(data, irqc->base + reg); > > How is this serialized against concurrent invocations of this code on different > CPUs for different interrupts? > > It's not and this requires a raw_spinlock for protection. > Will use raw_spinlock. > > +} > > + > > +static void starfive_intc_unmask(struct irq_data *d) { > > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d); > > + > > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), 0); > > +} > > + > > +static void starfive_intc_mask(struct irq_data *d) { > > + struct starfive_irq_chip *irqc = irq_data_get_irq_chip_data(d); > > + > > + starfive_intc_mod(irqc, STARFIVE_INTC_SRC0_MASK, BIT(d->hwirq), > > +BIT(d->hwirq)); } > > + [...] > > + irqc->root_domain = irq_domain_add_linear(intc, > STARFIVE_INTC_SRC_IRQ_NUM, > > + &starfive_intc_domain_ops, irqc); > > + if (!irqc->root_domain) { > > + pr_err("Unable to create IRQ domain\n"); > > + ret = -EINVAL; > > + goto err_clk; > > + } > > + > > + parent_irq = of_irq_get(intc, 0); > > + if (parent_irq < 0) { > > + pr_err("Failed to get main IRQ: %d\n", parent_irq); > > + ret = parent_irq; > > + goto err_clk; > > Leaks the interrupt domain, no? > > Thanks, > Will use irq_domain_remove() free domain. regards Changhuang ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: 回复: [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller 2024-02-18 2:36 ` 回复: " Changhuang Liang @ 2024-02-20 9:28 ` Thomas Gleixner 2024-02-20 9:39 ` 回复: " Changhuang Liang 0 siblings, 1 reply; 7+ messages in thread From: Thomas Gleixner @ 2024-02-20 9:28 UTC (permalink / raw) To: Changhuang Liang, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Leyfoon Tan, Jack Zhu, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org On Sun, Feb 18 2024 at 02:36, Changhuang Liang wrote: >> On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote: > [...] >> > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 >> > +reg, u32 mask, u32 data) { >> > + u32 value; >> > + >> > + value = ioread32(irqc->base + reg) & ~mask; >> > + data &= mask; >> >> Why? >> > > If I want to update the reg GENMASK(7, 4) to value 5, the data I > will pass in 5 << 4 All call sites pass a single bit to set/clear, right? So this GENMASK argument does not make sense at all. Thanks, tglx ^ permalink raw reply [flat|nested] 7+ messages in thread
* 回复: 回复: [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller 2024-02-20 9:28 ` Thomas Gleixner @ 2024-02-20 9:39 ` Changhuang Liang 0 siblings, 0 replies; 7+ messages in thread From: Changhuang Liang @ 2024-02-20 9:39 UTC (permalink / raw) To: Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Philipp Zabel Cc: Leyfoon Tan, Jack Zhu, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Hi, Thomas > -----邮件原件----- > 发件人: Thomas Gleixner <tglx@linutronix.de> > 发送时间: 2024年2月20日 17:28 > 收件人: Changhuang Liang <changhuang.liang@starfivetech.com>; Rob > Herring <robh+dt@kernel.org>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; > Philipp Zabel <p.zabel@pengutronix.de> > 抄送: Leyfoon Tan <leyfoon.tan@starfivetech.com>; Jack Zhu > <jack.zhu@starfivetech.com>; linux-kernel@vger.kernel.org; > devicetree@vger.kernel.org > 主题: Re: 回复: [PATCH v2 2/2] irqchip: Add StarFive external interrupt > controller > > On Sun, Feb 18 2024 at 02:36, Changhuang Liang wrote: > >> On Mon, Jan 29 2024 at 21:58, Changhuang Liang wrote: > > [...] > >> > +static void starfive_intc_mod(struct starfive_irq_chip *irqc, u32 > >> > +reg, u32 mask, u32 data) { > >> > + u32 value; > >> > + > >> > + value = ioread32(irqc->base + reg) & ~mask; > >> > + data &= mask; > >> > >> Why? > >> > > > > If I want to update the reg GENMASK(7, 4) to value 5, the data I > > will pass in 5 << 4 > > All call sites pass a single bit to set/clear, right? So this GENMASK argument > does not make sense at all. > Yes, I will update this function ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2024-02-20 9:39 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-30 5:58 [PATCH v2 0/2] Add JH8100 external interrupt controller support Changhuang Liang 2024-01-30 5:58 ` [PATCH v2 1/2] dt-bindings: interrupt-controller: Add starfive,jh8100-intc Changhuang Liang 2024-01-30 5:58 ` [PATCH v2 2/2] irqchip: Add StarFive external interrupt controller Changhuang Liang 2024-02-13 9:03 ` Thomas Gleixner 2024-02-18 2:36 ` 回复: " Changhuang Liang 2024-02-20 9:28 ` Thomas Gleixner 2024-02-20 9:39 ` 回复: " Changhuang Liang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).