Re: [DO NOT MERGE v5 16/37] irqchip: Add SH7751 INTC driver
On Tue, Dec 05 2023 at 18:45, Yoshinori Sato wrote:
> +static void endisable_irq(struct irq_data *data, int enable)
bool enable?
> +{
> + struct sh7751_intc_priv *priv;
> + unsigned int irq;
> +
> + priv = irq_data_to_priv(data);
> +
> + irq = irqd_to_hwirq(data);
> + if (!is_valid_irq(irq)) {
> + /* IRQ out of range */
> + pr_warn_once("%s: IRQ %u is out of range\n", __FILE__, irq);
> + return;
> + }
> +
> + if (irq <= MAX_IRL && !priv->irlm)
> + /* IRL encoded external interrupt */
> + /* disable for SR.IMASK */
> + update_sr_imask(irq - IRQ_START, enable);
> + else
> + /* Internal peripheral interrupt */
> + /* mask for IPR priority 0 */
> + update_ipr(priv, irq, enable);
Lacks curly brackets on the if/else
> +static int irq_sh7751_map(struct irq_domain *h, unsigned int virq,
> + irq_hw_number_t hw_irq_num)
> +{
> + irq_set_chip_and_handler(virq, _irq_chip, handle_level_irq);
> + irq_get_irq_data(virq)->chip_data = h->host_data;
> + irq_modify_status(virq, IRQ_NOREQUEST, IRQ_NOPROBE);
> + return 0;
> +}
> +static const struct irq_domain_ops irq_ops = {
Newline before 'static ...'
> + .map= irq_sh7751_map,
> + .xlate = irq_domain_xlate_onecell,
> +};
> +
> +static int __init load_ipr_map(struct device_node *intc,
> +struct sh7751_intc_priv *priv)
> +{
> + struct property *ipr_map;
> + unsigned int num_ipr, i;
> + struct ipr *ipr;
> + const __be32 *p;
> + u32 irq;
> +
> + ipr_map = of_find_property(intc, "renesas,ipr-map", _ipr);
> + if (IS_ERR(ipr_map))
> + return PTR_ERR(ipr_map);
> + num_ipr /= sizeof(u32);
> + /* 3words per entry. */
> + if (num_ipr % 3)
Three words per ... But you can spare the comment by doing:
if (num_ipr % WORDS_PER_ENTRY)
> + goto error1;
> + num_ipr /= 3;
> +static int __init sh7751_intc_of_init(struct device_node *intc,
> + struct device_node *parent)
> +{
> + struct sh7751_intc_priv *priv;
> + void __iomem *base, *base2;
> + struct irq_domain *domain;
> + u16 icr;
> + int ret;
> +
> + base = of_iomap(intc, 0);
> + base2 = of_iomap(intc, 1);
> + if (!base || !base2) {
> + pr_err("%pOFP: Invalid register definition\n", intc);
What unmaps 'base' if 'base' is valid and base2 == NULL?
> + return -EINVAL;
> + }
> +
> + priv = kzalloc(sizeof(struct sh7751_intc_priv), GFP_KERNEL);
> + if (priv == NULL)
> + return -ENOMEM;
Leaks base[2] maps, no?
> + ret = load_ipr_map(intc, priv);
> + if (ret < 0) {
> + kfree(priv);
> + return ret;
> + }
> +
> + priv->base = base;
> + priv->intpri00 = base2;
> +
> + if (of_property_read_bool(intc, "renesas,irlm")) {
> + priv->irlm = true;
> + icr = __raw_readw(priv->base + R_ICR);
> + icr |= ICR_IRLM;
> + __raw_writew(icr, priv->base + R_ICR);
> + }
> +
> + domain = irq_domain_add_linear(intc, NR_IRQS, _ops, priv);
> + if (domain == NULL) {
> + pr_err("%pOFP: cannot initialize irq domain\n", intc);
> + kfree(priv);
> + return -ENOMEM;
> + }
> +
> + irq_set_default_host(domain);
> + pr_info("%pOFP: SH7751 Interrupt controller (%s external IRQ)",
> + intc, priv->irlm ? "4 lines" : "15 level");
> + return 0;
> +}
> +
> +IRQCHIP_DECLARE(sh_7751_intc,
> + "renesas,sh7751-intc", sh7751_intc_of_init);
One line please.
Thanks,
tglx
[DO NOT MERGE v5 16/37] irqchip: Add SH7751 INTC driver
Renesas SH7751 Internal interrupt controller driver.
Signed-off-by: Yoshinori Sato
---
drivers/irqchip/Kconfig | 8 +
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-renesas-sh7751.c | 290 +++
3 files changed, 299 insertions(+)
create mode 100644 drivers/irqchip/irq-renesas-sh7751.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index f7149d0f3d45..658523f65b1d 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -679,4 +679,12 @@ config SUNPLUS_SP7021_INTC
chained controller, routing all interrupt source in P-Chip to
the primary controller on C-Chip.
+config RENESAS_SH7751_INTC
+ bool "Renesas SH7751 Interrupt Controller"
+ depends on SH_DEVICE_TREE || COMPILE_TEST
+ select IRQ_DOMAIN_HIERARCHY
+ help
+ Support for the Renesas SH7751 On-chip interrupt controller.
+ And external interrupt encoder for some targets.
+
endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index ffd945fe71aa..26c91d075e25 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -120,3 +120,4 @@ obj-$(CONFIG_IRQ_IDT3243X) += irq-idt3243x.o
obj-$(CONFIG_APPLE_AIC)+= irq-apple-aic.o
obj-$(CONFIG_MCHP_EIC) += irq-mchp-eic.o
obj-$(CONFIG_SUNPLUS_SP7021_INTC) += irq-sp7021-intc.o
+obj-$(CONFIG_RENESAS_SH7751_INTC) += irq-renesas-sh7751.o
diff --git a/drivers/irqchip/irq-renesas-sh7751.c
b/drivers/irqchip/irq-renesas-sh7751.c
new file mode 100644
index ..2a5cb2444d99
--- /dev/null
+++ b/drivers/irqchip/irq-renesas-sh7751.c
@@ -0,0 +1,290 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas SH7751 interrupt controller driver
+ *
+ * Copyright 2023 Yoshinori Sato
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct ipr {
+ unsigned int off;
+ unsigned int idx;
+};
+
+struct sh7751_intc_priv {
+ void __iomem *base;
+ void __iomem *intpri00;
+ struct ipr *iprmap[2];
+ bool irlm;
+};
+
+enum {
+ R_ICR = 0x00,
+ R_IPR = 0x04,
+ R_INTPRI00= 0x00,
+ R_INTREQ00= 0x20,
+ R_INTMSK00= 0x40,
+ R_INTMSKCLR00 = 0x60,
+};
+
+#define ICR_IRLM BIT(7)
+
+/*
+ * SH7751 IRQ mapping
+ * IRQ16 - 63: Group0 - IPRA to IPRD
+ * IRQ16 - 31: external IRL input (ICR.IRLM is 0)
+ * IRQ80 - 92: Group1 - INTPRI00
+ */
+#define IRQ_START 16
+#define MAX_IRL(IRQ_START + NR_IRL)
+#define GRP0_IRQ_END 63
+#define GRP1_IRQ_START 80
+#define IRQ_END92
+
+#define NR_IPRMAP0 (GRP0_IRQ_END - IRQ_START + 1)
+#define NR_IPRMAP1 (IRQ_END - GRP1_IRQ_START)
+#define IPR_PRI_MASK 0x000f
+
+/*
+ * IPR registers have 4bit priority x 4 entry (16bits)
+ * Interrupts can be masked by setting pri to 0.
+ */
+static void update_ipr(struct sh7751_intc_priv *priv, unsigned int irq, u16
pri)
+{
+ struct ipr *ipr = NULL;
+ void __iomem *ipr_base;
+ unsigned int offset;
+ u16 mask;
+
+ if (irq < GRP1_IRQ_START) {
+ /* Group0 */
+ ipr = priv->iprmap[0];
+ ipr += irq - IRQ_START;
+ ipr_base = priv->base + R_IPR;
+ offset = ipr->off;
+ } else {
+ /* Group1 */
+ ipr = priv->iprmap[1];
+ ipr += irq - GRP1_IRQ_START;
+ ipr_base = priv->intpri00;
+ offset = ipr->off - INTPRI00;
+ }
+ if (ipr->off != ~0) {
+ mask = ~(IPR_PRI_MASK << ipr->idx);
+ pri = (pri & IPR_PRI_MASK) << ipr->idx;
+ mask &= __raw_readw(ipr_base + offset);
+ __raw_writew(mask | pri, ipr_base + offset);
+ } else {
+ pr_warn_once("%s: undefined IPR in irq %u\n", __FILE__, irq);
+ }
+}
+
+static inline bool is_valid_irq(unsigned int irq)
+{
+ /* IRQ16 - 63 */
+ if (irq >= IRQ_START && irq < IRQ_START + NR_IPRMAP0)
+ return true;
+ /* IRQ80 - 92 */
+ if (irq >= GRP1_IRQ_START && irq <= IRQ_END)
+ return true;
+ return false;
+}
+
+static inline struct sh7751_intc_priv *irq_data_to_priv(struct irq_data *data)
+{
+ return data->domain->host_data;
+}
+
+static void endisable_irq(struct irq_data *data, int enable)
+{
+ struct sh7751_intc_priv *priv;
+ unsigned int irq;
+
+ priv = irq_data_to_priv(data);
+
+ irq = irqd_to_hwirq(data);
+ if (!is_valid_irq(irq)) {
+ /* IRQ out of range */
+ pr_warn_once("%s: IRQ %u is out of range\n", __FILE__, irq);
+ return;
+ }
+
+ if (irq <= MAX_IRL && !priv->irlm)
+ /* IRL encoded external interrupt */
+ /* disable for SR.IMASK */
+ update_sr_imask(irq - IRQ_START, enable);
+
