Re: [RFC 3/4] soc/tegra: Initialize interrupt controller from DT

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > 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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]