On Thu, 17 Sep 2015, Oleksij Rempel wrote:
> static struct icoll_priv icoll_priv;
> static struct irq_domain *icoll_domain;
> +static DEFINE_RAW_SPINLOCK(icoll_lock);
> +
> +/* calculate bit offset depending on number of intterupt per register */
> +static u32 icoll_intr_bitshift(struct irq_data *d, u32 bit)
> +{
> + /*
> + * We expect intr_per_reg to be 4 or 1, it means
> + * "n" will be 3 or 0.
> + */
> + int n = icoll_priv.intr_per_reg - 1;
> +
> + /*
> + * If n = 0, "bit" is never shifted.
> + * If n = 3, mask lower part of hwirq to convert it
> + * in 0, 1, 2 or 3 and then multiply it by 8 (or shift by 3)
> + */
> + return bit << ((d->hwirq & n) << n);
> +}
> +
> +/* calculate mem offset depending on number of intterupt per register */
> +static void __iomem *icoll_intr_reg(struct irq_data *d)
> +{
> + int n = icoll_priv.intr_per_reg >> 1;
> +
> + /* offset = hwirq / intr_per_reg * 0x10 */
> + return icoll_priv.intr + ((d->hwirq >> n) * 0x10);
> +}
>
> static void icoll_ack_irq(struct irq_data *d)
> {
> @@ -76,14 +107,21 @@ static void icoll_ack_irq(struct irq_data *d)
>
> static void icoll_mask_irq(struct irq_data *d)
> {
> - __raw_writel(BM_ICOLL_INTR_ENABLE,
> - icoll_priv.intr + CLR_REG +
> HW_ICOLL_INTERRUPTn(d->hwirq));
> + __raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
> + icoll_intr_reg(d) + CLR_REG);
> }
>
> static void icoll_unmask_irq(struct irq_data *d)
> {
> - __raw_writel(BM_ICOLL_INTR_ENABLE,
> - icoll_priv.intr + SET_REG +
> HW_ICOLL_INTERRUPTn(d->hwirq));
> + raw_spin_lock(&icoll_lock);
> + if (icoll_priv.clear)
> + __raw_writel(ASM9260_BM_CLEAR_BIT(d->hwirq),
> + icoll_priv.clear +
> + ASM9260_HW_ICOLL_CLEARn(d->hwirq));
> +
> + __raw_writel(icoll_intr_bitshift(d, BM_ICOLL_INTR_ENABLE),
> + icoll_intr_reg(d) + SET_REG);
> + raw_spin_unlock(&icoll_lock);
> }
I don't think this is a brilliant idea. You burden the existing users
with that bit shift mask machinery, an extra conditional and the
spinlock in the unmask path.
You can simply implement a seperate pair of callbacks and assign them
at setup time.
Btw, why is that spinlock only in unmask()?
> static struct irq_chip mxs_icoll_chip = {
> @@ -115,12 +153,34 @@ static const struct irq_domain_ops icoll_irq_domain_ops
> = {
> .xlate = irq_domain_xlate_onecell,
> };
>
> +static void __init icoll_add_domain(struct device_node *np,
> + int num)
> +{
> + icoll_domain = irq_domain_add_linear(np, num,
> + &icoll_irq_domain_ops, NULL);
> +
> + if (!icoll_domain)
> + panic("%s: unable add irq domain", np->full_name);
> + irq_set_default_host(icoll_domain);
> + set_handle_irq(icoll_handle_irq);
> +}
> +
> +static void __iomem * __init icoll_init_iobase(struct device_node *np)
> +{
> + void __iomem *icoll_base;
> +
> + icoll_base = of_io_request_and_map(np, 0, np->name);
> + if (!icoll_base)
> + panic("%s: unable to map resource", np->full_name);
> + return icoll_base;
> +}
> +
> static int __init icoll_of_init(struct device_node *np,
> struct device_node *interrupt_parent)
> {
> - void __iomem *icoll_base = of_iomap(np, 0);
> - WARN_ON(!icoll_base);
> + void __iomem *icoll_base;
>
> + icoll_base = icoll_init_iobase(np);
> icoll_priv.vector = icoll_base + HW_ICOLL_VECTOR;
> icoll_priv.levelack = icoll_base + HW_ICOLL_LEVELACK;
> icoll_priv.ctrl = icoll_base + HW_ICOLL_CTRL;
> @@ -135,8 +195,38 @@ static int __init icoll_of_init(struct device_node *np,
> */
> stmp_reset_block(icoll_priv.ctrl);
>
> - icoll_domain = irq_domain_add_linear(np, ICOLL_NUM_IRQS,
> - &icoll_irq_domain_ops, NULL);
> + icoll_add_domain(np, ICOLL_NUM_IRQS);
> +
> return icoll_domain ? 0 : -ENODEV;
> }
That part of the patch should move into the preparatory patch. It has
nothing to do with asm9260.
Thanks,
tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
