Re: [v2 3/3] ARM: tegra: Unify Device tree board files

[Hiroshi Doyu]

Stephen Warren <swarren@xxxxxxxxxxxxx> wrote @ Tue, 12 Feb 2013 05:47:20 +0100:

> >>>  static void __init harmony_init(void)
> >>>  {
> >>> -#ifdef CONFIG_TEGRA_PCI
> >>>  	int ret;
> >>>  
> >>>  	ret = harmony_pcie_init();
> >>>  	if (ret)
> >>>  		pr_err("harmony_pcie_init() failed: %d\n", ret);
> >>> -#endif
> >>>  }
> >>
> >> Why drop those ifdefs? Does the code still compile (link) if built for
> >> Tegra30-only or Tegra114-only, where the Tegra PCI driver won't be
> >> enabled, and hence those functions don't exist?
> > 
> > This function itself will be dropped by the following IS_ENABLED().
> > 
> >>>  static void __init paz00_init(void)
> >>> @@ -129,6 +128,9 @@ static void __init tegra_dt_init_late(void)
> >>>  
> >>>  	tegra_init_late();
> >>>  
> >>> +	if (IS_ENABLED(CONFIG_PCI) && IS_ENABLED(CONFIG_ARCH_TEGRA_2x_SOC))
> >>> +		return;
> >>
> >> I don't think that's going to help any link issues, so I'd drop it and
> >> keep this function simple.
> > 
> > As explained in the above, a complier will drop unnecessary functions
> > automatically with this IS_ENABLED(), which could save many ifdefs.
> 
> That sounds extremely brittle. Have you validated this on a wide variety
> of compiler versions?

I verified with gcc-4.6.
IIRC, that's the point of IS_ENABLED() being introduced. Arnd?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/