Hi Mesih, On Wed, Nov 21, 2018 at 09:30:38PM +0300, Mesih Kilinc wrote: > The new F-series SoCs (suniv) from Allwinner use an stripped version of > the interrupt controller in A10/A13 > > Add support for it in irq-sun4i driver. > > Signed-off-by: Mesih Kilinc <mesihkilinc@xxxxxxxxx> > --- > drivers/irqchip/irq-sun4i.c | 104 +++++++++++++++++++++++++++++++------------- > 1 file changed, 74 insertions(+), 30 deletions(-) > > diff --git a/drivers/irqchip/irq-sun4i.c b/drivers/irqchip/irq-sun4i.c > index e3e5b91..7ca4a4d 100644 > --- a/drivers/irqchip/irq-sun4i.c > +++ b/drivers/irqchip/irq-sun4i.c > @@ -28,11 +28,21 @@ > #define SUN4I_IRQ_NMI_CTRL_REG 0x0c > #define SUN4I_IRQ_PENDING_REG(x) (0x10 + 0x4 * x) > #define SUN4I_IRQ_FIQ_PENDING_REG(x) (0x20 + 0x4 * x) > -#define SUN4I_IRQ_ENABLE_REG(x) (0x40 + 0x4 * x) > -#define SUN4I_IRQ_MASK_REG(x) (0x50 + 0x4 * x) > +#define SUN4I_IRQ_ENABLE_REG(x) (irq_ic_data->enable_req_offset + 0x4 * x) > +#define SUN4I_IRQ_MASK_REG(x) (irq_ic_data->mask_req_offset + 0x4 * x) You shouldn't have all the values you use passed as argument, so irq_ic_data should be one of them here. > +#define SUN4I_IRQ_ENABLE_REG_OFFSET 0x40 > +#define SUN4I_IRQ_MASK_REG_OFFSET 0x50 > +#define SUNIV_IRQ_ENABLE_REG_OFFSET 0x20 > +#define SUNIV_IRQ_MASK_REG_OFFSET 0x30 > + > +struct sunxi_irq_chip_data{ ^ a space here > + void __iomem *irq_base; > + struct irq_domain *irq_domain; > + u32 enable_req_offset; > + u32 mask_req_offset; s/req/reg/ ? > +}; > > -static void __iomem *sun4i_irq_base; > -static struct irq_domain *sun4i_irq_domain; > +static struct sunxi_irq_chip_data *irq_ic_data; This is a very welcome change, but you should do it in two patches: one to convert the existing code to that structure (with only the domain and base), and then a second patch to add the register offsets. (also the structure should be prefixed with sun4i, for consistency with the rest of the driver). > > static void __exception_irq_entry sun4i_handle_irq(struct pt_regs *regs); > > @@ -43,7 +53,7 @@ static void sun4i_irq_ack(struct irq_data *irqd) > if (irq != 0) > return; /* Only IRQ 0 / the ENMI needs to be acked */ > > - writel(BIT(0), sun4i_irq_base + SUN4I_IRQ_PENDING_REG(0)); > + writel(BIT(0), irq_ic_data->irq_base + SUN4I_IRQ_PENDING_REG(0)); > } > > static void sun4i_irq_mask(struct irq_data *irqd) > @@ -53,9 +63,9 @@ static void sun4i_irq_mask(struct irq_data *irqd) > int reg = irq / 32; > u32 val; > > - val = readl(sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > + val = readl(irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > writel(val & ~(1 << irq_off), > - sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > + irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > } > > static void sun4i_irq_unmask(struct irq_data *irqd) > @@ -65,9 +75,9 @@ static void sun4i_irq_unmask(struct irq_data *irqd) > int reg = irq / 32; > u32 val; > > - val = readl(sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > + val = readl(irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > writel(val | (1 << irq_off), > - sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > + irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(reg)); > } > > static struct irq_chip sun4i_irq_chip = { > @@ -95,42 +105,76 @@ static const struct irq_domain_ops sun4i_irq_ops = { > static int __init sun4i_of_init(struct device_node *node, > struct device_node *parent) > { > - sun4i_irq_base = of_iomap(node, 0); > - if (!sun4i_irq_base) > + irq_ic_data->irq_base = of_iomap(node, 0); > + if (!irq_ic_data->irq_base) > panic("%pOF: unable to map IC registers\n", > node); > > /* Disable all interrupts */ > - writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(0)); > - writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(1)); > - writel(0, sun4i_irq_base + SUN4I_IRQ_ENABLE_REG(2)); > + writel(0, irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(0)); > + writel(0, irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(1)); > + writel(0, irq_ic_data->irq_base + SUN4I_IRQ_ENABLE_REG(2)); > > /* Unmask all the interrupts, ENABLE_REG(x) is used for masking */ > - writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(0)); > - writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(1)); > - writel(0, sun4i_irq_base + SUN4I_IRQ_MASK_REG(2)); > + writel(0, irq_ic_data->irq_base + SUN4I_IRQ_MASK_REG(0)); > + writel(0, irq_ic_data->irq_base + SUN4I_IRQ_MASK_REG(1)); > + writel(0, irq_ic_data->irq_base + SUN4I_IRQ_MASK_REG(2)); > > /* Clear all the pending interrupts */ > - writel(0xffffffff, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(0)); > - writel(0xffffffff, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(1)); > - writel(0xffffffff, sun4i_irq_base + SUN4I_IRQ_PENDING_REG(2)); > + writel(0xffffffff, irq_ic_data->irq_base + SUN4I_IRQ_PENDING_REG(0)); > + writel(0xffffffff, irq_ic_data->irq_base + SUN4I_IRQ_PENDING_REG(1)); > + writel(0xffffffff, irq_ic_data->irq_base + SUN4I_IRQ_PENDING_REG(2)); > > - /* Enable protection mode */ > - writel(0x01, sun4i_irq_base + SUN4I_IRQ_PROTECTION_REG); > + /* Enable protection mode (not available in suniv) */ > + if (of_device_is_compatible(node, "allwinner,sun4i-a10-ic")) > + writel(0x01, irq_ic_data->irq_base + SUN4I_IRQ_PROTECTION_REG); > > /* Configure the external interrupt source type */ > - writel(0x00, sun4i_irq_base + SUN4I_IRQ_NMI_CTRL_REG); > + writel(0x00, irq_ic_data->irq_base + SUN4I_IRQ_NMI_CTRL_REG); > > - sun4i_irq_domain = irq_domain_add_linear(node, 3 * 32, > + irq_ic_data->irq_domain = irq_domain_add_linear(node, 3 * 32, > &sun4i_irq_ops, NULL); > - if (!sun4i_irq_domain) > + if (!irq_ic_data->irq_domain) > panic("%pOF: unable to create IRQ domain\n", node); > > set_handle_irq(sun4i_handle_irq); > > return 0; > } > -IRQCHIP_DECLARE(allwinner_sun4i_ic, "allwinner,sun4i-a10-ic", sun4i_of_init); > + > +static int __init sun4i_ic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + irq_ic_data = kzalloc(sizeof(struct sunxi_irq_chip_data), GFP_KERNEL); > + if (!irq_ic_data) { > + pr_err("kzalloc failed!\n"); > + return -ENOMEM; > + } > + > + irq_ic_data->enable_req_offset = SUN4I_IRQ_ENABLE_REG_OFFSET; > + irq_ic_data->mask_req_offset = SUN4I_IRQ_MASK_REG_OFFSET; > + > + return sun4i_of_init(node, parent); > +} > + > +IRQCHIP_DECLARE(allwinner_sun4i_ic, "allwinner,sun4i-a10-ic", sun4i_ic_of_init); > + > +static int __init suniv_ic_of_init(struct device_node *node, > + struct device_node *parent) > +{ > + irq_ic_data = kzalloc(sizeof(struct sunxi_irq_chip_data), GFP_KERNEL); > + if (!irq_ic_data) { > + pr_err("kzalloc failed!\n"); > + return -ENOMEM; > + } > + > + irq_ic_data->enable_req_offset = SUNIV_IRQ_ENABLE_REG_OFFSET; > + irq_ic_data->mask_req_offset = SUNIV_IRQ_MASK_REG_OFFSET; > + > + return sun4i_of_init(node, parent); > +} > + > +IRQCHIP_DECLARE(allwinner_sunvi_ic, "allwinner,suniv-f1c100s-ic", suniv_ic_of_init); You can even split that addition to a new patch as well. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Attachment:
signature.asc
Description: PGP signature