| Message ID | 1328237992-14953-14-git-send-email-afaerber@suse.de |
|---|---|
| State | New |
| Headers | show |
On 3 February 2012 02:59, Andreas Färber <afaerber@suse.de> wrote: > That way we can remove some more CPUID cases without losing info. > > Signed-off-by: Andreas Färber <afaerber@suse.de> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: Andrzej Zaborowski <balrogg@gmail.com> > --- > target-arm/cpu-core.h | 1 + > target-arm/cpu.c | 2 ++ > target-arm/cpu.h | 5 ----- > target-arm/helper.c | 8 -------- > 4 files changed, 3 insertions(+), 13 deletions(-) > > diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h > index 3e7dfae..eac1a03 100644 > --- a/target-arm/cpu-core.h > +++ b/target-arm/cpu-core.h > @@ -48,6 +48,7 @@ typedef struct ARMCPUClass { > uint32_t c0_c2[8]; > uint32_t c1_sys; > } cp15; > + uint64_t jtag_id; If we're not using this anywhere we should just not have it. -- PMM
Am 07.02.2012 18:47, schrieb Peter Maydell: > On 3 February 2012 02:59, Andreas Färber <afaerber@suse.de> wrote: >> That way we can remove some more CPUID cases without losing info. >> >> Signed-off-by: Andreas Färber <afaerber@suse.de> >> Cc: Peter Maydell <peter.maydell@linaro.org> >> Cc: Andrzej Zaborowski <balrogg@gmail.com> >> --- >> target-arm/cpu-core.h | 1 + >> target-arm/cpu.c | 2 ++ >> target-arm/cpu.h | 5 ----- >> target-arm/helper.c | 8 -------- >> 4 files changed, 3 insertions(+), 13 deletions(-) >> >> diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h >> index 3e7dfae..eac1a03 100644 >> --- a/target-arm/cpu-core.h >> +++ b/target-arm/cpu-core.h >> @@ -48,6 +48,7 @@ typedef struct ARMCPUClass { >> uint32_t c0_c2[8]; >> uint32_t c1_sys; >> } cp15; >> + uint64_t jtag_id; > > If we're not using this anywhere we should just not have it. Andrzej will have had his reasons to put it in the code. This series just moves code around so I don't want to remove it without his ack. I cc'ed him in the hopes of getting an explanation. :) Andreas
On 7 February 2012 18:41, Andreas Färber <afaerber@suse.de> wrote: > Am 07.02.2012 18:47, schrieb Peter Maydell: >> On 3 February 2012 02:59, Andreas Färber <afaerber@suse.de> wrote: >>> + uint64_t jtag_id; >> >> If we're not using this anywhere we should just not have it. > > Andrzej will have had his reasons to put it in the code. This series > just moves code around so I don't want to remove it without his ack. That was my point -- as far as I can see this patch *is* adding code and a struct field that wasn't there before. -- PMM
Am 07.02.2012 20:06, schrieb Peter Maydell: > On 7 February 2012 18:41, Andreas Färber <afaerber@suse.de> wrote: >> Am 07.02.2012 18:47, schrieb Peter Maydell: >>> On 3 February 2012 02:59, Andreas Färber <afaerber@suse.de> wrote: >>>> + uint64_t jtag_id; >>> >>> If we're not using this anywhere we should just not have it. >> >> Andrzej will have had his reasons to put it in the code. This series >> just moves code around so I don't want to remove it without his ack. > > That was my point -- as far as I can see this patch *is* adding code > and a struct field that wasn't there before. Huh? Look closely, it *was* a comment in the original code. And I don't want to *move* that comment over into my new code. Therefore I'm adding a class field where such informational data can properly be stored and inspected - per class, not per CPUState instance. I can imagine storing much more information in a CPU class such as vendor name*, description, logo, link to docs and chip color ;) to allow for a nice lspci-like inspection. * Yeah, in case of ARM the vendor can be inferred from CPUID so a getter would be sufficient but you get the idea. Again, I don't specifically need a jtag_id field, but this data is definitely coming from existing code. If Andrzej doesn't respond here, feel free to send a trivial patch to remove it from there. Andreas
On 7 February 2012 22:07, Andreas Färber <afaerber@suse.de> wrote: > Am 07.02.2012 20:06, schrieb Peter Maydell: >> On 7 February 2012 18:41, Andreas Färber <afaerber@suse.de> wrote: >>> Andrzej will have had his reasons to put it in the code. This series >>> just moves code around so I don't want to remove it without his ack. >> >> That was my point -- as far as I can see this patch *is* adding code >> and a struct field that wasn't there before. > > Huh? Look closely, it *was* a comment in the original code. Exactly. It was a comment; now it's code. I don't think we should be adding code. > And I don't want to *move* that comment over into my new code. You can move the comment, or you can just drop it; I'm happy with either. I don't want new code/struct fields we don't use. -- PMM
diff --git a/target-arm/cpu-core.h b/target-arm/cpu-core.h index 3e7dfae..eac1a03 100644 --- a/target-arm/cpu-core.h +++ b/target-arm/cpu-core.h @@ -48,6 +48,7 @@ typedef struct ARMCPUClass { uint32_t c0_c2[8]; uint32_t c1_sys; } cp15; + uint64_t jtag_id; /* Internal CPU feature flags. */ uint32_t features; diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 2c1e1a6..7d4f0f6 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -223,6 +223,7 @@ static void pxa25x_class_init(ARMCPUClass *k, const ARMCPUInfo *info) { k->cp15.c0_cachetype = 0xd172172; k->cp15.c1_sys = 0x00000078; + k->jtag_id = ((uint64_t)k->cp15.c0_cpuid << 28) | 0x09265013; set_class_feature(k, ARM_FEATURE_V5); set_class_feature(k, ARM_FEATURE_XSCALE); @@ -232,6 +233,7 @@ static void pxa270_class_init(ARMCPUClass *k, const ARMCPUInfo *info) { k->cp15.c0_cachetype = 0xd172172; k->cp15.c1_sys = 0x00000078; + k->jtag_id = ((uint64_t)k->cp15.c0_cpuid << 28) | 0x09265013; set_class_feature(k, ARM_FEATURE_V5); set_class_feature(k, ARM_FEATURE_XSCALE); diff --git a/target-arm/cpu.h b/target-arm/cpu.h index e16befd..351b057 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -406,11 +406,6 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum, #define ARM_CPUID_ARM926 0x41069265 #define ARM_CPUID_TI915T 0x54029152 #define ARM_CPUID_TI925T 0x54029252 -#define ARM_CPUID_PXA250 0x69052100 -#define ARM_CPUID_PXA255 0x69052d00 -#define ARM_CPUID_PXA260 0x69052903 -#define ARM_CPUID_PXA261 0x69052d05 -#define ARM_CPUID_PXA262 0x69052d06 #define ARM_CPUID_PXA270_A0 0x69054110 #define ARM_CPUID_PXA270_A1 0x69054111 #define ARM_CPUID_PXA270_B0 0x69054112 diff --git a/target-arm/helper.c b/target-arm/helper.c index 32319e4..bf28fea 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -76,20 +76,12 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id) env->cp15.c15_i_max = 0x000; env->cp15.c15_i_min = 0xff0; break; - case ARM_CPUID_PXA250: - case ARM_CPUID_PXA255: - case ARM_CPUID_PXA260: - case ARM_CPUID_PXA261: - case ARM_CPUID_PXA262: - /* JTAG_ID is ((id << 28) | 0x09265013) */ - break; case ARM_CPUID_PXA270_A0: case ARM_CPUID_PXA270_A1: case ARM_CPUID_PXA270_B0: case ARM_CPUID_PXA270_B1: case ARM_CPUID_PXA270_C0: case ARM_CPUID_PXA270_C5: - /* JTAG_ID is ((id << 28) | 0x09265013) */ env->iwmmxt.cregs[ARM_IWMMXT_wCID] = 0x69051000 | 'Q'; break; default:
That way we can remove some more CPUID cases without losing info. Signed-off-by: Andreas Färber <afaerber@suse.de> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: Andrzej Zaborowski <balrogg@gmail.com> --- target-arm/cpu-core.h | 1 + target-arm/cpu.c | 2 ++ target-arm/cpu.h | 5 ----- target-arm/helper.c | 8 -------- 4 files changed, 3 insertions(+), 13 deletions(-)