Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1313183 > unrolled thread

Re: [PATCH 4/6] MIPS: ath79: irq: Move the MISC driver to drivers/irqchip

Started byMarc Zyngier <marc.zyngier@arm.com>
First post2016-01-20 13:40 +0100
Last post2016-01-20 21:00 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 4/6] MIPS: ath79: irq: Move the MISC driver to  drivers/irqchip Marc Zyngier <marc.zyngier@arm.com> - 2016-01-20 13:40 +0100
    Re: [PATCH 4/6] MIPS: ath79: irq: Move the MISC driver to  drivers/irqchip Alban <albeu@free.fr> - 2016-01-20 21:00 +0100

#1313183 — Re: [PATCH 4/6] MIPS: ath79: irq: Move the MISC driver to drivers/irqchip

FromMarc Zyngier <marc.zyngier@arm.com>
Date2016-01-20 13:40 +0100
SubjectRe: [PATCH 4/6] MIPS: ath79: irq: Move the MISC driver to drivers/irqchip
Message-ID<qT0dl-6mw-15@gated-at.bofh.it>
On Tue, 17 Nov 2015 20:34:54 +0100
Alban Bedel <albeu@free.fr> wrote:

Hi Alban,

> The driver stays the same but the initialization changes a bit.
> For OF boards we now get the memory map from the OF node and use
> a linear mapping instead of the legacy mapping. For legacy boards
> we still use a legacy mapping and just pass down all the parameters
> from the board init code.
> 
> Signed-off-by: Alban Bedel <albeu@free.fr>
> ---
>  arch/mips/ath79/irq.c                    | 163 +++------------------------
>  arch/mips/include/asm/mach-ath79/ath79.h |   3 +
>  drivers/irqchip/Makefile                 |   1 +
>  drivers/irqchip/irq-ath79-misc.c         | 182 +++++++++++++++++++++++++++++++
>  4 files changed, 201 insertions(+), 148 deletions(-)
>  create mode 100644 drivers/irqchip/irq-ath79-misc.c
> 

[...]

> diff --git a/drivers/irqchip/irq-ath79-misc.c b/drivers/irqchip/irq-ath79-misc.c
> new file mode 100644
> index 0000000..bd2121f1
> --- /dev/null
> +++ b/drivers/irqchip/irq-ath79-misc.c
> @@ -0,0 +1,182 @@
> +/*
> + *  Atheros AR71xx/AR724x/AR913x MISC interrupt controller
> + *
> + *  Copyright (C) 2015 Alban Bedel <albeu@free.fr>
> + *  Copyright (C) 2010-2011 Jaiganesh Narayanan <jnarayanan@atheros.com>
> + *  Copyright (C) 2008-2011 Gabor Juhos <juhosg@openwrt.org>
> + *  Copyright (C) 2008 Imre Kaloz <kaloz@openwrt.org>
> + *
> + *  Parts of this file are based on Atheros' 2.6.15/2.6.31 BSP
> + *
> + *  This program is free software; you can redistribute it and/or modify it
> + *  under the terms of the GNU General Public License version 2 as published
> + *  by the Free Software Foundation.
> + */
> +
> +#include <linux/irqchip.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +
> +#define AR71XX_RESET_REG_MISC_INT_STATUS	0
> +#define AR71XX_RESET_REG_MISC_INT_ENABLE	4
> +
> +#define ATH79_MISC_IRQ_COUNT			32
> +
> +static void ath79_misc_irq_handler(struct irq_desc *desc)
> +{
> +	struct irq_domain *domain = irq_desc_get_handler_data(desc);
> +	void __iomem *base = domain->host_data;
> +	u32 pending;
> +
> +	pending = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_STATUS) &
> +		  __raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +
> +	if (!pending) {
> +		spurious_interrupt();
> +		return;
> +	}
> +
> +	while (pending) {
> +		int bit = __ffs(pending);
> +
> +		generic_handle_irq(irq_linear_revmap(domain, bit));
> +		pending &= ~BIT(bit);
> +	}
> +}

Given that this function is used as a chained handler, it seems to be
missing the usual chained_irq_enter/exit...

