From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S967500Ab3DRJff (ORCPT <rfc822;w@1wt.eu>);
	Thu, 18 Apr 2013 05:35:35 -0400
Received: from www.linutronix.de ([62.245.132.108]:43226 "EHLO
	Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S967442Ab3DRJfa (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Thu, 18 Apr 2013 05:35:30 -0400
Date: Thu, 18 Apr 2013 11:35:22 +0200 (CEST)
From: Thomas Gleixner <tglx@linutronix.de>
To: =?ISO-8859-15?Q?Uwe_Kleine-K=F6nig?= 
	<u.kleine-koenig@pengutronix.de>
cc: kernel@pengutronix.de, linux-arm-kernel@lists.infradead.org,
        linux-kernel@vger.kernel.org,
        Jonathan Austin <jonathan.austin@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH v3] irqchip: Add support for ARMv7-M's NVIC
In-Reply-To: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>
Message-ID: <alpine.LFD.2.02.1304180947070.21884@ionos>
References: <1366214540-31166-1-git-send-email-u.kleine-koenig@pengutronix.de>
User-Agent: Alpine 2.02 (LFD 1266 2009-07-14)
MIME-Version: 1.0
Content-Type: MULTIPART/MIXED; BOUNDARY="8323328-2068579284-1366272803=:21884"
Content-ID: <alpine.LFD.2.02.1304181131040.21884@ionos>
X-Linutronix-Spam-Score: -1.0
X-Linutronix-Spam-Level: -
X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required,  ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323328-2068579284-1366272803=:21884
Content-Type: TEXT/PLAIN; CHARSET=ISO-8859-15
Content-Transfer-Encoding: 8BIT
Content-ID: <alpine.LFD.2.02.1304181131041.21884@ionos>

On Wed, 17 Apr 2013, Uwe Kleine-König wrote:
> +struct nvic_bank_data {
> +	/*
> +	 * For irq i base holds nvic_base + 4 * i / 32. So you can access the
> +	 * right ISER register (i.e ISER[i / 32]) by just taking base + ISER.
> +	 * Ditto for ICER.
> +	 */
> +	void __iomem *base;
> +};

What's the point of a struct with a single member? Why not having an
array of base pointers ?

> +static struct nvic_chip_data {
> +	struct irq_domain *domain;
> +	struct nvic_bank_data bdata[NVIC_MAX_BANKS];
> +} nvic_chip_data;
> +
> +asmlinkage void __exception_irq_entry
> +nvic_do_IRQ(irq_hw_number_t hwirq, struct pt_regs *regs)
> +{
> +	unsigned int irq = irq_linear_revmap(nvic_chip_data.domain, hwirq);
> +
> +	handle_IRQ(irq, regs);
> +}
> +
> +static inline void __iomem *nvic_bank_base(struct irq_data *d)
> +{
> +	struct nvic_bank_data *bank_data = irq_data_get_irq_chip_data(d);
> +	return bank_data->base;
> +}
> +
> +static void nvic_mask_irq(struct irq_data *d)
> +{
> +	u32 mask = 1 << (d->hwirq % 32);
> +
> +	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ICER);
> +}
> +
> +static void nvic_unmask_irq(struct irq_data *d)
> +{
> +	u32 mask = 1 << (d->hwirq % 32);
> +
> +	writel_relaxed(mask, nvic_bank_base(d) + NVIC_ISER);
> +}

How is that different from what the generic irq chip implementation
does? The only difference is that mask is generated by d->hwirq and
not by d->irq. And due to the fact, that you use a full linear mapping
between hwirq and virq the generic code simply works.

Even if it would not work, it would be trivial to extend the generic
chip with that functionality instead of hacking another slightly
different copy of the same thing.

Thanks,

	tglx


--8323328-2068579284-1366272803=:21884--