lkml.org 
[lkml]   [2012]   [Sep]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 08/31] arm64: CPU support
    On Wed, Aug 15, 2012 at 01:10:43AM +0100, Olof Johansson wrote:
    > On Tue, Aug 14, 2012 at 06:52:09PM +0100, Catalin Marinas wrote:
    > > +#ifndef __ASM_CPUTYPE_H
    > > +#define __ASM_CPUTYPE_H
    > > +
    > > +#define ID_MIDR_EL1 "midr_el1"
    > > +#define ID_CTR_EL0 "ctr_el0"
    > > +
    > > +#define ID_AA64PFR0_EL1 "id_aa64pfr0_el1"
    > > +#define ID_AA64DFR0_EL1 "id_aa64dfr0_el1"
    > > +#define ID_AA64AFR0_EL1 "id_aa64afr0_el1"
    > > +#define ID_AA64ISAR0_EL1 "id_aa64isar0_el1"
    > > +#define ID_AA64MMFR0_EL1 "id_aa64mmfr0_el1"
    > > +
    > > +#define read_cpuid(reg) ({ \
    > > + u64 __val; \
    > > + asm("mrs %0, " reg : "=r" (__val)); \
    > > + __val; \
    > > +})
    > > +
    > > +/*
    > > + * The CPU ID never changes at run time, so we might as well tell the
    > > + * compiler that it's constant. Use this function to read the CPU ID
    > > + * rather than directly reading processor_id or read_cpuid() directly.
    > > + */
    > > +static inline u32 __attribute_const__ read_cpuid_id(void)
    > > +{
    > > + return read_cpuid(ID_MIDR_EL1);
    > > +}
    > > +
    > > +static inline u32 __attribute_const__ read_cpuid_cachetype(void)
    > > +{
    > > + return read_cpuid(ID_CTR_EL0);
    > > +}
    >
    > Is this perhaps a carry-over from arch/arm? Abstracting out read_cpuid()
    > doesn't seem to buy anything here, just opencode the one-line assembly
    > in each.

    read_cpuid() is called from several other files under arch/arm64 and
    also used in expressions, so it's a good abstraction.

    > > --- /dev/null
    > > +++ b/arch/arm64/mm/proc.S
    > > @@ -0,0 +1,193 @@
    > > + .section ".proc.info.init", #alloc, #execinstr
    > > +
    > > + .type __v8_proc_info, #object
    > > +__v8_proc_info:
    > > + .long 0x000f0000 // Required ID value
    > > + .long 0x000f0000 // Mask for ID
    > > + b __cpu_setup
    > > + nop
    > > + .quad cpu_name
    > > + .long 0
    > > + .size __v8_proc_info, . - __v8_proc_info
    >
    > I know this is a carry-over from arch/arm, but how about moving this
    > to more of a C construct similar to arch/powerpc/kernel/cputable.c
    > instead? It's considerably easier to read that way, and it's convenient
    > to have the definitions all in one place, making it easier to share some
    > of the functions, etc.

    Done in version 3. It's easier to read :).

    --
    Catalin


    \
     
     \ /
      Last update: 2012-09-14 20:21    [W:0.038 / U:3.660 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site