linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: thierry.reding@gmail.com (Thierry Reding)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 3/4] soc/tegra: Initialize interrupt controller from DT
Date: Sat, 28 Jun 2014 03:12:09 +0200	[thread overview]
Message-ID: <20140628011208.GA13290@ulmo> (raw)
In-Reply-To: <53ADDC0C.4060401@wwwdotorg.org>

On Fri, Jun 27, 2014 at 03:03:08PM -0600, Stephen Warren wrote:
> On 06/27/2014 10:58 AM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Obtains the register ranges for the legacy interrupt controller from DT
> > and provide hard-coded values as fallback.
> 
> > diff --git a/drivers/soc/tegra/irq.c b/drivers/soc/tegra/irq.c
> 
> >  void __init tegra_init_irq(void)
> 
> > -	distbase = IO_ADDRESS(TEGRA_ARM_INT_DIST_BASE);
> > +	np = of_find_matching_node(NULL, ictlr_matches);
> > +	if (np) {
> > +		for (i = 0; i < ARRAY_SIZE(ictlr_regs); i++)
> > +			if (of_address_to_resource(np, i, &res) == 0)
> > +				ictlr_regs[i] = res;
> 
> It'd be nice to check that loop ran exactly the number of times expected
> based on the compatible value (i.e. SoC) if the legacy interrupt
> controller node.
> 
> What about erroring out if entries are missing or can't be parsed.

I've added some strict checking of the expected number of entries. I
decided against erroring out on missing entries or when they can't be
parsed because we can still continue using the fallback entries in the
ictlr_regs table. However if a mismatch between the number of detected
entries and the expected entries is detected, the code will now WARN.

> >  	if (num_ictlrs > ARRAY_SIZE(ictlr_reg_base)) {
> > -		WARN(1, "Too many (%d) interrupt controllers found. Maximum is %d.",
> > +		WARN(1, "Too many (%d) interrupt controllers found. Maximum is %zu.",
> >  			num_ictlrs, ARRAY_SIZE(ictlr_reg_base));
> 
> While we're changing this, maybe we should change that test to
> "num_ictlrs != the expected value", so too few is found as well.

Done. This is slightly complicated by the fact that the non-DT code has
no notion of how many are expected. The maximum number is only used to
prevent overflows when accessing the ictlr_reg_base array.

I've solved this by selecting the maximum depending on tegra_chip_id. I
have sent a v2 to the list with the patch removed that moves all the
drivers to drivers/soc since more work will be required for that. In the
meantime this change is separate and can be merged independent of that.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140628/a0ff50f7/attachment.sig>

  reply	other threads:[~2014-06-28  1:12 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-27 16:58 [RFC 1/4] ARM: tegra: Move SoC drivers to drivers/soc/tegra Thierry Reding
2014-06-27 16:58 ` [RFC 2/4] ARM: tegra: Add legacy interrupt controller nodes Thierry Reding
2014-06-27 20:58   ` Stephen Warren
2014-06-27 23:44     ` Thierry Reding
2014-06-27 16:58 ` [RFC 3/4] soc/tegra: Initialize interrupt controller from DT Thierry Reding
2014-06-27 21:03   ` Stephen Warren
2014-06-28  1:12     ` Thierry Reding [this message]
2014-06-30 11:30   ` Peter De Schrijver
2014-06-30 19:51     ` Thierry Reding
2014-06-27 16:58 ` [RFC 4/4] soc/tegra: Remove unused defines Thierry Reding
2014-06-27 21:03   ` Stephen Warren
2014-06-28  1:12     ` Thierry Reding
2014-06-27 17:30 ` [RFC 1/4] ARM: tegra: Move SoC drivers to drivers/soc/tegra Santosh Shilimkar
2014-06-27 23:27   ` Thierry Reding
2014-06-28 17:15     ` Santosh Shilimkar
2014-06-28 20:40       ` Thierry Reding
2014-06-30  7:20         ` Arnd Bergmann
2014-06-30  9:01           ` Thierry Reding
2014-06-30 10:36             ` Catalin Marinas
2014-06-30 10:48               ` Thierry Reding
2014-06-30 13:16                 ` Lorenzo Pieralisi
2014-06-30 19:36                   ` Thierry Reding
2014-07-01 10:50                     ` Catalin Marinas
2014-07-01 15:05                       ` Stephen Warren
2014-07-01 17:00                         ` Catalin Marinas
2014-06-30 19:21           ` Thierry Reding
2014-07-01  7:51             ` Peter De Schrijver
2014-07-16 19:31           ` Olof Johansson
2014-07-16 19:47             ` Thierry Reding
2014-07-17  9:31             ` Catalin Marinas
2014-07-17 16:21               ` Olof Johansson
2014-06-30  7:23       ` Arnd Bergmann
2014-06-30  9:44         ` Catalin Marinas
2014-06-30 18:45         ` Stephen Warren
2014-06-30 10:25     ` Catalin Marinas
2014-06-30 10:49       ` Thierry Reding
2014-06-30 11:46       ` Peter De Schrijver
2014-06-30 14:13         ` Catalin Marinas
2014-06-30 14:42           ` Peter De Schrijver
2014-06-30 14:50             ` Peter De Schrijver
2014-06-30 16:08             ` Catalin Marinas
2014-06-27 21:10 ` Stephen Warren
2014-06-28  1:24   ` Thierry Reding

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=20140628011208.GA13290@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).