Re: [PATCH 2/2]: sparc64: Add Niagara2 RNG driver.

[David Miller <davem@davemloft.net>]

From: Sam Ravnborg <sam@ravnborg.org>
Date: Sat, 17 May 2008 08:54:37 +0200

> > diff --git a/drivers/char/hw_random/Makefile b/drivers/char/hw_random/Makefile
> > index c8b7300..32b327b 100644
> > --- a/drivers/char/hw_random/Makefile
> > +++ b/drivers/char/hw_random/Makefile
> > @@ -7,6 +7,8 @@ rng-core-y := core.o
> >  obj-$(CONFIG_HW_RANDOM_INTEL) += intel-rng.o
> >  obj-$(CONFIG_HW_RANDOM_AMD) += amd-rng.o
> >  obj-$(CONFIG_HW_RANDOM_GEODE) += geode-rng.o
> > +obj-$(CONFIG_HW_RANDOM_N2RNG) += n2-rng.o
> > +n2-rng-objs := n2-drv.o n2-asm.o
> 
> These days the preferred syntax is:
> 
> n2-rng-y := n2-drv.o n2-asm.o
> 
> Following this scheme in general allows us to easily
> introduce:
> n2-rng-$(CONFIG_FOO) += foo.o

Thanks I've fixed this.

> > +#include <asm/hypervisor.h>
> > +#include "n2rng.h"
> > +
> > +	.text
> 
> I had expected:
> 
> 	.section .text, "aw"
> 
> here??

".text" is used consistently for this in the first 5 assembler files
I tossed into my editor (arch/x86/kernel/head_{32,64},
arch/x86/kernel/entry_{32,64}.S, arch/powerpc/kernel/head_64.S)
I don't see any value in using the expanded version, especially is
"aw" is not the default on some platform for some reason, that
detail is hidden in the assembler's implementation of ".text"

> > +	.globl	sun4v_rng_get_diag_ctl
> > +	.type	sun4v_rng_get_diag_ctl,#function
> > +sun4v_rng_get_diag_ctl:
> > +	mov	HV_FAST_RNG_GET_DIAG_CTL, %o5
> > +	ta	HV_FAST_TRAP
> > +	retl
> > +	 nop
> > +	.size	sun4v_rng_get_diag_ctl,.-sun4v_rng_get_diag_ctl
> 
> Any specific reason why you do not use:
> 
> ENTRY(sun4v_rng_get_diag_ctl)
>      mov     HV_FAST_RNG_GET_DIAG_CTL, %o5
>      ta      HV_FAST_TRAP
>      retl
>       nop
> ENDPROC(sun4v_rng_get_diag_ctl)

I simply didn't know they existed and were usable. :-)
I've changed the assembler code to use them, thanks.

Longer term I'll need to define __ALIGN et al. on sparc
so that a more appropriate fill constant is used.  It
should be a NOP in sections that should contain instructions
and 0x90 is not a NOP on sparc :-)

But it's harmless to use what's there now, so there is no
need for me to define a specific __ALIGN before using
ENTRY and ENDPROC.

Thanks for your feedback.