Re: [OpenBIOS] [PATCH v2] arch/ppc/qemu: Clean up and add more cpu infos

[BALATON Zoltan]

On Fri, 9 May 2014, BALATON Zoltan wrote:
> On Fri, 9 May 2014, Alexander Graf wrote:
> > > Am 09.05.2014 um 01:46 schrieb BALATON Zoltan :
> > > > On Thu, 8 May 2014, Alexander Graf wrote:
> > > > > On 04/17/2014 07:25 PM, BALATON Zoltan wrote:
> > > > > Removed unused clock_frequency member from struct cpudef which is get 
> > > > > from
> > > > > FW_CFG and added bus-frequency property to complete the frequency
> > > > > properties. Also added tlb infos to cpudef (with values only for G4
> > > > > currently as I have no data for other CPUs).
> > > > > Signed-off-by: BALATON Zoltan
> > > > >
> > > > > v2: Get bus-frequency from FW_CFG similarly to clock-frequency and
> > > > > tb-frequency (corresponding patch submitted to QEMU)
> > > > > Index: openbios-devel/arch/ppc/qemu/init.c
> > > > >
> > > > > --- openbios-devel/arch/ppc/qemu/init.c    (revision 1286)
> > > > > +++ openbios-devel/arch/ppc/qemu/init.c    (working copy)
> > > > > @@ -44,7 +44,7 @@
> > > > >      int icache_size, dcache_size;
> > > > >      int icache_sets, dcache_sets;
> > > > >      int icache_block_size, dcache_block_size;
> > > > > int clock_frequency;
> > > > > int tlb_sets, tlb_size;
> > > > > void (*initfn)(const struct cpudef *cpu);
> > > > >
> > > > > };
> > > > > @@ -269,6 +269,18 @@
> > > > >      push_str("i-cache-block-size");
> > > > >      fword("property");
> > > > > if(cpu->tlb_size) {
> > > > >    PUSH(cpu->tlb_sets);
> > > > >    fword("encode-int");
> > > > >    push_str("tlb-sets");
> > > > >    fword("property");
> > > > >    PUSH(cpu->tlb_size);
> > > > >    fword("encode-int");
> > > > >    push_str("tlb-size");
> > > > >    fword("property");
> > > > > }
> > > > > PUSH(fw_cfg_read_i32(FW_CFG_PPC_TBFREQ));
> > > > >    fword("encode-int");
> > > > >    push_str("timebase-frequency");
> > > > >
> > > > > @@ -279,6 +291,11 @@
> > > > >      push_str("clock-frequency");
> > > > >      fword("property");
> > > > > PUSH(fw_cfg_read_i32(FW_CFG_PPC_BUSFREQ));
> > > > > fword("encode-int");
> > > > > push_str("bus-frequency");
> > > > > fword("property");
> > > > > push_str("running");
> > > > >    fword("encode-string");
> > > > >    push_str("state");
> > > > >
> > > > > @@ -391,7 +408,8 @@
> > > > >          .dcache_sets = 0x80,
> > > > >          .icache_block_size = 0x20,
> > > > >          .dcache_block_size = 0x20,
> > > > >    .clock_frequency = 0x07de2900,
> > > > >    .tlb_sets = 0x00,
> > > > >    .tlb_size = 0x00,
> > > >
> > > > Oh, I was too quick to comment here. The if()  check above means we won't 
> > > > add those properties for these CPUs then.
> > > >
> > > > No need to add the fields in this case. C by default zero initializes 
> > > > unset fields.
> > >
> > > Do you prefer if I remove them? I though having them there does not hurt 
> > > and shows they could be set.
> >
> > Hrm, I don't have strong feelings either way :)
>
> So can this be applied as is or are there any modifications needed?

Who decides about this patch? (I'd like to clean my tree a little and get 
things that can be taken upstream merged, so I can see what changes need 
still more work.)

Regards,
BALATON Zoltan