Re: [coreboot] [PATCH] x86: ioremap mptable -v2

[Yinghai Lu]

Ingo Molnar wrote:
> Yinghai Lu  wrote:
> > V3: according to Ingo, seperate get_mpc_size()
>
> No, that was not my suggestion. My suggestion was to separate 
> this whole 'else if' branch:
> > } else if (mpf->physptr) {
> > struct mpc_table *mpc;
> > unsigned long size;
> >
> > size = get_mpc_size(mpf->physptr);
> > mpc = early_ioremap(mpf->physptr, size);
> >
> > /*
> > Read the physical hardware table.  Anything here will
> > override the defaults.
> >
> > */
> > if (!smp_read_mpc(phys_to_virt(mpf->physptr), early)) {
> > if (!smp_read_mpc(mpc, early)) {
> >
> > #ifdef CONFIG_X86_LOCAL_APIC
> >    		smp_found_config = 0;
> >  #endif
>
> ... into a helper function - if that improves the code.

oh, i missed it
> Your patch does early_ioremap, iounmap then ioremap and iounmap - 
> quite pointlessly.

try to get exact mpc size.
> You should resist cleanup suggestions that make the code worse, 
> even if it comes from a maintainer :-)

we could do that later. to make __get_smp_config smaller and readable.

YH