[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20090626071524.GD14078@elte.hu>
Date: Fri, 26 Jun 2009 09:15:24 +0200
From: Ingo Molnar <mingo@...e.hu>
To: "Pan, Jacob jun" <jacob.jun.pan@...el.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"H. Peter Anvin" <hpa@...ux.intel.com>
Subject: Re: [PATCH 6/9] x86: add moorestown specific setup code
* Pan, Jacob jun <jacob.jun.pan@...el.com> wrote:
> >From f648870658331b0337a881a405857dfc0baee6b8 Mon Sep 17 00:00:00 2001
> From: Jacob Pan <jacob.jun.pan@...el.com>
> Date: Thu, 11 Jun 2009 21:01:00 -0700
> Subject: [PATCH] x86: add moorestown specific setup code
>
> Intel moorestown platform does not contain many devices found on a PC.
> This patch adds x86_quirks and assigns platform feature sets for
> Moorestown.
>
> Signed-off-by: Jacob Pan <jacob.jun.pan@...el.com>
> ---
> arch/x86/include/asm/setup.h | 8 +++
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/mrst.c | 105 ++++++++++++++++++++++++++++++++++++++++++
> arch/x86/kernel/setup.c | 4 +-
> 4 files changed, 117 insertions(+), 1 deletions(-)
> create mode 100644 arch/x86/kernel/mrst.c
this patch looks cleaner than the others (it reuses the existing
quirks mechanism and stays out of the way), but there's still a few
details:
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index 7bf325a..8253b71 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -83,6 +83,14 @@ static inline int is_visws_box(void) { return 0; }
> extern struct x86_quirks *x86_quirks;
> extern unsigned long saved_video_mode;
>
> +#ifdef CONFIG_MRST
> +extern void mrst_early_detect(int);
> +extern void setup_mrst_default_feature(void);
> +#else
> +static inline void mrst_early_detect(int subarch) { };
> +static inline void setup_mrst_default_feature(void) { };
> +#endif
stray semicolon.
> +
> #ifndef CONFIG_PARAVIRT
> #define paravirt_post_allocator_init() do {} while (0)
> #endif
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 8d1ae61..370aba5 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -103,6 +103,7 @@ obj-$(CONFIG_SCx200) += scx200.o
> scx200-y += scx200_32.o
>
> obj-$(CONFIG_OLPC) += olpc.o
> +obj-$(CONFIG_MRST) += mrst.o
>
> microcode-y := microcode_core.o
> microcode-$(CONFIG_MICROCODE_INTEL) += microcode_intel.o
> diff --git a/arch/x86/kernel/mrst.c b/arch/x86/kernel/mrst.c
> new file mode 100644
> index 0000000..56ed345
> --- /dev/null
> +++ b/arch/x86/kernel/mrst.c
> @@ -0,0 +1,105 @@
> +/*
> + * mrst.c: Intel Moorestown platform specific setup code
> + *
> + * (C) Copyright 2008 Intel Corporation
> + * Author: Jacob Pan (jacob.jun.pan@...el.com)
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; version 2
> + * of the License.
> + *
> + * Note:
> + *
> + */
> +#include <linux/spi/spi.h>
> +#include <linux/sfi.h>
> +
> +#include <asm/platform_feature.h>
> +#include <asm/apb_timer.h>
> +#include <asm/setup.h>
> +#include <asm/apic.h>
> +
> +static int __init mrst_pre_intr_init(void)
> +{
> +#ifdef CONFIG_X86_IO_APIC
> + if (platform_has(X86_PLATFORM_FEATURE_APBT)) {
> + pre_init_apic_IRQ();
> + return 1;
> + }
> +#endif
> + return 0;
> +}
> +
> +static struct x86_quirks mrst_x86_quirks = {
> + .arch_pre_intr_init = mrst_pre_intr_init,
> + .setup_secondary_clock = apbt_setup_secondary_clock,
> +};
> +
> +inline void setup_mrst_default_feature(void)
> +{
> + clear_all_platform_feature();
> + platform_feature_set_flag(X86_PLATFORM_FEATURE_SFI);
> + platform_feature_set_flag(X86_PLATFORM_FEATURE_IOAPIC);
> + platform_feature_set_flag(X86_PLATFORM_FEATURE_APBT);
> + platform_feature_set_flag(X86_PLATFORM_FEATURE_VRTC);
> +}
> +
> +inline void mrst_early_detect(int subarch)
> +{
> + if (subarch == X86_SUBARCH_MRST) {
> + x86_quirks = &mrst_x86_quirks;
> + }
unnecessary curly braces.
> +}
> +
> +static int __init sfi_parse_spib(struct sfi_table_header *table)
> +{
> + struct sfi_table *sb;
> + struct sfi_spi_table_entry *pentry;
> + struct spi_board_info *info;
> + int num, i, j;
> +
> + sb = (struct sfi_table *)table;
> + num = SFI_GET_ENTRY_NUM(sb, sfi_spi_table_entry);
> + pentry = (struct sfi_spi_table_entry *) sb->pentry;
> +
> + info = kzalloc(num * sizeof(*info), GFP_KERNEL);
> + if (!info) {
> + pr_info("%s(): Error in kzalloc\n", __func__);
> + return -ENOMEM;
> + }
> +
> + for (i = 0, j = 0; i < num; i++) {
> + if (pentry->host_num != 0)
> + continue;
> + strncpy(info[j].modalias, pentry->name, 16);
> + info[j].irq = pentry->irq_info;
> + info[j].bus_num = pentry->host_num;
> + info[j].chip_select = pentry->cs;
> + info[j].max_speed_hz = 3125000; /* hard coded */
> +
> + printk(KERN_INFO "info[%d]: name = %16s, irq = 0x%x, bus = %d,"
> + " cs = %d\n", j, info[j].modalias, info[j].irq,
> + info[j].bus_num, info[j].chip_select);
> + j++;
> + pentry++;
> + }
> + spi_register_board_info(info, j);
> + kfree(info);
> + return 0;
> +
> +}
> +
> +static int __init sfi_parse_i2cb(struct sfi_table_header *table)
> +{
> + return 0;
> +}
> +
> +static int __init mrst_platform_init(void)
> +{
> + sfi_table_parse(SFI_SIG_SPIB, sfi_parse_spib);
> + sfi_table_parse(SFI_SIG_I2CB, sfi_parse_i2cb);
> + return 0;
> +}
> +
> +postcore_initcall(mrst_platform_init);
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 1a5ef7b..9a7f2a5 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -626,7 +626,7 @@ early_param("elfcorehdr", setup_elfcorehdr);
>
> static struct x86_quirks default_x86_quirks __initdata;
>
> -struct x86_quirks *x86_quirks __initdata = &default_x86_quirks;
> +struct x86_quirks *x86_quirks = &default_x86_quirks;
there's no explanation in the changelog about this change. Which
non-PC quirk will now be used runtime?
>
> #ifdef CONFIG_X86_RESERVE_LOW_64K
> static int __init dmi_low_memory_corruption(const struct dmi_system_id *d)
> @@ -681,6 +681,7 @@ void __init setup_arch(char **cmdline_p)
> #ifdef CONFIG_X86_32
> memcpy(&boot_cpu_data, &new_cpu_data, sizeof(new_cpu_data));
> visws_early_detect();
> + mrst_early_detect(boot_params.hdr.hardware_subarch);
> #else
> printk(KERN_INFO "Command line: %s\n", boot_command_line);
> #endif
instead of piling another early-detect call here please introduce a
helper inline that lists all the early detect calls.
Also, why does mrst_early_detect() need a parameter? boot_params is
a global variable ...
> @@ -732,6 +733,7 @@ void __init setup_arch(char **cmdline_p)
> #endif
>
> ARCH_SETUP
> +
> platform_feature_init_default(boot_params.hdr.hardware_subarch);
> setup_memory_map();
> parse_setup_data();
stray newline.
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists