| Message ID | 1354290073-55801-1-git-send-email-c.dall@virtualopensystems.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Nov 30, 2012 at 10:41:13AM -0500, Christoffer Dall wrote: > Decoding the implementor and part number of the CPU id in the CPU ID > register is needed by KVM, so we factor it out to share the code. > > Cc: Will Deacon <will.deacon@arm.com> > Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com> > --- > Changes since v2: > - Take implementor as argument to read_cpuid_part_number Actually, I don't think this is the correct way. There's noting inherently different about Xscale wrt part numbers, it's just that someone decided to do stuff "differently" and use the CPU part number to identify the SoC as a whole. Here's the full list: 80200 69052000 fffffff0 PXA250 69052100 fffff7f0 PXA210 69052120 fffff3f0 8032x 69052420 fffff7e0 PXA255 69052d00 fffffff0 80219 69052e20 ffffffe0 8033x 69054010 fffffd30 IXP43x 69054040 fffffff0 PXA270 69054110 fffffff0 IXP2400 69054190 fffffff0 IXP2800 690541a0 fffffff0 IXP42x 690541c0 ffffffc0 IXP46x 69054200 ffffff00 Xscale3 69056000 ffffe000 PXA935 56056000 ffffe000 What we get from these is: XScale1 can be identified by (id & 0xe000) == 0x2000 Xscale2 can be identified by (id & 0xe000) == 0x4000 Xscale3 can be identified by (id & 0xe000) == 0x6000 I don't think we should make a distinction in read_cpuid_part_number() between these; if manufacturers are silly enough to abuse these fields then its their problem. :) What I suggest is that read_cpuid_part_number() returns the part number field as defined by ARM. We then also define XSCALE_ARCH_MASK to be 0xe000 and a bunch of XSCALE_ARCH_V1..V3 along those lines. Maybe even a xscale_cpu_version() macro which returns the XScale CPU version pre-masked. That would mean this becomes: > @@ -202,47 +202,48 @@ static int __devinit probe_current_pmu(struct arm_pmu *pmu) > { > int cpu = get_cpu(); > unsigned long cpuid = read_cpuid_id(); > - unsigned long implementor = (cpuid & 0xFF000000) >> 24; > - unsigned long part_number = (cpuid & 0xFFF0); > + unsigned long implementor = read_cpuid_implementor(); Why not pass the cpuid that we read into these functions? > + unsigned long part_number; unsigned long part_number = read_cpuid_part_number(cpuid); > int ret = -ENODEV; > > pr_info("probing PMU on CPU %d\n", cpu); > > /* ARM Ltd CPUs. */ > - if (0x41 == implementor) { > + if (implementor == ARM_CPU_IMP_ARM) { > + part_number = read_cpuid_part_number(implementor); This goes. > switch (part_number) { > - case 0xB360: /* ARM1136 */ > - case 0xB560: /* ARM1156 */ > - case 0xB760: /* ARM1176 */ > + case ARM_CPU_PART_ARM1136: > + case ARM_CPU_PART_ARM1156: > + case ARM_CPU_PART_ARM1176: > ret = armv6pmu_init(pmu); > break; > - case 0xB020: /* ARM11mpcore */ > + case ARM_CPU_PART_ARM11MPCORE: > ret = armv6mpcore_pmu_init(pmu); > break; > - case 0xC080: /* Cortex-A8 */ > + case ARM_CPU_PART_CORTEX_A8: > ret = armv7_a8_pmu_init(pmu); > break; > - case 0xC090: /* Cortex-A9 */ > + case ARM_CPU_PART_CORTEX_A9: > ret = armv7_a9_pmu_init(pmu); > break; > - case 0xC050: /* Cortex-A5 */ > + case ARM_CPU_PART_CORTEX_A5: > ret = armv7_a5_pmu_init(pmu); > break; > - case 0xC0F0: /* Cortex-A15 */ > + case ARM_CPU_PART_CORTEX_A15: > ret = armv7_a15_pmu_init(pmu); > break; > - case 0xC070: /* Cortex-A7 */ > + case ARM_CPU_PART_CORTEX_A7: > ret = armv7_a7_pmu_init(pmu); > break; > } > /* Intel CPUs [xscale]. */ > - } else if (0x69 == implementor) { > - part_number = (cpuid >> 13) & 0x7; > + } else if (implementor == ARM_CPU_IMP_INTEL) { > + part_number = read_cpuid_part_number(implementor); > switch (part_number) { switch (xscale_cpu_version(part_number)) { > - case 1: > + case ARM_CPU_PART_XSCALE1: case XSCALE_ARCH_V1: > ret = xscale1pmu_init(pmu); > break; > - case 2: > + case ARM_CPU_PART_XSCALE2: case XSCALE_ARCH_V2: and we can use read_cpuid_part_number() where getting at that field in the CPU ID information matters.
On Fri, Nov 30, 2012 at 04:03:38PM +0000, Russell King - ARM Linux wrote: > On Fri, Nov 30, 2012 at 10:41:13AM -0500, Christoffer Dall wrote: > > Decoding the implementor and part number of the CPU id in the CPU ID > > register is needed by KVM, so we factor it out to share the code. > > > > Cc: Will Deacon <will.deacon@arm.com> > > Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com> > > --- > > Changes since v2: > > - Take implementor as argument to read_cpuid_part_number > > Actually, I don't think this is the correct way. There's noting > inherently different about Xscale wrt part numbers, it's just that > someone decided to do stuff "differently" and use the CPU part > number to identify the SoC as a whole. > > Here's the full list: > > 80200 69052000 fffffff0 > PXA250 69052100 fffff7f0 > PXA210 69052120 fffff3f0 > 8032x 69052420 fffff7e0 > PXA255 69052d00 fffffff0 > 80219 69052e20 ffffffe0 > 8033x 69054010 fffffd30 > IXP43x 69054040 fffffff0 > PXA270 69054110 fffffff0 > IXP2400 69054190 fffffff0 > IXP2800 690541a0 fffffff0 > IXP42x 690541c0 ffffffc0 > IXP46x 69054200 ffffff00 > Xscale3 69056000 ffffe000 > PXA935 56056000 ffffe000 > > What we get from these is: > XScale1 can be identified by (id & 0xe000) == 0x2000 > Xscale2 can be identified by (id & 0xe000) == 0x4000 > Xscale3 can be identified by (id & 0xe000) == 0x6000 > > I don't think we should make a distinction in read_cpuid_part_number() > between these; if manufacturers are silly enough to abuse these fields > then its their problem. :) > > What I suggest is that read_cpuid_part_number() returns the part number > field as defined by ARM. We then also define XSCALE_ARCH_MASK to be > 0xe000 and a bunch of XSCALE_ARCH_V1..V3 along those lines. Maybe even > a xscale_cpu_version() macro which returns the XScale CPU version > pre-masked. Here's the other place where the CPU ID is parsed on Xscale devices: drivers/usb/gadget/pxa25x_udc.c-#define CP15R0_VENDOR_MASK 0xffffe000 drivers/usb/gadget/pxa25x_udc.c:#define CP15R0_XSCALE_VALUE 0x69052000 /* intel/arm/xscale */ drivers/usb/gadget/pxa25x_udc.c:#define CP15R0_XSCALE_VALUE 0x69054000 /* intel/arm/ixp4xx */ drivers/usb/gadget/pxa25x_udc.c- asm("mrc%? p15, 0, %0, c0, c0" : "=r" (chiprev)); drivers/usb/gadget/pxa25x_udc.c: if ((chiprev & CP15R0_VENDOR_MASK) != CP15R0_XSCALE_VALUE) { drivers/usb/gadget/pxa25x_udc.c- pr_err("%s: not XScale!\n", driver_name); drivers/usb/gadget/pxa25x_udc.c- return -ENODEV; drivers/usb/gadget/pxa25x_udc.c- }
On Fri, Nov 30, 2012 at 11:03 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Fri, Nov 30, 2012 at 10:41:13AM -0500, Christoffer Dall wrote: >> Decoding the implementor and part number of the CPU id in the CPU ID >> register is needed by KVM, so we factor it out to share the code. >> >> Cc: Will Deacon <will.deacon@arm.com> >> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com> >> --- >> Changes since v2: >> - Take implementor as argument to read_cpuid_part_number > > Actually, I don't think this is the correct way. There's noting > inherently different about Xscale wrt part numbers, it's just that > someone decided to do stuff "differently" and use the CPU part > number to identify the SoC as a whole. > > Here's the full list: > > 80200 69052000 fffffff0 > PXA250 69052100 fffff7f0 > PXA210 69052120 fffff3f0 > 8032x 69052420 fffff7e0 > PXA255 69052d00 fffffff0 > 80219 69052e20 ffffffe0 > 8033x 69054010 fffffd30 > IXP43x 69054040 fffffff0 > PXA270 69054110 fffffff0 > IXP2400 69054190 fffffff0 > IXP2800 690541a0 fffffff0 > IXP42x 690541c0 ffffffc0 > IXP46x 69054200 ffffff00 > Xscale3 69056000 ffffe000 > PXA935 56056000 ffffe000 > > What we get from these is: > XScale1 can be identified by (id & 0xe000) == 0x2000 > Xscale2 can be identified by (id & 0xe000) == 0x4000 > Xscale3 can be identified by (id & 0xe000) == 0x6000 > > I don't think we should make a distinction in read_cpuid_part_number() > between these; if manufacturers are silly enough to abuse these fields > then its their problem. :) > > What I suggest is that read_cpuid_part_number() returns the part number > field as defined by ARM. We then also define XSCALE_ARCH_MASK to be > 0xe000 and a bunch of XSCALE_ARCH_V1..V3 along those lines. Maybe even > a xscale_cpu_version() macro which returns the XScale CPU version > pre-masked. > > That would mean this becomes: > sounds great to me. >> @@ -202,47 +202,48 @@ static int __devinit probe_current_pmu(struct arm_pmu *pmu) >> { >> int cpu = get_cpu(); >> unsigned long cpuid = read_cpuid_id(); >> - unsigned long implementor = (cpuid & 0xFF000000) >> 24; >> - unsigned long part_number = (cpuid & 0xFFF0); >> + unsigned long implementor = read_cpuid_implementor(); > > Why not pass the cpuid that we read into these functions? > we actually don't need that one anymore, and it all has __attribute_const__ anyhow. >> + unsigned long part_number; > unsigned long part_number = read_cpuid_part_number(cpuid); > >> int ret = -ENODEV; >> >> pr_info("probing PMU on CPU %d\n", cpu); >> >> /* ARM Ltd CPUs. */ >> - if (0x41 == implementor) { >> + if (implementor == ARM_CPU_IMP_ARM) { >> + part_number = read_cpuid_part_number(implementor); > > This goes. > >> switch (part_number) { >> - case 0xB360: /* ARM1136 */ >> - case 0xB560: /* ARM1156 */ >> - case 0xB760: /* ARM1176 */ >> + case ARM_CPU_PART_ARM1136: >> + case ARM_CPU_PART_ARM1156: >> + case ARM_CPU_PART_ARM1176: >> ret = armv6pmu_init(pmu); >> break; >> - case 0xB020: /* ARM11mpcore */ >> + case ARM_CPU_PART_ARM11MPCORE: >> ret = armv6mpcore_pmu_init(pmu); >> break; >> - case 0xC080: /* Cortex-A8 */ >> + case ARM_CPU_PART_CORTEX_A8: >> ret = armv7_a8_pmu_init(pmu); >> break; >> - case 0xC090: /* Cortex-A9 */ >> + case ARM_CPU_PART_CORTEX_A9: >> ret = armv7_a9_pmu_init(pmu); >> break; >> - case 0xC050: /* Cortex-A5 */ >> + case ARM_CPU_PART_CORTEX_A5: >> ret = armv7_a5_pmu_init(pmu); >> break; >> - case 0xC0F0: /* Cortex-A15 */ >> + case ARM_CPU_PART_CORTEX_A15: >> ret = armv7_a15_pmu_init(pmu); >> break; >> - case 0xC070: /* Cortex-A7 */ >> + case ARM_CPU_PART_CORTEX_A7: >> ret = armv7_a7_pmu_init(pmu); >> break; >> } >> /* Intel CPUs [xscale]. */ >> - } else if (0x69 == implementor) { >> - part_number = (cpuid >> 13) & 0x7; >> + } else if (implementor == ARM_CPU_IMP_INTEL) { >> + part_number = read_cpuid_part_number(implementor); >> switch (part_number) { > > switch (xscale_cpu_version(part_number)) { > >> - case 1: >> + case ARM_CPU_PART_XSCALE1: > > case XSCALE_ARCH_V1: > >> ret = xscale1pmu_init(pmu); >> break; >> - case 2: >> + case ARM_CPU_PART_XSCALE2: > > case XSCALE_ARCH_V2: > > and we can use read_cpuid_part_number() where getting at that field > in the CPU ID information matters. sending out a new patch. Thanks! -Christoffer
diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h index cb47d28..3e6cd4e 100644 --- a/arch/arm/include/asm/cputype.h +++ b/arch/arm/include/asm/cputype.h @@ -51,6 +51,22 @@ extern unsigned int processor_id; #define read_cpuid_ext(reg) 0 #endif +#define ARM_CPU_IMP_ARM 0x41 +#define ARM_CPU_IMP_INTEL 0x69 + +#define ARM_CPU_PART_ARM1136 0xB360 +#define ARM_CPU_PART_ARM1156 0xB560 +#define ARM_CPU_PART_ARM1176 0xB760 +#define ARM_CPU_PART_ARM11MPCORE 0xB020 +#define ARM_CPU_PART_CORTEX_A8 0xC080 +#define ARM_CPU_PART_CORTEX_A9 0xC090 +#define ARM_CPU_PART_CORTEX_A5 0xC050 +#define ARM_CPU_PART_CORTEX_A15 0xC0F0 +#define ARM_CPU_PART_CORTEX_A7 0xC070 + +#define ARM_CPU_PART_XSCALE1 0x1 +#define ARM_CPU_PART_XSCALE2 0x2 + /* * 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 @@ -61,6 +77,24 @@ static inline unsigned int __attribute_const__ read_cpuid_id(void) return read_cpuid(CPUID_ID); } +static inline unsigned int __attribute_const__ read_cpuid_implementor(void) +{ + return (read_cpuid_id() & 0xFF000000) >> 24; +} + +static inline unsigned int __attribute_const__ +read_cpuid_part_number(unsigned int implementor) +{ + switch (implementor) { + case ARM_CPU_IMP_ARM: + return (read_cpuid_id() & 0xFFF0); + case ARM_CPU_IMP_INTEL: + return (read_cpuid_id() >> 13) & 0x7; + default: + return 0; + } +} + static inline unsigned int __attribute_const__ read_cpuid_cachetype(void) { return read_cpuid(CPUID_CACHETYPE); diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c index 9a4f630..0ec7b54 100644 --- a/arch/arm/kernel/perf_event_cpu.c +++ b/arch/arm/kernel/perf_event_cpu.c @@ -202,47 +202,48 @@ static int __devinit probe_current_pmu(struct arm_pmu *pmu) { int cpu = get_cpu(); unsigned long cpuid = read_cpuid_id(); - unsigned long implementor = (cpuid & 0xFF000000) >> 24; - unsigned long part_number = (cpuid & 0xFFF0); + unsigned long implementor = read_cpuid_implementor(); + unsigned long part_number; int ret = -ENODEV; pr_info("probing PMU on CPU %d\n", cpu); /* ARM Ltd CPUs. */ - if (0x41 == implementor) { + if (implementor == ARM_CPU_IMP_ARM) { + part_number = read_cpuid_part_number(implementor); switch (part_number) { - case 0xB360: /* ARM1136 */ - case 0xB560: /* ARM1156 */ - case 0xB760: /* ARM1176 */ + case ARM_CPU_PART_ARM1136: + case ARM_CPU_PART_ARM1156: + case ARM_CPU_PART_ARM1176: ret = armv6pmu_init(pmu); break; - case 0xB020: /* ARM11mpcore */ + case ARM_CPU_PART_ARM11MPCORE: ret = armv6mpcore_pmu_init(pmu); break; - case 0xC080: /* Cortex-A8 */ + case ARM_CPU_PART_CORTEX_A8: ret = armv7_a8_pmu_init(pmu); break; - case 0xC090: /* Cortex-A9 */ + case ARM_CPU_PART_CORTEX_A9: ret = armv7_a9_pmu_init(pmu); break; - case 0xC050: /* Cortex-A5 */ + case ARM_CPU_PART_CORTEX_A5: ret = armv7_a5_pmu_init(pmu); break; - case 0xC0F0: /* Cortex-A15 */ + case ARM_CPU_PART_CORTEX_A15: ret = armv7_a15_pmu_init(pmu); break; - case 0xC070: /* Cortex-A7 */ + case ARM_CPU_PART_CORTEX_A7: ret = armv7_a7_pmu_init(pmu); break; } /* Intel CPUs [xscale]. */ - } else if (0x69 == implementor) { - part_number = (cpuid >> 13) & 0x7; + } else if (implementor == ARM_CPU_IMP_INTEL) { + part_number = read_cpuid_part_number(implementor); switch (part_number) { - case 1: + case ARM_CPU_PART_XSCALE1: ret = xscale1pmu_init(pmu); break; - case 2: + case ARM_CPU_PART_XSCALE2: ret = xscale2pmu_init(pmu); break; }
Decoding the implementor and part number of the CPU id in the CPU ID register is needed by KVM, so we factor it out to share the code. Cc: Will Deacon <will.deacon@arm.com> Signed-off-by: Christoffer Dall <c.dall@virtualopensystems.com> --- Changes since v2: - Take implementor as argument to read_cpuid_part_number Changes since v1: - Accidentally pointed to an old file, this one has more consistent naming of the cpu implementor and part number defines. arch/arm/include/asm/cputype.h | 34 ++++++++++++++++++++++++++++++++++ arch/arm/kernel/perf_event_cpu.c | 33 +++++++++++++++++---------------- 2 files changed, 51 insertions(+), 16 deletions(-)