Messages in this thread | | | Date | Mon, 02 Mar 2009 12:46:23 -0800 | From | Yinghai Lu <> | Subject | Re: [PATCH] x86: ioremap mptable -v2 |
| |
Ingo Molnar wrote: > * Yinghai Lu <yinghai@kernel.org> 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
| |