[07/19] irqchip/mmp: mask off interrupts from other cores
diff mbox series

Message ID 20190809093158.7969-8-lkundrak@v3.sk
State New
Headers show
Series
  • Initial support for Marvell MMP3 SoC
Related show

Commit Message

Lubomir Rintel Aug. 9, 2019, 9:31 a.m. UTC
From: Andres Salomon <dilinger@queued.net>

On mmp3, there's an extra set of ICU registers (ICU2) that handle
interrupts on the extra cores.  When masking off interrupts on MP1,
these should be masked as well.

We add a new interrupt controller via device tree to identify when we're
looking at an mmp3 machine via compatible field of "marvell,mmp3-intc".

[lkundrak@v3.sk: Changed "mrvl,mmp3-intc" compatible strings to
"marvell,mmp3-intc". Tidied up the subject line a bit.]

Signed-off-by: Andres Salomon <dilinger@queued.net>
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>

---
 arch/arm/mach-mmp/regs-icu.h |  3 +++
 drivers/irqchip/irq-mmp.c    | 51 ++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

Comments

Marc Zyngier Aug. 9, 2019, 12:18 p.m. UTC | #1
On 09/08/2019 10:31, Lubomir Rintel wrote:
> From: Andres Salomon <dilinger@queued.net>
> 
> On mmp3, there's an extra set of ICU registers (ICU2) that handle
> interrupts on the extra cores.  When masking off interrupts on MP1,
> these should be masked as well.
> 
> We add a new interrupt controller via device tree to identify when we're
> looking at an mmp3 machine via compatible field of "marvell,mmp3-intc".
> 
> [lkundrak@v3.sk: Changed "mrvl,mmp3-intc" compatible strings to
> "marvell,mmp3-intc". Tidied up the subject line a bit.]
> 
> Signed-off-by: Andres Salomon <dilinger@queued.net>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> 
> ---
>  arch/arm/mach-mmp/regs-icu.h |  3 +++
>  drivers/irqchip/irq-mmp.c    | 51 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/arch/arm/mach-mmp/regs-icu.h b/arch/arm/mach-mmp/regs-icu.h
> index 0375d5a7fcb2b..410743d2b4020 100644
> --- a/arch/arm/mach-mmp/regs-icu.h
> +++ b/arch/arm/mach-mmp/regs-icu.h
> @@ -11,6 +11,9 @@
>  #define ICU_VIRT_BASE	(AXI_VIRT_BASE + 0x82000)
>  #define ICU_REG(x)	(ICU_VIRT_BASE + (x))
>  
> +#define ICU2_VIRT_BASE	(AXI_VIRT_BASE + 0x84000)
> +#define ICU2_REG(x)	(ICU2_VIRT_BASE + (x))
> +
>  #define ICU_INT_CONF(n)		ICU_REG((n) << 2)
>  #define ICU_INT_CONF_MASK	(0xf)
>  
> diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> index cd8d2253f56d1..25497c75cc861 100644
> --- a/drivers/irqchip/irq-mmp.c
> +++ b/drivers/irqchip/irq-mmp.c
> @@ -44,6 +44,7 @@ struct icu_chip_data {
>  	unsigned int		conf_enable;
>  	unsigned int		conf_disable;
>  	unsigned int		conf_mask;
> +	unsigned int		conf2_mask;
>  	unsigned int		clr_mfp_irq_base;
>  	unsigned int		clr_mfp_hwirq;
>  	struct irq_domain	*domain;
> @@ -53,9 +54,11 @@ struct mmp_intc_conf {
>  	unsigned int	conf_enable;
>  	unsigned int	conf_disable;
>  	unsigned int	conf_mask;
> +	unsigned int	conf2_mask;
>  };
>  
>  static void __iomem *mmp_icu_base;
> +static void __iomem *mmp_icu2_base;
>  static struct icu_chip_data icu_data[MAX_ICU_NR];
>  static int max_icu_nr;
>  
> @@ -98,6 +101,16 @@ static void icu_mask_irq(struct irq_data *d)
>  		r &= ~data->conf_mask;
>  		r |= data->conf_disable;
>  		writel_relaxed(r, mmp_icu_base + (hwirq << 2));
> +
> +		if (data->conf2_mask) {
> +			/*
> +			 * ICU1 (above) only controls PJ4 MP1; if using SMP,
> +			 * we need to also mask the MP2 and MM cores via ICU2.
> +			 */
> +			r = readl_relaxed(mmp_icu2_base + (hwirq << 2));
> +			r &= ~data->conf2_mask;
> +			writel_relaxed(r, mmp_icu2_base + (hwirq << 2));
> +		}
>  	} else {
>  		r = readl_relaxed(data->reg_mask) | (1 << hwirq);
>  		writel_relaxed(r, data->reg_mask);
> @@ -201,6 +214,14 @@ static const struct mmp_intc_conf mmp2_conf = {
>  			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
>  };
>  
> +static struct mmp_intc_conf mmp3_conf = {
> +	.conf_enable	= 0x20,
> +	.conf_disable	= 0x0,
> +	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
> +			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
> +	.conf2_mask	= 0xf0,
> +};
> +
>  static void __exception_irq_entry mmp_handle_irq(struct pt_regs *regs)
>  {
>  	int hwirq;
> @@ -364,6 +385,14 @@ static int __init mmp_init_bases(struct device_node *node)
>  		pr_err("Failed to get interrupt controller register\n");
>  		return -ENOMEM;
>  	}
> +	if (of_device_is_compatible(node, "marvell,mmp3-intc")) {

Instead of harcoding the compatible property once more, why don't you
simply pass a flag from mmpx_of_init()?

> +		mmp_icu2_base = of_iomap(node, 1);
> +		if (!mmp_icu2_base) {
> +			pr_err("Failed to get interrupt controller register #2\n");
> +			iounmap(mmp_icu_base);
> +			return -ENOMEM;
> +		}
> +	}
>  
>  	icu_data[0].virq_base = 0;
>  	icu_data[0].domain = irq_domain_add_linear(node, nr_irqs,
> @@ -386,6 +415,8 @@ static int __init mmp_init_bases(struct device_node *node)
>  			irq_dispose_mapping(icu_data[0].virq_base + i);
>  	}
>  	irq_domain_remove(icu_data[0].domain);
> +	if (of_device_is_compatible(node, "marvell,mmp3-intc"))
> +		iounmap(mmp_icu2_base);
>  	iounmap(mmp_icu_base);
>  	return -EINVAL;
>  }
> @@ -428,6 +459,26 @@ static int __init mmp2_of_init(struct device_node *node,
>  }
>  IRQCHIP_DECLARE(mmp2_intc, "mrvl,mmp2-intc", mmp2_of_init);
>  
> +static int __init mmp3_of_init(struct device_node *node,
> +			       struct device_node *parent)
> +{
> +	int ret;
> +
> +	ret = mmp_init_bases(node);
> +	if (ret < 0)
> +		return ret;
> +
> +	icu_data[0].conf_enable = mmp3_conf.conf_enable;
> +	icu_data[0].conf_disable = mmp3_conf.conf_disable;
> +	icu_data[0].conf_mask = mmp3_conf.conf_mask;
> +	icu_data[0].conf2_mask = mmp3_conf.conf2_mask;
> +	irq_set_default_host(icu_data[0].domain);

Why do you need this? On a fully DT-ified platform, there should be no
notion of a default domain.

> +	set_handle_irq(mmp2_handle_irq);
> +	max_icu_nr = 1;
> +	return 0;
> +}
> +IRQCHIP_DECLARE(mmp3_intc, "marvell,mmp3-intc", mmp3_of_init);
> +
>  static int __init mmp2_mux_of_init(struct device_node *node,
>  				   struct device_node *parent)
>  {
> 

Thanks,

	M.
Lubomir Rintel Aug. 16, 2019, 6:15 p.m. UTC | #2
On Fri, 2019-08-09 at 13:18 +0100, Marc Zyngier wrote:
> On 09/08/2019 10:31, Lubomir Rintel wrote:
> > From: Andres Salomon <dilinger@queued.net>
> > 
> > On mmp3, there's an extra set of ICU registers (ICU2) that handle
> > interrupts on the extra cores.  When masking off interrupts on MP1,
> > these should be masked as well.
> > 
> > We add a new interrupt controller via device tree to identify when we're
> > looking at an mmp3 machine via compatible field of "marvell,mmp3-intc".
> > 
> > [lkundrak@v3.sk: Changed "mrvl,mmp3-intc" compatible strings to
> > "marvell,mmp3-intc". Tidied up the subject line a bit.]
> > 
> > Signed-off-by: Andres Salomon <dilinger@queued.net>
> > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > 
> > ---
> >  arch/arm/mach-mmp/regs-icu.h |  3 +++
> >  drivers/irqchip/irq-mmp.c    | 51 ++++++++++++++++++++++++++++++++++++
> >  2 files changed, 54 insertions(+)
> > 
> > diff --git a/arch/arm/mach-mmp/regs-icu.h b/arch/arm/mach-mmp/regs-icu.h
> > index 0375d5a7fcb2b..410743d2b4020 100644
> > --- a/arch/arm/mach-mmp/regs-icu.h
> > +++ b/arch/arm/mach-mmp/regs-icu.h
> > @@ -11,6 +11,9 @@
> >  #define ICU_VIRT_BASE	(AXI_VIRT_BASE + 0x82000)
> >  #define ICU_REG(x)	(ICU_VIRT_BASE + (x))
> >  
> > +#define ICU2_VIRT_BASE	(AXI_VIRT_BASE + 0x84000)
> > +#define ICU2_REG(x)	(ICU2_VIRT_BASE + (x))
> > +
> >  #define ICU_INT_CONF(n)		ICU_REG((n) << 2)
> >  #define ICU_INT_CONF_MASK	(0xf)
> >  
> > diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
> > index cd8d2253f56d1..25497c75cc861 100644
> > --- a/drivers/irqchip/irq-mmp.c
> > +++ b/drivers/irqchip/irq-mmp.c
> > @@ -44,6 +44,7 @@ struct icu_chip_data {
> >  	unsigned int		conf_enable;
> >  	unsigned int		conf_disable;
> >  	unsigned int		conf_mask;
> > +	unsigned int		conf2_mask;
> >  	unsigned int		clr_mfp_irq_base;
> >  	unsigned int		clr_mfp_hwirq;
> >  	struct irq_domain	*domain;
> > @@ -53,9 +54,11 @@ struct mmp_intc_conf {
> >  	unsigned int	conf_enable;
> >  	unsigned int	conf_disable;
> >  	unsigned int	conf_mask;
> > +	unsigned int	conf2_mask;
> >  };
> >  
> >  static void __iomem *mmp_icu_base;
> > +static void __iomem *mmp_icu2_base;
> >  static struct icu_chip_data icu_data[MAX_ICU_NR];
> >  static int max_icu_nr;
> >  
> > @@ -98,6 +101,16 @@ static void icu_mask_irq(struct irq_data *d)
> >  		r &= ~data->conf_mask;
> >  		r |= data->conf_disable;
> >  		writel_relaxed(r, mmp_icu_base + (hwirq << 2));
> > +
> > +		if (data->conf2_mask) {
> > +			/*
> > +			 * ICU1 (above) only controls PJ4 MP1; if using SMP,
> > +			 * we need to also mask the MP2 and MM cores via ICU2.
> > +			 */
> > +			r = readl_relaxed(mmp_icu2_base + (hwirq << 2));
> > +			r &= ~data->conf2_mask;
> > +			writel_relaxed(r, mmp_icu2_base + (hwirq << 2));
> > +		}
> >  	} else {
> >  		r = readl_relaxed(data->reg_mask) | (1 << hwirq);
> >  		writel_relaxed(r, data->reg_mask);
> > @@ -201,6 +214,14 @@ static const struct mmp_intc_conf mmp2_conf = {
> >  			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
> >  };
> >  
> > +static struct mmp_intc_conf mmp3_conf = {
> > +	.conf_enable	= 0x20,
> > +	.conf_disable	= 0x0,
> > +	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
> > +			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
> > +	.conf2_mask	= 0xf0,
> > +};
> > +
> >  static void __exception_irq_entry mmp_handle_irq(struct pt_regs *regs)
> >  {
> >  	int hwirq;
> > @@ -364,6 +385,14 @@ static int __init mmp_init_bases(struct device_node *node)
> >  		pr_err("Failed to get interrupt controller register\n");
> >  		return -ENOMEM;
> >  	}
> > +	if (of_device_is_compatible(node, "marvell,mmp3-intc")) {
> 
> Instead of harcoding the compatible property once more, why don't you
> simply pass a flag from mmpx_of_init()?

Will do so in next version.

> > +		mmp_icu2_base = of_iomap(node, 1);
> > +		if (!mmp_icu2_base) {
> > +			pr_err("Failed to get interrupt controller register #2\n");
> > +			iounmap(mmp_icu_base);
> > +			return -ENOMEM;
> > +		}
> > +	}
> >  
> >  	icu_data[0].virq_base = 0;
> >  	icu_data[0].domain = irq_domain_add_linear(node, nr_irqs,
> > @@ -386,6 +415,8 @@ static int __init mmp_init_bases(struct device_node *node)
> >  			irq_dispose_mapping(icu_data[0].virq_base + i);
> >  	}
> >  	irq_domain_remove(icu_data[0].domain);
> > +	if (of_device_is_compatible(node, "marvell,mmp3-intc"))
> > +		iounmap(mmp_icu2_base);
> >  	iounmap(mmp_icu_base);
> >  	return -EINVAL;
> >  }
> > @@ -428,6 +459,26 @@ static int __init mmp2_of_init(struct device_node *node,
> >  }
> >  IRQCHIP_DECLARE(mmp2_intc, "mrvl,mmp2-intc", mmp2_of_init);
> >  
> > +static int __init mmp3_of_init(struct device_node *node,
> > +			       struct device_node *parent)
> > +{
> > +	int ret;
> > +
> > +	ret = mmp_init_bases(node);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	icu_data[0].conf_enable = mmp3_conf.conf_enable;
> > +	icu_data[0].conf_disable = mmp3_conf.conf_disable;
> > +	icu_data[0].conf_mask = mmp3_conf.conf_mask;
> > +	icu_data[0].conf2_mask = mmp3_conf.conf2_mask;
> > +	irq_set_default_host(icu_data[0].domain);
> 
> Why do you need this? On a fully DT-ified platform, there should be no
> notion of a default domain.

I didn't know. Pretty sure this was cargo-culted, because it's done
elsewhere too and also unnecessary. Will remove those cases too.

> > +	set_handle_irq(mmp2_handle_irq);
> > +	max_icu_nr = 1;
> > +	return 0;
> > +}
> > +IRQCHIP_DECLARE(mmp3_intc, "marvell,mmp3-intc", mmp3_of_init);
> > +
> >  static int __init mmp2_mux_of_init(struct device_node *node,
> >  				   struct device_node *parent)
> >  {
> > 
> 
> Thanks,
> 
> 	M.

Thanks
Lubo

Patch
diff mbox series

diff --git a/arch/arm/mach-mmp/regs-icu.h b/arch/arm/mach-mmp/regs-icu.h
index 0375d5a7fcb2b..410743d2b4020 100644
--- a/arch/arm/mach-mmp/regs-icu.h
+++ b/arch/arm/mach-mmp/regs-icu.h
@@ -11,6 +11,9 @@ 
 #define ICU_VIRT_BASE	(AXI_VIRT_BASE + 0x82000)
 #define ICU_REG(x)	(ICU_VIRT_BASE + (x))
 
+#define ICU2_VIRT_BASE	(AXI_VIRT_BASE + 0x84000)
+#define ICU2_REG(x)	(ICU2_VIRT_BASE + (x))
+
 #define ICU_INT_CONF(n)		ICU_REG((n) << 2)
 #define ICU_INT_CONF_MASK	(0xf)
 
diff --git a/drivers/irqchip/irq-mmp.c b/drivers/irqchip/irq-mmp.c
index cd8d2253f56d1..25497c75cc861 100644
--- a/drivers/irqchip/irq-mmp.c
+++ b/drivers/irqchip/irq-mmp.c
@@ -44,6 +44,7 @@  struct icu_chip_data {
 	unsigned int		conf_enable;
 	unsigned int		conf_disable;
 	unsigned int		conf_mask;
+	unsigned int		conf2_mask;
 	unsigned int		clr_mfp_irq_base;
 	unsigned int		clr_mfp_hwirq;
 	struct irq_domain	*domain;
@@ -53,9 +54,11 @@  struct mmp_intc_conf {
 	unsigned int	conf_enable;
 	unsigned int	conf_disable;
 	unsigned int	conf_mask;
+	unsigned int	conf2_mask;
 };
 
 static void __iomem *mmp_icu_base;
+static void __iomem *mmp_icu2_base;
 static struct icu_chip_data icu_data[MAX_ICU_NR];
 static int max_icu_nr;
 
@@ -98,6 +101,16 @@  static void icu_mask_irq(struct irq_data *d)
 		r &= ~data->conf_mask;
 		r |= data->conf_disable;
 		writel_relaxed(r, mmp_icu_base + (hwirq << 2));
+
+		if (data->conf2_mask) {
+			/*
+			 * ICU1 (above) only controls PJ4 MP1; if using SMP,
+			 * we need to also mask the MP2 and MM cores via ICU2.
+			 */
+			r = readl_relaxed(mmp_icu2_base + (hwirq << 2));
+			r &= ~data->conf2_mask;
+			writel_relaxed(r, mmp_icu2_base + (hwirq << 2));
+		}
 	} else {
 		r = readl_relaxed(data->reg_mask) | (1 << hwirq);
 		writel_relaxed(r, data->reg_mask);
@@ -201,6 +214,14 @@  static const struct mmp_intc_conf mmp2_conf = {
 			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
 };
 
+static struct mmp_intc_conf mmp3_conf = {
+	.conf_enable	= 0x20,
+	.conf_disable	= 0x0,
+	.conf_mask	= MMP2_ICU_INT_ROUTE_PJ4_IRQ |
+			  MMP2_ICU_INT_ROUTE_PJ4_FIQ,
+	.conf2_mask	= 0xf0,
+};
+
 static void __exception_irq_entry mmp_handle_irq(struct pt_regs *regs)
 {
 	int hwirq;
@@ -364,6 +385,14 @@  static int __init mmp_init_bases(struct device_node *node)
 		pr_err("Failed to get interrupt controller register\n");
 		return -ENOMEM;
 	}
+	if (of_device_is_compatible(node, "marvell,mmp3-intc")) {
+		mmp_icu2_base = of_iomap(node, 1);
+		if (!mmp_icu2_base) {
+			pr_err("Failed to get interrupt controller register #2\n");
+			iounmap(mmp_icu_base);
+			return -ENOMEM;
+		}
+	}
 
 	icu_data[0].virq_base = 0;
 	icu_data[0].domain = irq_domain_add_linear(node, nr_irqs,
@@ -386,6 +415,8 @@  static int __init mmp_init_bases(struct device_node *node)
 			irq_dispose_mapping(icu_data[0].virq_base + i);
 	}
 	irq_domain_remove(icu_data[0].domain);
+	if (of_device_is_compatible(node, "marvell,mmp3-intc"))
+		iounmap(mmp_icu2_base);
 	iounmap(mmp_icu_base);
 	return -EINVAL;
 }
@@ -428,6 +459,26 @@  static int __init mmp2_of_init(struct device_node *node,
 }
 IRQCHIP_DECLARE(mmp2_intc, "mrvl,mmp2-intc", mmp2_of_init);
 
+static int __init mmp3_of_init(struct device_node *node,
+			       struct device_node *parent)
+{
+	int ret;
+
+	ret = mmp_init_bases(node);
+	if (ret < 0)
+		return ret;
+
+	icu_data[0].conf_enable = mmp3_conf.conf_enable;
+	icu_data[0].conf_disable = mmp3_conf.conf_disable;
+	icu_data[0].conf_mask = mmp3_conf.conf_mask;
+	icu_data[0].conf2_mask = mmp3_conf.conf2_mask;
+	irq_set_default_host(icu_data[0].domain);
+	set_handle_irq(mmp2_handle_irq);
+	max_icu_nr = 1;
+	return 0;
+}
+IRQCHIP_DECLARE(mmp3_intc, "marvell,mmp3-intc", mmp3_of_init);
+
 static int __init mmp2_mux_of_init(struct device_node *node,
 				   struct device_node *parent)
 {