[PATCH 1/4] powerpc: Add firmware_set_feature()

[David Gibson]

On Thu, Jan 12, 2006 at 05:09:59PM +1100, Michael Ellerman wrote:
> We have firmware_has_feature() to test for features, add firmware_set_feature()
> so that no one needs to touch ppc64_firmware_features directly.
> 
> ppc64_firmware_features will be initialised to 0, but make it explicit.
> 
> Signed-off-by: Michael Ellerman <michael at ellerman.id.au>
> ---
> 
>  arch/powerpc/kernel/firmware.c         |    2 +-
>  arch/powerpc/platforms/iseries/setup.c |    2 +-
>  include/asm-powerpc/firmware.h         |    7 ++++++-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> 
> Index: linux/include/asm-powerpc/firmware.h
> ===================================================================
> --- linux.orig/include/asm-powerpc/firmware.h
> +++ linux/include/asm-powerpc/firmware.h
> @@ -79,7 +79,7 @@ enum {
>  };
>  
>  /* This is used to identify firmware features which are available
> - * to the kernel.
> + * to the kernel. Don't use this directly, use the macros below.
>   */
>  extern unsigned long	ppc64_firmware_features;
>  
> @@ -89,6 +89,11 @@ static inline unsigned long firmware_has
>  		(FW_FEATURE_POSSIBLE & ppc64_firmware_features & feature);
>  }
>  
> +static inline void firmware_set_feature(unsigned long feature)
> +{
> +	ppc64_firmware_features |= FW_FEATURE_POSSIBLE & feature;
> +}

Hrm.. should we really mask here, or just BUG_ON(! (feature &
FW_FEATURE_POSSIBLE))?

> +
>  #ifdef CONFIG_PPC_PSERIES
>  typedef struct {
>      unsigned long val;
> Index: linux/arch/powerpc/platforms/iseries/setup.c
> ===================================================================
> --- linux.orig/arch/powerpc/platforms/iseries/setup.c
> +++ linux/arch/powerpc/platforms/iseries/setup.c
> @@ -303,7 +303,7 @@ static void __init iSeries_init_early(vo
>  {
>  	DBG(" -> iSeries_init_early()\n");
>  
> -	ppc64_firmware_features = FW_FEATURE_ISERIES;
> +	firmware_set_feature(FW_FEATURE_ISERIES);
>  
>  	ppc64_interrupt_controller = IC_ISERIES;
>  
> Index: linux/arch/powerpc/kernel/firmware.c
> ===================================================================
> --- linux.orig/arch/powerpc/kernel/firmware.c
> +++ linux/arch/powerpc/kernel/firmware.c
> @@ -17,7 +17,7 @@
>  
>  #include <asm/firmware.h>
>  
> -unsigned long ppc64_firmware_features;
> +unsigned long ppc64_firmware_features = 0;

Kernel convention is generally not to explicitly initialize to zero.
Doing so forces the variable into .data rather than .bss, and therby
increases the vmlinux size.  I usually but a /* = 0 */ comment
instead.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson