Ingo Molnar wrote: > * Yinghai Lu <[email protected]> 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 -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
