On 03/20/2014 07:25 PM, Simon Glass wrote: > Hi Stephen, > > On 20 March 2014 12:57, Stephen Warren <swar...@wwwdotorg.org> wrote: >> >> On 03/14/2014 01:37 PM, Simon Glass wrote: >>> Hi Stephen, >>> >>> On 13 March 2014 11:42, Stephen Warren <swar...@wwwdotorg.org> wrote: >>>> From: Stephen Warren <swar...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
