On 03/20/2014 07:25 PM, Simon Glass wrote:
Hi Stephen,
On 20 March 2014 12:57, Stephen Warren swarren@wwwdotorg.org wrote:
On 03/14/2014 01:37 PM, Simon Glass wrote:
Hi Stephen,
On 13 March 2014 11:42, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Much of arch/arm/cpu/tegra*-common/pinmux.c is identical. Remove the duplication by creating pinmux-common.c for all the identical code.
This leaves:
- arch/arm/include/asm/arch-tegra*/pinmux.h defining only the names of the various pins/pin groups, drive groups, and mux functions.
- arch/arm/cpu/tegra*-common/pinmux.c containing only the lookup table stating which pin groups support which mux functions.
The code in pinmux-common.c is semantically identical to that in the various original pinmux.c, but had some consistency and cleanup fixes applied during migration.
diff --git a/arch/arm/cpu/tegra-common/pinmux-common.c b/arch/arm/cpu/tegra-common/pinmux-common.c
+/* return 1 if a pin_pupd_is in range */ +#define pmux_pin_pupd_isvalid(pupd) \
(((pupd) >= PMUX_PULL_NORMAL) && ((pupd) <= PMUX_PULL_UP))+/* return 1 if a pin_tristate_is in range */ +#define pmux_pin_tristate_isvalid(tristate) \
(((tristate) >= PMUX_TRI_NORMAL) && ((tristate) <= PMUX_TRI_TRISTATE))+#ifdef TEGRA_PMX_HAS_PIN_IO_BIT_ETC
Do we need this #Ifdef?
+/* return 1 if a pin_io_is in range */ +#define pmux_pin_io_isvalid(io) \
(((io) >= PMUX_PIN_OUTPUT) && ((io) <= PMUX_PIN_INPUT))We certainly need not to compile this code, since e.g. PMUX_PIN_INPUT doesn't exist on Tegra20 due to equivalent #ifdefs in pinmux.h.
I do explicitly want to keep the ifdefs in pinmux.h, so that APIs are not prototyped, and values not defined, for features that don't exist on the SoC that U-Boot is being built for. This validates at compile time that code isn't using invalid APIs. While pinmux.h could be split up into a few separate header files to avoid the ifdefs, I think that would make the header situation far more complex than it needs to be.
Arguably you have created this problem by having #ifdefs in the C file
- if there was a separate file for each SoC then it would be much
harder to mess this up.
Well, then you get a link error rather than a compiler error for an unprototyped function or undefined enum/#define. I think the compile error is a bit more obvious myself, but granted either would work.
...
So in summary, I'd like to keep the ifdefs. I think they're pretty simple and not a maintenance burden. Do you object?
No. I can't possibly object given that you have completed such a great clean-up.
Great, thanks. I'll post V2 tomorrow.