Re: [PATCH] PPC/PPC64: Introduce CPU_HAS_FEATURE() macro

[Arnd Bergmann <arnd@arndb.de>]

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

On Freedag 04 Februar 2005 08:22, Olof Johansson wrote:
> It's getting pretty old to have see and type cur_cpu_spec->cpu_features
> & CPU_FTR_<feature>, when a shorter and less TLA-ridden macro is more
> readable.
> 
> This also takes care of the differences between PPC and PPC64 cpu
> features for the common code; most places in PPC could be replaced with
> the macro as well.

I have a somewhat similar patch that does the same to the
systemcfg->platform checks. I'm not sure if we should use the same inline
function for both checks, but I do think that they should be used in a
similar way, e.g. CPU_HAS_FEATURE(x) and PLATFORM_HAS_FEATURE(x).

My implementation of the platform checks tries to be extra clever by turning
runtime checks into compile time checks if possible. This reduces code size
and in some cases execution speed. It can also be used to replace compile
time checks, i.e. it allows us to write

static inline unsigned int readl(const volatile void __iomem *addr)
{
	if (platform_is(PLATFORM_PPC_ISERIES))
		return iSeries_readl(addr);
	if (platform_possible(PLATFORM_PPC_PSERIES))
		return eeh_readl(addr);
	return in_le32();
}

which will always result in the shortest code for any combination of
CONFIG_PPC_ISERIES, CONFIG_PPC_PSERIES and the other platforms.

The required code for this is roughly

enum {
	PPC64_PLATFORM_POSSIBLE =
#ifdef CONFIG_PPC_ISERIES
		PLATFORM_ISERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
		PLATFORM_PSERIES |
#endif
#ifdef CONFIG_PPC_PSERIES
		PLATFORM_PSERIES_LPAR |
#endif
#ifdef CONFIG_PPC_POWERMAC
		PLATFORM_POWERMAC |
#endif
#ifdef CONFIG_PPC_MAPLE
		PLATFORM_MAPLE |
#endif
		0,
	PPC64_PLATFORM_ONLY =
#ifdef CONFIG_PPC_ISERIES
		PLATFORM_ISERIES &
#endif
#ifdef CONFIG_PPC_PSERIES
		PLATFORM_PSERIES &
#endif
#ifdef CONFIG_PPC_POWERMAC
		PLATFORM_POWERMAC &
#endif
#ifdef CONFIG_PPC_MAPLE
		PLATFORM_MAPLE &
#endif
		-1ul,
};

static inline platform_is(unsigned long platform)
{
	return ((PPC64_PLATFORM_ONLY & platform) 
	 || (PPC64_PLATFORM_POSSIBLE & platform & systemcfg->platform));
}

static inline platform_possible(unsigned long platform)
{
	reutrn !!(PPC64_PLATFORM_POSSIBLE & platform);
}

The same stuff is obviously possible for cur_cpu_spec->cpu_features as well.
Do you think that it will help there?

	Arnd <><

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