From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stephen Warren Date: Thu, 20 Mar 2014 22:07:46 -0600 Subject: [U-Boot] [PATCH 05/11] ARM: tegra: pinctrl: remove duplication In-Reply-To: References: <1394732527-13961-1-git-send-email-swarren@wwwdotorg.org> <1394732527-13961-6-git-send-email-swarren@wwwdotorg.org> <532B482A.20503@wwwdotorg.org> Message-ID: <532BBB12.9090501@wwwdotorg.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 03/20/2014 07:25 PM, Simon Glass wrote: > Hi Stephen, > > On 20 March 2014 12:57, Stephen Warren wrote: >> >> On 03/14/2014 01:37 PM, Simon Glass wrote: >>> Hi Stephen, >>> >>> On 13 March 2014 11:42, Stephen Warren wrote: >>>> From: Stephen Warren >>>> >>>> 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.