Re: [patch v14 4/4] irqchip/qeic: remove PPCisms for QEIC

From: Marc Zyngier
Date: Mon Mar 25 2019 - 05:27:10 EST


On Mon, 25 Mar 2019 03:53:48 +0000,
Qiang Zhao <qiang.zhao@xxxxxxx> wrote:
>
> QEIC was supported on PowerPC, and dependent on PPC,
> Now it is supported on other platforms, so remove PPCisms.
>
> Signed-off-by: Zhao Qiang <qiang.zhao@xxxxxxx>
> ---
> arch/powerpc/platforms/83xx/km83xx.c | 1 -
> arch/powerpc/platforms/83xx/misc.c | 1 -
> arch/powerpc/platforms/83xx/mpc832x_mds.c | 1 -
> arch/powerpc/platforms/83xx/mpc832x_rdb.c | 1 -
> arch/powerpc/platforms/83xx/mpc836x_mds.c | 1 -
> arch/powerpc/platforms/83xx/mpc836x_rdk.c | 1 -
> arch/powerpc/platforms/85xx/corenet_generic.c | 1 -
> arch/powerpc/platforms/85xx/mpc85xx_mds.c | 1 -
> arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 1 -
> arch/powerpc/platforms/85xx/twr_p102x.c | 1 -
> drivers/irqchip/irq-qeic.c | 188 +++++++++++--------------
> include/soc/fsl/qe/qe_ic.h | 132 -----------------
> 12 files changed, 80 insertions(+), 250 deletions(-)
> delete mode 100644 include/soc/fsl/qe/qe_ic.h
>
> diff --git a/arch/powerpc/platforms/83xx/km83xx.c b/arch/powerpc/platforms/83xx/km83xx.c
> index d8642a4..b1cef0a 100644
> --- a/arch/powerpc/platforms/83xx/km83xx.c
> +++ b/arch/powerpc/platforms/83xx/km83xx.c
> @@ -38,7 +38,6 @@
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/misc.c b/arch/powerpc/platforms/83xx/misc.c
> index 4150b56..b033a10 100644
> --- a/arch/powerpc/platforms/83xx/misc.c
> +++ b/arch/powerpc/platforms/83xx/misc.c
> @@ -18,7 +18,6 @@
> #include <asm/io.h>
> #include <asm/hw_irq.h>
> #include <asm/ipic.h>
> -#include <soc/fsl/qe/qe_ic.h>
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
>
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> index 74c154e..f86371b 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> @@ -37,7 +37,6 @@
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> index 4389865..da91395 100644
> --- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> +++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> @@ -26,7 +26,6 @@
> #include <asm/ipic.h>
> #include <asm/udbg.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
>
> diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> index fd44dd0..9b8bc8b 100644
> --- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
> +++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> @@ -45,7 +45,6 @@
> #include <sysdev/fsl_pci.h>
> #include <sysdev/simple_gpio.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #include "mpc83xx.h"
>
> diff --git a/arch/powerpc/platforms/83xx/mpc836x_rdk.c b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
> index 93f024f..82fa344 100644
> --- a/arch/powerpc/platforms/83xx/mpc836x_rdk.c
> +++ b/arch/powerpc/platforms/83xx/mpc836x_rdk.c
> @@ -21,7 +21,6 @@
> #include <asm/ipic.h>
> #include <asm/udbg.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
>
> diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c
> index e44bb44..ac2478d 100644
> --- a/arch/powerpc/platforms/85xx/corenet_generic.c
> +++ b/arch/powerpc/platforms/85xx/corenet_generic.c
> @@ -28,7 +28,6 @@
> #include <asm/mpic.h>
> #include <asm/ehv_pic.h>
> #include <asm/swiotlb.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #include <linux/of_platform.h>
> #include <sysdev/fsl_soc.h>
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> index 6892bc1..809266d 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c
> @@ -49,7 +49,6 @@
> #include <sysdev/fsl_pci.h>
> #include <sysdev/simple_gpio.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
> #include <asm/mpic.h>
> #include <asm/swiotlb.h>
> #include "smp.h"
> diff --git a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> index 000d385..f806b6b 100644
> --- a/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> +++ b/arch/powerpc/platforms/85xx/mpc85xx_rdb.c
> @@ -27,7 +27,6 @@
> #include <asm/udbg.h>
> #include <asm/mpic.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
> diff --git a/arch/powerpc/platforms/85xx/twr_p102x.c b/arch/powerpc/platforms/85xx/twr_p102x.c
> index 6be9b33..4f620f2 100644
> --- a/arch/powerpc/platforms/85xx/twr_p102x.c
> +++ b/arch/powerpc/platforms/85xx/twr_p102x.c
> @@ -23,7 +23,6 @@
> #include <asm/udbg.h>
> #include <asm/mpic.h>
> #include <soc/fsl/qe/qe.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #include <sysdev/fsl_soc.h>
> #include <sysdev/fsl_pci.h>
> diff --git a/drivers/irqchip/irq-qeic.c b/drivers/irqchip/irq-qeic.c
> index a6ccbfb..723c52e 100644
> --- a/drivers/irqchip/irq-qeic.c
> +++ b/drivers/irqchip/irq-qeic.c
> @@ -18,8 +18,11 @@
> #include <linux/of_address.h>
> #include <linux/kernel.h>
> #include <linux/init.h>
> +#include <linux/irqdomain.h>
> #include <linux/irqchip.h>
> #include <linux/errno.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> #include <linux/reboot.h>
> #include <linux/slab.h>
> #include <linux/stddef.h>
> @@ -27,9 +30,8 @@
> #include <linux/signal.h>
> #include <linux/device.h>
> #include <linux/spinlock.h>
> -#include <asm/irq.h>
> +#include <linux/irq.h>
> #include <asm/io.h>
> -#include <soc/fsl/qe/qe_ic.h>
>
> #define NR_QE_IC_INTS 64
>
> @@ -87,6 +89,43 @@
> #define SIGNAL_HIGH 2
> #define SIGNAL_LOW 0
>
> +#define NUM_OF_QE_IC_GROUPS 6
> +
> +/* Flags when we init the QE IC */
> +#define QE_IC_SPREADMODE_GRP_W BIT(0)
> +#define QE_IC_SPREADMODE_GRP_X BIT(1)
> +#define QE_IC_SPREADMODE_GRP_Y BIT(2)
> +#define QE_IC_SPREADMODE_GRP_Z BIT(3)
> +#define QE_IC_SPREADMODE_GRP_RISCA BIT(4)
> +#define QE_IC_SPREADMODE_GRP_RISCB BIT(5)
> +
> +#define QE_IC_LOW_SIGNAL BIT(8)
> +#define QE_IC_HIGH_SIGNAL BIT(9)
> +
> +#define QE_IC_GRP_W_PRI0_DEST_SIGNAL_HIGH BIT(12)
> +#define QE_IC_GRP_W_PRI1_DEST_SIGNAL_HIGH BIT(13)
> +#define QE_IC_GRP_X_PRI0_DEST_SIGNAL_HIGH BIT(14)
> +#define QE_IC_GRP_X_PRI1_DEST_SIGNAL_HIGH BIT(15)
> +#define QE_IC_GRP_Y_PRI0_DEST_SIGNAL_HIGH BIT(16)
> +#define QE_IC_GRP_Y_PRI1_DEST_SIGNAL_HIGH BIT(17)
> +#define QE_IC_GRP_Z_PRI0_DEST_SIGNAL_HIGH BIT(18)
> +#define QE_IC_GRP_Z_PRI1_DEST_SIGNAL_HIGH BIT(19)
> +#define QE_IC_GRP_RISCA_PRI0_DEST_SIGNAL_HIGH BIT(20)
> +#define QE_IC_GRP_RISCA_PRI1_DEST_SIGNAL_HIGH BIT(21)
> +#define QE_IC_GRP_RISCB_PRI0_DEST_SIGNAL_HIGH BIT(22)
> +#define QE_IC_GRP_RISCB_PRI1_DEST_SIGNAL_HIGH BIT(23)
> +#define QE_IC_GRP_W_DEST_SIGNAL_SHIFT (12)
> +
> +/* QE interrupt sources groups */
> +enum qe_ic_grp_id {
> + QE_IC_GRP_W = 0, /* QE interrupt controller group W */
> + QE_IC_GRP_X, /* QE interrupt controller group X */
> + QE_IC_GRP_Y, /* QE interrupt controller group Y */
> + QE_IC_GRP_Z, /* QE interrupt controller group Z */
> + QE_IC_GRP_RISCA, /* QE interrupt controller RISC group A */
> + QE_IC_GRP_RISCB /* QE interrupt controller RISC group B */
> +};
> +
> struct qe_ic {
> /* Control registers offset */
> u32 __iomem *regs;
> @@ -265,15 +304,15 @@ struct qe_ic_info {
> },
> };
>
> -static inline u32 qe_ic_read(volatile __be32 __iomem * base, unsigned int reg)
> +static u32 qe_ic_read(__be32 __iomem *base, unsigned int reg)
> {
> - return in_be32(base + (reg >> 2));
> + return ioread32be(base + (reg >> 2));
> }
>
> -static inline void qe_ic_write(volatile __be32 __iomem * base, unsigned int reg,
> +static void qe_ic_write(__be32 __iomem *base, unsigned int reg,
> u32 value)
> {
> - out_be32(base + (reg >> 2), value);
> + iowrite32be(value, base + (reg >> 2));
> }
>
> static inline struct qe_ic *qe_ic_from_irq(unsigned int virq)
> @@ -375,8 +414,8 @@ static int qe_ic_host_map(struct irq_domain *h, unsigned int virq,
> .xlate = irq_domain_xlate_onetwocell,
> };
>
> -/* Return an interrupt vector or NO_IRQ if no interrupt is pending. */
> -unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
> +/* Return an interrupt vector or 0 if no interrupt is pending. */
> +static unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
> {
> int irq;
>
> @@ -386,13 +425,13 @@ unsigned int qe_ic_get_low_irq(struct qe_ic *qe_ic)
> irq = qe_ic_read(qe_ic->regs, QEIC_CIVEC) >> 26;
>
> if (irq == 0)
> - return NO_IRQ;
> + return 0;
>
> return irq_linear_revmap(qe_ic->irqhost, irq);
> }
>
> -/* Return an interrupt vector or NO_IRQ if no interrupt is pending. */
> -unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
> +/* Return an interrupt vector or 0 if no interrupt is pending. */
> +static unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
> {
> int irq;
>
> @@ -402,11 +441,38 @@ unsigned int qe_ic_get_high_irq(struct qe_ic *qe_ic)
> irq = qe_ic_read(qe_ic->regs, QEIC_CHIVEC) >> 26;
>
> if (irq == 0)
> - return NO_IRQ;
> + return 0;
>
> return irq_linear_revmap(qe_ic->irqhost, irq);
> }
>
> +static void qe_ic_cascade_mpic(struct irq_desc *desc, int is_high)
> +{
> + struct qe_ic *qe_ic = irq_desc_get_handler_data(desc);
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + unsigned int cascade_irq;
> +
> + if (is_high)
> + cascade_irq = qe_ic_get_high_irq(qe_ic);
> + else
> + cascade_irq = qe_ic_get_low_irq(qe_ic);
> +
> + if (cascade_irq != 0)
> + generic_handle_irq(cascade_irq);
> +
> + chip->irq_eoi(&desc->irq_data);

NAK. This is not how we deal with chained interrupt controllers. If
that's what you want to do, implement a proper chained irqchip, use
and the chained_irq{enter,exit} macros.

Thanks,

M.

--
Jazz is not dead, it just smell funny.