public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: arnd@arndb•de (Arnd Bergmann)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 12/14] ARM: SPEAr3xx: shirq: simplify and move the shared irq multiplexor to DT
Date: Mon, 12 Nov 2012 15:09:44 +0000	[thread overview]
Message-ID: <201211121509.45176.arnd@arndb.de> (raw)
In-Reply-To: <71bbb1faf3cd048953dcf24ef9ce6d9dd37fe8e1.1352608333.git.viresh.kumar@linaro.org>

On Sunday 11 November 2012, Viresh Kumar wrote:
> From: Shiraz Hashim <shiraz.hashim@st•com>
> 
> SPEAr3xx architecture includes shared/multiplexed irqs for certain set
> of devices. The multiplexor provides a single interrupt to parent
> interrupt controller (VIC) on behalf of a group of devices.
> 
> There can be multiple groups available on SPEAr3xx variants but not
> exceeding 4. The number of devices in a group can differ, further they
> may share same set of status/mask registers spanning across different
> bit masks. Also in some cases the group may not have enable or other
> registers. This makes software little complex.
> 
> Present implementation was non-DT and had few complex data structures to
> decipher banks, number of irqs supported, mask and registers involved.
> 
> This patch simplifies the overall design and convert it in to DT.  It
> also removes all registration from individual SoC files and bring them
> in to common shirq.c.
> 
> Also updated the corresponding documentation for DT binding of shirq.

Looks basically ok, but I have a few comments.

> Signed-off-by: Shiraz Hashim <shiraz.hashim@st•com>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro•org>
> ---
>  .../devicetree/bindings/arm/spear/shirq.txt        |  48 ++++
>  arch/arm/mach-spear3xx/include/mach/irqs.h         |  10 +-
>  arch/arm/mach-spear3xx/spear300.c                  | 103 -------
>  arch/arm/mach-spear3xx/spear310.c                  | 202 --------------
>  arch/arm/mach-spear3xx/spear320.c                  | 204 --------------
>  arch/arm/mach-spear3xx/spear3xx.c                  |   4 +
>  arch/arm/plat-spear/include/plat/shirq.h           |  35 +--
>  arch/arm/plat-spear/shirq.c                        | 305 +++++++++++++++++----

I guess it would be nice to move this to drivers/irqchip/st-shirq.c now
that we have introduced that directory.

