lkml.org 
[lkml]   [2008]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 2/2]: sparc64: Add Niagara2 RNG driver.
From
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.


\
 
 \ /
  Last update: 2008-06-04 00:55    [W:0.072 / U:0.436 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site