Re: [OpenBIOS] [PATCH] macio: Rename arch_nvram_{put,get,size}()

[Alexander Graf]

On 08.06.2013, at 12:05, Andreas Färber wrote:
> PReP does not have MacIO, so don't call its functions arch_*.
> Instead indirect from arch_* functions to macio_* functions via is_apple().
> Signed-off-by: Andreas Färber
>
> arch/ppc/qemu/qemu.c      | 22 ++++++++++++++++++++++
> drivers/macio.c           |  6 +++---
> include/drivers/drivers.h |  5 +++++
> 3 files changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/arch/ppc/qemu/qemu.c b/arch/ppc/qemu/qemu.c
> index 208669c..bed5bb3 100644
> --- a/arch/ppc/qemu/qemu.c
> +++ b/arch/ppc/qemu/qemu.c
> @@ -76,3 +76,25 @@ printk( const char *fmt, ... )
>    }
>    return i;
> }
>
> +int arch_nvram_size(void)
> +{
> if (is_apple()) {
>    return macio_nvram_size();
> }

Please add a comment here that NVRAM support for non-Apple machines is a TODO item.
> return 0;
>
> +}
>
> +void arch_nvram_put(char *buf)
> +{
> if (is_apple()) {
>    macio_nvram_put(buf);
> }
>
> +}
>
> +void arch_nvram_get(char *buf)
> +{
> if (is_apple()) {
>    macio_nvram_get(buf);
> }
>
> +}
> diff --git a/drivers/macio.c b/drivers/macio.c
> index 59ad1f6..c6aee0a 100644
> --- a/drivers/macio.c
> +++ b/drivers/macio.c
> @@ -35,7 +35,7 @@
> static char *nvram;
>
> int
> -arch_nvram_size( void )
> +macio_nvram_size(void)
> {
>         if (is_oldworld())
>                 return OW_IO_NVRAM_SIZE >> OW_IO_NVRAM_SHIFT;
> @@ -100,7 +100,7 @@ dump_nvram(void)
>
> void
> -arch_nvram_put( char *buf )
> +macio_nvram_put(char *buf)
> {
>    int i;
>         unsigned int it_shift;
> @@ -119,7 +119,7 @@ arch_nvram_put( char *buf )
> }
>
> void
> -arch_nvram_get( char *buf )
> +macio_nvram_get(char *buf)
> {
>    int i;
>         unsigned int it_shift;
> diff --git a/include/drivers/drivers.h b/include/drivers/drivers.h
> index d139ace..5e1f9f1 100644
> --- a/include/drivers/drivers.h
> +++ b/include/drivers/drivers.h
> @@ -123,6 +123,11 @@ int keyboard_dataready(void);
> unsigned char keyboard_readdata(void);
> #endif
> #endif
> +#ifdef CONFIG_DRIVER_MACIO
> +int macio_nvram_size(void);
> +void macio_nvram_put(char *buf);
> +void macio_nvram_get(char *buf);
> +#endif

I'd just always leave them defined. The linker will tell you if you accessed any of these without including the respective .c file.

The rest looks all good to me :). It's certainly a good cleanup.

Alex