> +
> +static void ar71xx_misc_irq_unmask(struct irq_data *d)
> +{
> +	void __iomem *base = irq_data_get_irq_chip_data(d);
> +	unsigned int irq = d->hwirq;
> +	u32 t;
> +
> +	t = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +	__raw_writel(t | BIT(irq), base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +
> +	/* flush write */
> +	__raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +}
> +
> +static void ar71xx_misc_irq_mask(struct irq_data *d)
> +{
> +	void __iomem *base = irq_data_get_irq_chip_data(d);
> +	unsigned int irq = d->hwirq;
> +	u32 t;
> +
> +	t = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +	__raw_writel(t & ~BIT(irq), base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +
> +	/* flush write */
> +	__raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +}
> +
> +static void ar724x_misc_irq_ack(struct irq_data *d)
> +{
> +	void __iomem *base = irq_data_get_irq_chip_data(d);
> +	unsigned int irq = d->hwirq;
> +	u32 t;
> +
> +	t = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_STATUS);
> +	__raw_writel(t & ~BIT(irq), base + AR71XX_RESET_REG_MISC_INT_STATUS);
> +
> +	/* flush write */
> +	__raw_readl(base + AR71XX_RESET_REG_MISC_INT_STATUS);
> +}
> +
> +static struct irq_chip ath79_misc_irq_chip = {
> +	.name		= "MISC",
> +	.irq_unmask	= ar71xx_misc_irq_unmask,
> +	.irq_mask	= ar71xx_misc_irq_mask,
> +};
> +
> +static int misc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> +{
> +	irq_set_chip_and_handler(irq, &ath79_misc_irq_chip, handle_level_irq);
> +	irq_set_chip_data(irq, d->host_data);
> +	return 0;
> +}
> +
> +static const struct irq_domain_ops misc_irq_domain_ops = {
> +	.xlate = irq_domain_xlate_onecell,
> +	.map = misc_map,
> +};
> +
> +static void __init ath79_misc_intc_domain_init(
> +	struct irq_domain *domain, int irq)
> +{
> +	void __iomem *base = domain->host_data;
> +
> +	/* Disable and clear all interrupts */
> +	__raw_writel(0, base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> +	__raw_writel(0, base + AR71XX_RESET_REG_MISC_INT_STATUS);
> +
> +	irq_set_chained_handler_and_data(irq, ath79_misc_irq_handler, domain);
> +}
> +
> +static int __init ath79_misc_intc_of_init(
> +	struct device_node *node, struct device_node *parent)
> +{
> +	struct irq_domain *domain;
> +	void __iomem *base;
> +	int irq;
> +
> +	irq = irq_of_parse_and_map(node, 0);
> +	if (!irq) {
> +		pr_err("Failed to get MISC IRQ\n");
> +		return -EINVAL;
> +	}
> +
> +	base = of_iomap(node, 0);
> +	if (!base) {
> +		pr_err("Failed to get MISC IRQ registers\n");
> +		return -ENOMEM;
> +	}
> +
> +	domain = irq_domain_add_linear(node, ATH79_MISC_IRQ_COUNT,
> +				&misc_irq_domain_ops, base);
> +	if (!domain) {
> +		pr_err("Failed to add MISC irqdomain\n");
> +		return -EINVAL;
> +	}
> +
> +	ath79_misc_intc_domain_init(domain, irq);
> +	return 0;
> +}
> +
> +static int __init ar7100_misc_intc_of_init(
> +	struct device_node *node, struct device_node *parent)
> +{
> +	ath79_misc_irq_chip.irq_mask_ack = ar71xx_misc_irq_mask;
> +	return ath79_misc_intc_of_init(node, parent);
> +}
> +
> +IRQCHIP_DECLARE(ar7100_misc_intc, "qca,ar7100-misc-intc",
> +		ar7100_misc_intc_of_init);
> +
> +static int __init ar7240_misc_intc_of_init(
> +	struct device_node *node, struct device_node *parent)
> +{
> +	ath79_misc_irq_chip.irq_ack = ar724x_misc_irq_ack;
> +	return ath79_misc_intc_of_init(node, parent);
> +}
> +
> +IRQCHIP_DECLARE(ar7240_misc_intc, "qca,ar7240-misc-intc",
> +		ar7240_misc_intc_of_init);
> +
> +void __init ath79_misc_irq_init(void __iomem *regs, int irq,
> +				int irq_base, bool is_ar71xx)
> +{
> +	struct irq_domain *domain;
> +
> +	if (is_ar71xx)
> +		ath79_misc_irq_chip.irq_mask_ack = ar71xx_misc_irq_mask;
> +	else
> +		ath79_misc_irq_chip.irq_ack = ar724x_misc_irq_ack;
> +
> +	domain = irq_domain_add_legacy(NULL, ATH79_MISC_IRQ_COUNT,
> +			irq_base, 0, &misc_irq_domain_ops, regs);
> +	if (!domain)
> +		panic("Failed to create MISC irqdomain");
> +
> +	ath79_misc_intc_domain_init(domain, irq);
> +}

Other than that, this looks OK.

Thanks,

	M.
-- 
AAAFNRAA

[toc] | [next] | [standalone]

#1313476

FromAlban <albeu@free.fr>
Date2016-01-20 21:00 +0100
Message-ID<qT758-2KF-7@gated-at.bofh.it>
In reply to#1313183
On Wed, 20 Jan 2016 12:38:41 +0000
Marc Zyngier <marc.zyngier@arm.com> wrote:

> On Tue, 17 Nov 2015 20:34:54 +0100
> Alban Bedel <albeu@free.fr> wrote:
> 
> Hi Alban,
> 
> > The driver stays the same but the initialization changes a bit.
> > For OF boards we now get the memory map from the OF node and use
> > a linear mapping instead of the legacy mapping. For legacy boards
> > we still use a legacy mapping and just pass down all the parameters
> > from the board init code.
> > 
> > Signed-off-by: Alban Bedel <albeu@free.fr>
> > ---
> >  arch/mips/ath79/irq.c                    | 163 +++------------------------
> >  arch/mips/include/asm/mach-ath79/ath79.h |   3 +
> >  drivers/irqchip/Makefile                 |   1 +
> >  drivers/irqchip/irq-ath79-misc.c         | 182 +++++++++++++++++++++++++++++++
> >  4 files changed, 201 insertions(+), 148 deletions(-)
> >  create mode 100644 drivers/irqchip/irq-ath79-misc.c
> > 
> 
> [...]
> 
> > diff --git a/drivers/irqchip/irq-ath79-misc.c b/drivers/irqchip/irq-ath79-misc.c
> > new file mode 100644
> > index 0000000..bd2121f1
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-ath79-misc.c
> > @@ -0,0 +1,182 @@
> > +/*
> > + *  Atheros AR71xx/AR724x/AR913x MISC interrupt controller
> > + *
> > + *  Copyright (C) 2015 Alban Bedel <albeu@free.fr>
> > + *  Copyright (C) 2010-2011 Jaiganesh Narayanan <jnarayanan@atheros.com>
> > + *  Copyright (C) 2008-2011 Gabor Juhos <juhosg@openwrt.org>
> > + *  Copyright (C) 2008 Imre Kaloz <kaloz@openwrt.org>
> > + *
> > + *  Parts of this file are based on Atheros' 2.6.15/2.6.31 BSP
> > + *
> > + *  This program is free software; you can redistribute it and/or modify it
> > + *  under the terms of the GNU General Public License version 2 as published
> > + *  by the Free Software Foundation.
> > + */
> > +
> > +#include <linux/irqchip.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +
> > +#define AR71XX_RESET_REG_MISC_INT_STATUS	0
> > +#define AR71XX_RESET_REG_MISC_INT_ENABLE	4
> > +
> > +#define ATH79_MISC_IRQ_COUNT			32
> > +
> > +static void ath79_misc_irq_handler(struct irq_desc *desc)
> > +{
> > +	struct irq_domain *domain = irq_desc_get_handler_data(desc);
> > +	void __iomem *base = domain->host_data;
> > +	u32 pending;
> > +
> > +	pending = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_STATUS) &
> > +		  __raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +
> > +	if (!pending) {
> > +		spurious_interrupt();
> > +		return;
> > +	}
> > +
> > +	while (pending) {
> > +		int bit = __ffs(pending);
> > +
> > +		generic_handle_irq(irq_linear_revmap(domain, bit));
> > +		pending &= ~BIT(bit);
> > +	}
> > +}
> 
> Given that this function is used as a chained handler, it seems to be
> missing the usual chained_irq_enter/exit...

I'll fix this.

> > +
> > +static void ar71xx_misc_irq_unmask(struct irq_data *d)
> > +{
> > +	void __iomem *base = irq_data_get_irq_chip_data(d);
> > +	unsigned int irq = d->hwirq;
> > +	u32 t;
> > +
> > +	t = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +	__raw_writel(t | BIT(irq), base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +
> > +	/* flush write */
> > +	__raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +}
> > +
> > +static void ar71xx_misc_irq_mask(struct irq_data *d)
> > +{
> > +	void __iomem *base = irq_data_get_irq_chip_data(d);
> > +	unsigned int irq = d->hwirq;
> > +	u32 t;
> > +
> > +	t = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +	__raw_writel(t & ~BIT(irq), base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +
> > +	/* flush write */
> > +	__raw_readl(base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +}
> > +
> > +static void ar724x_misc_irq_ack(struct irq_data *d)
> > +{
> > +	void __iomem *base = irq_data_get_irq_chip_data(d);
> > +	unsigned int irq = d->hwirq;
> > +	u32 t;
> > +
> > +	t = __raw_readl(base + AR71XX_RESET_REG_MISC_INT_STATUS);
> > +	__raw_writel(t & ~BIT(irq), base + AR71XX_RESET_REG_MISC_INT_STATUS);
> > +
> > +	/* flush write */
> > +	__raw_readl(base + AR71XX_RESET_REG_MISC_INT_STATUS);
> > +}
> > +
> > +static struct irq_chip ath79_misc_irq_chip = {
> > +	.name		= "MISC",
> > +	.irq_unmask	= ar71xx_misc_irq_unmask,
> > +	.irq_mask	= ar71xx_misc_irq_mask,
> > +};
> > +
> > +static int misc_map(struct irq_domain *d, unsigned int irq, irq_hw_number_t hw)
> > +{
> > +	irq_set_chip_and_handler(irq, &ath79_misc_irq_chip, handle_level_irq);
> > +	irq_set_chip_data(irq, d->host_data);
> > +	return 0;
> > +}
> > +
> > +static const struct irq_domain_ops misc_irq_domain_ops = {
> > +	.xlate = irq_domain_xlate_onecell,
> > +	.map = misc_map,
> > +};
> > +
> > +static void __init ath79_misc_intc_domain_init(
> > +	struct irq_domain *domain, int irq)
> > +{
> > +	void __iomem *base = domain->host_data;
> > +
> > +	/* Disable and clear all interrupts */
> > +	__raw_writel(0, base + AR71XX_RESET_REG_MISC_INT_ENABLE);
> > +	__raw_writel(0, base + AR71XX_RESET_REG_MISC_INT_STATUS);
> > +
> > +	irq_set_chained_handler_and_data(irq, ath79_misc_irq_handler, domain);
> > +}
> > +
> > +static int __init ath79_misc_intc_of_init(
> > +	struct device_node *node, struct device_node *parent)
> > +{
> > +	struct irq_domain *domain;
> > +	void __iomem *base;
> > +	int irq;
> > +
> > +	irq = irq_of_parse_and_map(node, 0);
> > +	if (!irq) {
> > +		pr_err("Failed to get MISC IRQ\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	base = of_iomap(node, 0);
> > +	if (!base) {
> > +		pr_err("Failed to get MISC IRQ registers\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	domain = irq_domain_add_linear(node, ATH79_MISC_IRQ_COUNT,
> > +				&misc_irq_domain_ops, base);
> > +	if (!domain) {
> > +		pr_err("Failed to add MISC irqdomain\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ath79_misc_intc_domain_init(domain, irq);
> > +	return 0;
> > +}
> > +
> > +static int __init ar7100_misc_intc_of_init(
> > +	struct device_node *node, struct device_node *parent)
> > +{
> > +	ath79_misc_irq_chip.irq_mask_ack = ar71xx_misc_irq_mask;
> > +	return ath79_misc_intc_of_init(node, parent);
> > +}
> > +
> > +IRQCHIP_DECLARE(ar7100_misc_intc, "qca,ar7100-misc-intc",
> > +		ar7100_misc_intc_of_init);
> > +
> > +static int __init ar7240_misc_intc_of_init(
> > +	struct device_node *node, struct device_node *parent)
> > +{
> > +	ath79_misc_irq_chip.irq_ack = ar724x_misc_irq_ack;
> > +	return ath79_misc_intc_of_init(node, parent);
> > +}
> > +
> > +IRQCHIP_DECLARE(ar7240_misc_intc, "qca,ar7240-misc-intc",
> > +		ar7240_misc_intc_of_init);
> > +
> > +void __init ath79_misc_irq_init(void __iomem *regs, int irq,
> > +				int irq_base, bool is_ar71xx)
> > +{
> > +	struct irq_domain *domain;
> > +
> > +	if (is_ar71xx)
> > +		ath79_misc_irq_chip.irq_mask_ack = ar71xx_misc_irq_mask;
> > +	else
> > +		ath79_misc_irq_chip.irq_ack = ar724x_misc_irq_ack;
> > +
> > +	domain = irq_domain_add_legacy(NULL, ATH79_MISC_IRQ_COUNT,
> > +			irq_base, 0, &misc_irq_domain_ops, regs);
> > +	if (!domain)
> > +		panic("Failed to create MISC irqdomain");
> > +
> > +	ath79_misc_intc_domain_init(domain, irq);
> > +}
> 
> Other than that, this looks OK.
> 
> Thanks,

Great, thanks for the review.

Alban

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web