Re: [PATCH] powerpc: New cpu type shows up as "unknown" even when logical PVR is recognized

[Dave Kleikamp <shaggy@linux.vnet.ibm.com>]

On Sun, 2009-02-15 at 01:00 +1100, Michael Ellerman wrote:
> On Fri, 2009-02-13 at 14:45 -0600, Dave Kleikamp wrote:
> > When identify_cpu() is called a second time with a logical PVR, it only
> > copies a subset of the cpu_spec structure to avoid overwriting the
> > performance monitor fields that were initialized based on the real PVR.
> > 
> > If the real PVR is not recognized, the structure is initialized with
> > default data in which the pvr_mask field is zero.  Overriding the default
> > definition with the one based on the logical PVR does not update the
> > pvr_mask field.  Since the pvr_mask field remains zero, show_cpuinfo()
> > reports the cpu as "unknown".
> > 
> > identify_cpu() should update the pvr_mask, so that show_cpuinfo() reports
> > the logical cpu name, as it would if the kernel recognized the real PVR.
> > 
> > For completeness, this patch also updates pvr_value, although its only
> > use appears to be in booke_wdt_init() setting ident.firmware_version.  I'm
> > not sure which value (the real or logical) is the most useful here, if it
> > matters at all.
> > 
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> > 
> > diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
> > index 923f87a..a216fad 100644
> > --- a/arch/powerpc/kernel/cputable.c
> > +++ b/arch/powerpc/kernel/cputable.c
> > @@ -1780,6 +1780,8 @@ struct cpu_spec * __init identify_cpu(unsigned long offset, unsigned int pvr)
> >  			 * performance monitor fields.
> >  			 */
> >  			if (t->num_pmcs && !s->num_pmcs) {
> > +				t->pvr_mask = s->pvr_mask;
> > +				t->pvr_value = s->pvr_value;
> >  				t->cpu_name = s->cpu_name;
> >  				t->cpu_features = s->cpu_features;
> >  				t->cpu_user_features = s->cpu_user_features;
> 
> Seems like we'd be better off copying the whole structure, and then
> putting back the performance monitor fields. So we don't end up with
> another bug if something's added to struct cpu_spec.

I feel that may be a bit of overkill, but I don't feel strongly either
way.  I'd like the patch to be picked up in one fashion or another.

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center