>  static const char * const spear320_dt_board_compat[] = {
> diff --git a/arch/arm/mach-spear3xx/spear3xx.c b/arch/arm/mach-spear3xx/spear3xx.c
> index 98144ba..781aec9 100644
> --- a/arch/arm/mach-spear3xx/spear3xx.c
> +++ b/arch/arm/mach-spear3xx/spear3xx.c
> @@ -121,6 +122,9 @@ struct sys_timer spear3xx_timer = {
>  
>  static const struct of_device_id vic_of_match[] __initconst = {
>  	{ .compatible = "arm,pl190-vic", .data = vic_of_init, },
> +	{ .compatible = "st,spear300-shirq", .data = spear3xx_shirq_of_init, },
> +	{ .compatible = "st,spear310-shirq", .data = spear3xx_shirq_of_init, },
> +	{ .compatible = "st,spear320-shirq", .data = spear3xx_shirq_of_init, },
>  	{ /* Sentinel */ }
>  };

You list three "compatible" values here with the same init function, and then

> +int __init spear3xx_shirq_of_init(struct device_node *np,
> +		struct device_node *parent)
> +{
> +	struct spear_shirq **shirq_blocks;
> +	void __iomem *base;
> +	int block_nr, ret;
> +
> +	base = of_iomap(np, 0);
> +	if (!base) {
> +		pr_err("%s: failed to map shirq registers\n", __func__);
> +		return -ENXIO;
> +	}
> +
> +	if (of_device_is_compatible(np, "st,spear300-shirq")) {
> +		shirq_blocks = spear300_shirq_blocks;
> +		block_nr = ARRAY_SIZE(spear300_shirq_blocks);
> +	} else if (of_device_is_compatible(np, "st,spear310-shirq")) {
> +		shirq_blocks = spear310_shirq_blocks;
> +		block_nr = ARRAY_SIZE(spear310_shirq_blocks);
> +	} else if (of_device_is_compatible(np, "st,spear320-shirq")) {
> +		shirq_blocks = spear320_shirq_blocks;
> +		block_nr = ARRAY_SIZE(spear320_shirq_blocks);
> +	} else {
> +		pr_err("%s: unknown platform\n", __func__);
> +		ret = -EINVAL;
> +		goto unmap;
> +	}
> +
> +	ret = shirq_init(shirq_blocks, block_nr, base, np);
> +	if (ret) {
> +		pr_err("%s: shirq initialization failed\n", __func__);
> +		goto unmap;
> +	}
> +
> +	return ret;
> +
> +unmap:
> +	iounmap(base);
> +	return ret;
> +}

In that multiplex between thre three again. I think it would be cleaner to have
three separate functions and move the call to of_iomap into shirq_init.

	Arnd

  reply	other threads:[~2012-11-12 15:09 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-11  4:39 [PATCH 00/14] ARM: SPEAr: DT updates Viresh Kumar
2012-11-11  4:39 ` [PATCH 01/14] pinctrl: SPEAr: add spi chipselect control driver Viresh Kumar
2012-11-12 15:03   ` Arnd Bergmann
2012-11-15 14:17   ` Linus Walleij
2012-11-15 14:19     ` Viresh Kumar
2012-11-15 19:09       ` Linus Walleij
2012-11-16  5:15   ` [PATCH 01/16] gpio: " Viresh Kumar
2012-11-16  8:19     ` Arnd Bergmann
2012-11-16  8:19       ` Viresh Kumar
2012-11-17 23:02     ` Linus Walleij
2012-11-11  4:39 ` [PATCH 02/14] ARM: SPEAr13xx: DT: Add spics gpio controller nodes Viresh Kumar
2012-11-13 14:08   ` Linus Walleij
2012-11-13 14:34     ` Viresh Kumar
2012-11-15 14:06       ` Linus Walleij
2012-11-15 14:08         ` Viresh Kumar
2012-11-15 17:25           ` Linus Walleij
2012-11-11  4:39 ` [PATCH 03/14] ARM: SPEAr: DT: Update pinctrl list Viresh Kumar
2012-11-11  4:39 ` [PATCH 04/14] ARM: SPEAr: DT: Update partition info for MTD devices Viresh Kumar
2012-11-11  4:39 ` [PATCH 05/14] ARM: SPEAr: DT: Fix existing DT support Viresh Kumar
2012-11-11  4:39 ` [PATCH 06/14] ARM: SPEAr: DT: Modify DT bindings for STMMAC Viresh Kumar
2012-11-11  4:39 ` [PATCH 07/14] ARM: SPEAr: DT: add uart state to fix warning Viresh Kumar
2012-11-11  4:39 ` [PATCH 08/14] ARM: SPEAr: DT: Update device nodes Viresh Kumar
2012-11-26 11:25   ` Viresh Kumar
2012-11-11  4:39 ` [PATCH 09/14] ARM: SPEAr1310: Move 1310 specific misc register into machine specific files Viresh Kumar
2012-11-11  4:39 ` [PATCH 10/14] ARM: SPEAr13xx: Remove fields not required for ssp controller Viresh Kumar
2012-11-11  4:39 ` [PATCH 11/14] ARM: SPEAr1310: Fix AUXDATA for compact flash controller Viresh Kumar
2012-11-11  4:39 ` [PATCH 12/14] ARM: SPEAr3xx: shirq: simplify and move the shared irq multiplexor to DT Viresh Kumar
2012-11-12 15:09   ` Arnd Bergmann [this message]
2012-11-12 16:37     ` viresh kumar
2012-11-12 17:00       ` Arnd Bergmann
2012-11-12 17:16   ` [PATCH] fixup! " Viresh Kumar
2012-11-12 17:35   ` [PATCH] ARM: SPEAr3xx: Shirq: Move shirq controller out of plat/ Viresh Kumar
2012-11-11  4:39 ` [PATCH 13/14] ARM: SPEAr3xx: DT: add shirq node for interrupt multiplexor Viresh Kumar
2012-11-11  4:39 ` [PATCH 14/14] ARM: SPEAr320: DT: Add SPEAr 320 HMI board support Viresh Kumar
2012-11-12 15:21 ` [PATCH 00/14] ARM: SPEAr: DT updates Arnd Bergmann
2012-11-12 16:39   ` viresh kumar
2012-11-12 17:18   ` Viresh Kumar
2012-11-21 20:24     ` Olof Johansson
2012-11-22  3:19       ` Viresh Kumar
2012-11-22  9:47         ` Linus Walleij

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201211121509.45176.arnd@arndb.de \
    --to=arnd@arndb$(echo .)de \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox