Skip to contentSkip to conversation

Add Clovertrail and Merrifeld support to Intel MID

PATCH In review David Cohen 5 participants 36 messages Dec 16, 2013 – Jan 28, 2014

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover Add Clovertrail and Merrifeld support to Intel MID

Hi,

I've a bunch of Intel MID patches under review but it seems they are becoming old and start to need changes.
I gathered an up-to-date version of all of them in this single patch set.

This series implements support of Clovertrail and Merrifield to Intel MID. It also removes the unwanted X86_MDFLD option since *all* Intel MID platforms should be supported through CONFIG_X86_INTEL_MID option.

This makes obsolete the following patches:
https://lkml.org/lkml/2013/10/18/480
https://lkml.org/lkml/2013/10/22/533
http://lkml.indiana.edu/hypermail/linux/kernel/1311.1/02937.html

Br, David

---
David Cohen (3):
x86: intel-mid: move Medfield code out of intel-mid.c core file
x86: intel-mid: add Merrifield platform support
x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID
configs

Kuppuswamy Sathyanarayanan (1):
x86: intel-mid: add Clovertrail platform support

 arch/x86/Kconfig                                   |  29 +-----
 arch/x86/include/asm/intel-mid.h                   |  48 +++++++++-
 arch/x86/pci/intel_mid_pci.c                       |   6 +-
 arch/x86/platform/intel-mid/Makefile               |   4 +-
 arch/x86/platform/intel-mid/intel-mid.c            |  64 +++++++------
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h |  19 ++++
 arch/x86/platform/intel-mid/mfld.c                 |  75 +++++++++++++++
 arch/x86/platform/intel-mid/mrfl.c                 | 103 +++++++++++++++++++++
 arch/x86/platform/intel-mid/sfi.c                  |  34 +++++--
 9 files changed, 317 insertions(+), 65 deletions(-)
 create mode 100644 arch/x86/platform/intel-mid/intel_mid_weak_decls.h
 create mode 100644 arch/x86/platform/intel-mid/mfld.c
 create mode 100644 arch/x86/platform/intel-mid/mrfl.c
Signature · 1 line
1.8.4.2
#1 lore raw

PATCH v2 1/4 x86: intel-mid: move Medfield code out of intel-mid.c core file

In order make the driver more portable and support other Intel Mid platforms we need to move Medfield code from intel-mid.c core to its own mfld.c file.

This patch does no functional change.

---
 arch/x86/platform/intel-mid/Makefile               |  4 +-
 arch/x86/platform/intel-mid/intel-mid.c            | 37 +---------------
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 15 +++++++
 arch/x86/platform/intel-mid/mfld.c                 | 51 ++++++++++++++++++++++
 4 files changed, 70 insertions(+), 37 deletions(-)
 create mode 100644 arch/x86/platform/intel-mid/intel_mid_weak_decls.h
 create mode 100644 arch/x86/platform/intel-mid/mfld.c
arch/x86/platform/intel-mid/Makefile +2 -2
index 01cc29ea5ff7..78a14ba0e0db 100644
@@ -1,6 +1,6 @@
-obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o
-obj-$(CONFIG_X86_INTEL_MID) += intel_mid_vrtc.o
+obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o intel_mid_vrtc.o mfld.o
 obj-$(CONFIG_EARLY_PRINTK_INTEL_MID) += early_printk_intel_mid.o
+
 # SFI specific code
 ifdef CONFIG_X86_INTEL_MID
 obj-$(CONFIG_SFI) += sfi.o device_libs/
arch/x86/platform/intel-mid/intel-mid.c +2 -35
index f90e290f689f..527d6d50643d 100644
@@ -35,6 +35,8 @@
 #include <asm/apb_timer.h>
 #include <asm/reboot.h>
 
+#include "intel_mid_weak_decls.h"
+
 /*
  * the clockevent devices on Moorestown/Medfield can be APBT or LAPIC clock,
  * cmdline option x86_intel_mid_timer can be used to override the configuration
@@ -61,46 +63,11 @@ enum intel_mid_timer_options intel_mid_timer_options;
 enum intel_mid_cpu_type __intel_mid_cpu_chip;
 EXPORT_SYMBOL_GPL(__intel_mid_cpu_chip);
 
-static void intel_mid_power_off(void)
-{
-}
-
 static void intel_mid_reboot(void)
 {
 	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
 }
 
-static unsigned long __init intel_mid_calibrate_tsc(void)
-{
-	unsigned long fast_calibrate;
-	u32 lo, hi, ratio, fsb;
-
-	rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
-	pr_debug("IA32 perf status is 0x%x, 0x%0x\n", lo, hi);
-	ratio = (hi >> 8) & 0x1f;
-	pr_debug("ratio is %d\n", ratio);
-	if (!ratio) {
-		pr_err("read a zero ratio, should be incorrect!\n");
-		pr_err("force tsc ratio to 16 ...\n");
-		ratio = 16;
-	}
-	rdmsr(MSR_FSB_FREQ, lo, hi);
-	if ((lo & 0x7) == 0x7)
-		fsb = PENWELL_FSB_FREQ_83SKU;
-	else
-		fsb = PENWELL_FSB_FREQ_100SKU;
-	fast_calibrate = ratio * fsb;
-	pr_debug("read penwell tsc %lu khz\n", fast_calibrate);
-	lapic_timer_frequency = fsb * 1000 / HZ;
-	/* mark tsc clocksource as reliable */
-	set_cpu_cap(&boot_cpu_data, X86_FEATURE_TSC_RELIABLE);
-
-	if (fast_calibrate)
-		return fast_calibrate;
-
-	return 0;
-}
-
 static void __init intel_mid_time_init(void)
 {
 	sfi_table_parse(SFI_SIG_MTMR, NULL, NULL, sfi_parse_mtmr);
arch/x86/platform/intel-mid/intel_mid_weak_decls.h new file +15 -0
new file mode 100644
index 000000000000..519beb75ba4c
@@ -0,0 +1,15 @@
+/*
+ * intel_mid_weak_decls.h: Weak declarations of intel-mid.c
+ *
+ * (C) Copyright 2013 Intel Corporation
+ *
+ * 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.
+ */
+
+
+/* __attribute__((weak)) makes these declarations overridable */
+extern void intel_mid_power_off(void) __attribute__((weak));
+extern unsigned long __init intel_mid_calibrate_tsc(void) __attribute__((weak));
arch/x86/platform/intel-mid/mfld.c new file +51 -0

51 lines not on this page · raw message

Signature · 1 line
1.8.4.2
#2 lore raw

[tip:x86/intel-mid] x86, intel-mid: Move Medfield code out of intel-mid.c core file

x86, intel-mid: Move Medfield code out of intel-mid.c core file
Commit-ID:  ecd6910db979bc40ac19f0e71e027132fc906068
Gitweb:     http://git.kernel.org/tip/ecd6910db979bc40ac19f0e71e027132fc906068
Author:     David Cohen <[email protected]>
AuthorDate: Mon, 16 Dec 2013 12:07:36 -0800
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 15 Jan 2014 14:38:58 -0800

x86, intel-mid: Move Medfield code out of intel-mid.c core file

In order make the driver more portable and support other Intel MID (Mobile Internet Device) platforms we need to move Medfield code from intel-mid.c core to its own mfld.c file.

This patch contains no functional changes.

---
 arch/x86/platform/intel-mid/Makefile               |  4 +-
 arch/x86/platform/intel-mid/intel-mid.c            | 37 +---------------
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h | 15 +++++++
 arch/x86/platform/intel-mid/mfld.c                 | 51 ++++++++++++++++++++++
 4 files changed, 70 insertions(+), 37 deletions(-)
arch/x86/platform/intel-mid/Makefile +2 -2
index 01cc29e..78a14ba 100644
@@ -1,6 +1,6 @@
-obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o
-obj-$(CONFIG_X86_INTEL_MID) += intel_mid_vrtc.o
+obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o intel_mid_vrtc.o mfld.o
 obj-$(CONFIG_EARLY_PRINTK_INTEL_MID) += early_printk_intel_mid.o
+
 # SFI specific code
 ifdef CONFIG_X86_INTEL_MID
 obj-$(CONFIG_SFI) += sfi.o device_libs/
arch/x86/platform/intel-mid/intel-mid.c +2 -35
index f90e290..527d6d5 100644
@@ -35,6 +35,8 @@
 #include <asm/apb_timer.h>
 #include <asm/reboot.h>
 
+#include "intel_mid_weak_decls.h"
+
 /*
  * the clockevent devices on Moorestown/Medfield can be APBT or LAPIC clock,
  * cmdline option x86_intel_mid_timer can be used to override the configuration
@@ -61,46 +63,11 @@ enum intel_mid_timer_options intel_mid_timer_options;
 enum intel_mid_cpu_type __intel_mid_cpu_chip;
 EXPORT_SYMBOL_GPL(__intel_mid_cpu_chip);
 
-static void intel_mid_power_off(void)
-{
-}
-
 static void intel_mid_reboot(void)
 {
 	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
 }
 
-static unsigned long __init intel_mid_calibrate_tsc(void)
-{
-	unsigned long fast_calibrate;
-	u32 lo, hi, ratio, fsb;
-
-	rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
-	pr_debug("IA32 perf status is 0x%x, 0x%0x\n", lo, hi);
-	ratio = (hi >> 8) & 0x1f;
-	pr_debug("ratio is %d\n", ratio);
-	if (!ratio) {
-		pr_err("read a zero ratio, should be incorrect!\n");
-		pr_err("force tsc ratio to 16 ...\n");
-		ratio = 16;
-	}
-	rdmsr(MSR_FSB_FREQ, lo, hi);
-	if ((lo & 0x7) == 0x7)
-		fsb = PENWELL_FSB_FREQ_83SKU;
-	else
-		fsb = PENWELL_FSB_FREQ_100SKU;
-	fast_calibrate = ratio * fsb;
-	pr_debug("read penwell tsc %lu khz\n", fast_calibrate);
-	lapic_timer_frequency = fsb * 1000 / HZ;
-	/* mark tsc clocksource as reliable */
-	set_cpu_cap(&boot_cpu_data, X86_FEATURE_TSC_RELIABLE);
-
-	if (fast_calibrate)
-		return fast_calibrate;
-
-	return 0;
-}
-
 static void __init intel_mid_time_init(void)
 {
 	sfi_table_parse(SFI_SIG_MTMR, NULL, NULL, sfi_parse_mtmr);
arch/x86/platform/intel-mid/intel_mid_weak_decls.h new file +15 -0
new file mode 100644
index 0000000..519beb7
@@ -0,0 +1,15 @@
+/*
+ * intel_mid_weak_decls.h: Weak declarations of intel-mid.c
+ *
+ * (C) Copyright 2013 Intel Corporation
+ *
+ * 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.
+ */
+
+
+/* __attribute__((weak)) makes these declarations overridable */
+extern void intel_mid_power_off(void) __attribute__((weak));
+extern unsigned long __init intel_mid_calibrate_tsc(void) __attribute__((weak));
arch/x86/platform/intel-mid/mfld.c new file +51 -0

51 lines not on this page · raw message

#3 lore raw

PATCH v2 2/4 x86: intel-mid: add Clovertrail platform support

In reply to David Cohen

From: Kuppuswamy Sathyanarayanan <[email protected]>

This patch adds Clovertrail support on intel-mid and makes it more flexible to support other SoCs.

---
 arch/x86/include/asm/intel-mid.h                   | 46 +++++++++++++++++++++-
 arch/x86/platform/intel-mid/intel-mid.c            | 39 ++++++++++++++++--
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h |  7 +++-
 arch/x86/platform/intel-mid/mfld.c                 | 32 +++++++++++++--
 4 files changed, 113 insertions(+), 11 deletions(-)
arch/x86/include/asm/intel-mid.h +44 -2
index 459769d39263..f8a831431fe0 100644
@@ -51,10 +51,39 @@ struct devs_id {
 enum intel_mid_cpu_type {
 	/* 1 was Moorestown */
 	INTEL_MID_CPU_CHIP_PENWELL = 2,
+	INTEL_MID_CPU_CHIP_CLOVERVIEW,
 };
 
 extern enum intel_mid_cpu_type __intel_mid_cpu_chip;
 
+/**
+ * struct intel_mid_ops - Interface between intel-mid & sub archs
+ * @arch_setup: arch_setup function to re-initialize platform
+ *             structures (x86_init, x86_platform_init)
+ *
+ * This structure can be extended if any new interface is required
+ * between intel-mid & its sub arch files.
+ */
+struct intel_mid_ops {
+	void (*arch_setup)(void);
+};
+
+/* Helper API's for INTEL_MID_OPS_INIT */
+#define DECLARE_INTEL_MID_OPS_INIT(cpuname, cpuid)	\
+				[cpuid] = get_##cpuname##_ops
+
+/* Maximum number of CPU ops */
+#define MAX_CPU_OPS(a) (sizeof(a)/sizeof(void *))
+
+/*
+ * For every new cpu addition, a weak get_<cpuname>_ops() function needs be
+ * declared in arch/x86/platform/intel_mid/intel_mid_weak_decls.h.
+ */
+#define INTEL_MID_OPS_INIT {\
+	DECLARE_INTEL_MID_OPS_INIT(penwell, INTEL_MID_CPU_CHIP_PENWELL), \
+	DECLARE_INTEL_MID_OPS_INIT(cloverview, INTEL_MID_CPU_CHIP_CLOVERVIEW), \
+};
+
 #ifdef CONFIG_X86_INTEL_MID
 
 static inline enum intel_mid_cpu_type intel_mid_identify_cpu(void)
@@ -86,8 +115,21 @@ extern enum intel_mid_timer_options intel_mid_timer_options;
  * Penwell uses spread spectrum clock, so the freq number is not exactly
  * the same as reported by MSR based on SDM.
  */
-#define PENWELL_FSB_FREQ_83SKU         83200
-#define PENWELL_FSB_FREQ_100SKU        99840
+#define FSB_FREQ_83SKU	83200
+#define FSB_FREQ_100SKU	99840
+#define FSB_FREQ_133SKU	133000
+
+#define FSB_FREQ_167SKU	167000
+#define FSB_FREQ_200SKU	200000
+#define FSB_FREQ_267SKU	267000
+#define FSB_FREQ_333SKU	333000
+#define FSB_FREQ_400SKU	400000
+
+/* Bus Select SoC Fuse value */
+#define BSEL_SOC_FUSE_MASK	0x7
+#define BSEL_SOC_FUSE_001	0x1 /* FSB 133MHz */
+#define BSEL_SOC_FUSE_101	0x5 /* FSB 100MHz */
+#define BSEL_SOC_FUSE_111	0x7 /* FSB 83MHz */
 
 #define SFI_MTMR_MAX_NUM 8
 #define SFI_MRTC_MAX	8
arch/x86/platform/intel-mid/intel-mid.c +36 -3
index 527d6d50643d..40955841bb32 100644
@@ -60,14 +60,27 @@
 
 enum intel_mid_timer_options intel_mid_timer_options;
 
+/* intel_mid_ops to store sub arch ops */
+struct intel_mid_ops *intel_mid_ops;
+/* getter function for sub arch ops*/
+static void *(*get_intel_mid_ops[])(void) = INTEL_MID_OPS_INIT;
 enum intel_mid_cpu_type __intel_mid_cpu_chip;
 EXPORT_SYMBOL_GPL(__intel_mid_cpu_chip);
 
+static void intel_mid_power_off(void)
+{
+};
+
 static void intel_mid_reboot(void)
 {
 	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
 }
 
+static unsigned long __init intel_mid_calibrate_tsc(void)
+{
+	return 0;
+}
+
 static void __init intel_mid_time_init(void)
 {
 	sfi_table_parse(SFI_SIG_MTMR, NULL, NULL, sfi_parse_mtmr);
@@ -92,13 +105,33 @@ static void __init intel_mid_time_init(void)
 
 static void intel_mid_arch_setup(void)
 {
-	if (boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x27)
-		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
-	else {
+	if (boot_cpu_data.x86 != 6) {
 		pr_err("Unknown Intel MID CPU (%d:%d), default to Penwell\n",
 			boot_cpu_data.x86, boot_cpu_data.x86_model);
 		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
+		goto out;
+	}
+
+	switch (boot_cpu_data.x86_model) {
+	case 0x35:
+		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_CLOVERVIEW;
+		break;
+	case 0x27:
+	default:
+		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
+		break;
 	}
+
+	if (__intel_mid_cpu_chip < MAX_CPU_OPS(get_intel_mid_ops))
+		intel_mid_ops = get_intel_mid_ops[__intel_mid_cpu_chip]();
+	else {
+		intel_mid_ops = get_intel_mid_ops[INTEL_MID_CPU_CHIP_PENWELL]();
+		pr_info("ARCH: Uknown SoC, assuming PENWELL!\n");
+	}
+
+out:
+	if (intel_mid_ops->arch_setup)
+		intel_mid_ops->arch_setup();
 }
 
 /* MID systems don't have i8042 controller */
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +5 -2
index 519beb75ba4c..9ebce0447edf 100644
@@ -11,5 +11,8 @@
 
 
 /* __attribute__((weak)) makes these declarations overridable */
-extern void intel_mid_power_off(void) __attribute__((weak));
-extern unsigned long __init intel_mid_calibrate_tsc(void) __attribute__((weak));
+/* For every CPU addition a new get_<cpuname>_ops interface needs
+ * to be added.
+ */
+extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
+extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
arch/x86/platform/intel-mid/mfld.c +28 -4
index c7ff83c4576d..4f7884eebc14 100644
@@ -15,11 +15,19 @@
 #include <asm/intel-mid.h>
 #include <asm/intel_mid_vrtc.h>
 
-void intel_mid_power_off(void)
+#include "intel_mid_weak_decls.h"
+
+static void penwell_arch_setup(void);
+/* penwell arch ops */
+static struct intel_mid_ops penwell_ops = {
+	.arch_setup = penwell_arch_setup,
+};
+
+static void mfld_power_off(void)
 {
 }
 
-unsigned long __init intel_mid_calibrate_tsc(void)
+static unsigned long __init mfld_calibrate_tsc(void)
 {
 	unsigned long fast_calibrate;
 	u32 lo, hi, ratio, fsb;
@@ -35,9 +43,9 @@ unsigned long __init intel_mid_calibrate_tsc(void)
 	}
 	rdmsr(MSR_FSB_FREQ, lo, hi);
 	if ((lo & 0x7) == 0x7)
-		fsb = PENWELL_FSB_FREQ_83SKU;
+		fsb = FSB_FREQ_83SKU;
 	else
-		fsb = PENWELL_FSB_FREQ_100SKU;
+		fsb = FSB_FREQ_100SKU;
 	fast_calibrate = ratio * fsb;
 	pr_debug("read penwell tsc %lu khz\n", fast_calibrate);
 	lapic_timer_frequency = fsb * 1000 / HZ;
@@ -49,3 +57,19 @@ unsigned long __init intel_mid_calibrate_tsc(void)
 
 	return 0;
 }
+
+static void __init penwell_arch_setup()
+{
+	x86_platform.calibrate_tsc = mfld_calibrate_tsc;
+	pm_power_off = mfld_power_off;
+}
+
+void * __cpuinit get_penwell_ops()
+{
+	return &penwell_ops;
+}
+
+void * __cpuinit get_cloverview_ops()
+{
+	return &penwell_ops;
+}
Signature · 1 line
1.8.4.2
#4 lore raw

[tip:x86/intel-mid] x86, intel-mid: Add Clovertrail platform support

x86, intel-mid: Add Clovertrail platform support
Commit-ID:  85611e3febe78955a519f5f9eb47b941525c8c76
Gitweb:     http://git.kernel.org/tip/85611e3febe78955a519f5f9eb47b941525c8c76
Author:     Kuppuswamy Sathyanarayanan <[email protected]>
AuthorDate: Mon, 16 Dec 2013 12:07:37 -0800
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 15 Jan 2014 14:38:58 -0800

x86, intel-mid: Add Clovertrail platform support

This patch adds Clovertrail support on intel-mid and makes it more flexible to support other SoCs.

---
 arch/x86/include/asm/intel-mid.h                   | 46 +++++++++++++++++++++-
 arch/x86/platform/intel-mid/intel-mid.c            | 39 ++++++++++++++++--
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h |  7 +++-
 arch/x86/platform/intel-mid/mfld.c                 | 32 +++++++++++++--
 4 files changed, 113 insertions(+), 11 deletions(-)
arch/x86/include/asm/intel-mid.h +44 -2
index 459769d..f8a8314 100644
@@ -51,10 +51,39 @@ struct devs_id {
 enum intel_mid_cpu_type {
 	/* 1 was Moorestown */
 	INTEL_MID_CPU_CHIP_PENWELL = 2,
+	INTEL_MID_CPU_CHIP_CLOVERVIEW,
 };
 
 extern enum intel_mid_cpu_type __intel_mid_cpu_chip;
 
+/**
+ * struct intel_mid_ops - Interface between intel-mid & sub archs
+ * @arch_setup: arch_setup function to re-initialize platform
+ *             structures (x86_init, x86_platform_init)
+ *
+ * This structure can be extended if any new interface is required
+ * between intel-mid & its sub arch files.
+ */
+struct intel_mid_ops {
+	void (*arch_setup)(void);
+};
+
+/* Helper API's for INTEL_MID_OPS_INIT */
+#define DECLARE_INTEL_MID_OPS_INIT(cpuname, cpuid)	\
+				[cpuid] = get_##cpuname##_ops
+
+/* Maximum number of CPU ops */
+#define MAX_CPU_OPS(a) (sizeof(a)/sizeof(void *))
+
+/*
+ * For every new cpu addition, a weak get_<cpuname>_ops() function needs be
+ * declared in arch/x86/platform/intel_mid/intel_mid_weak_decls.h.
+ */
+#define INTEL_MID_OPS_INIT {\
+	DECLARE_INTEL_MID_OPS_INIT(penwell, INTEL_MID_CPU_CHIP_PENWELL), \
+	DECLARE_INTEL_MID_OPS_INIT(cloverview, INTEL_MID_CPU_CHIP_CLOVERVIEW), \
+};
+
 #ifdef CONFIG_X86_INTEL_MID
 
 static inline enum intel_mid_cpu_type intel_mid_identify_cpu(void)
@@ -86,8 +115,21 @@ extern enum intel_mid_timer_options intel_mid_timer_options;
  * Penwell uses spread spectrum clock, so the freq number is not exactly
  * the same as reported by MSR based on SDM.
  */
-#define PENWELL_FSB_FREQ_83SKU         83200
-#define PENWELL_FSB_FREQ_100SKU        99840
+#define FSB_FREQ_83SKU	83200
+#define FSB_FREQ_100SKU	99840
+#define FSB_FREQ_133SKU	133000
+
+#define FSB_FREQ_167SKU	167000
+#define FSB_FREQ_200SKU	200000
+#define FSB_FREQ_267SKU	267000
+#define FSB_FREQ_333SKU	333000
+#define FSB_FREQ_400SKU	400000
+
+/* Bus Select SoC Fuse value */
+#define BSEL_SOC_FUSE_MASK	0x7
+#define BSEL_SOC_FUSE_001	0x1 /* FSB 133MHz */
+#define BSEL_SOC_FUSE_101	0x5 /* FSB 100MHz */
+#define BSEL_SOC_FUSE_111	0x7 /* FSB 83MHz */
 
 #define SFI_MTMR_MAX_NUM 8
 #define SFI_MRTC_MAX	8
arch/x86/platform/intel-mid/intel-mid.c +36 -3
index 527d6d5..4095584 100644
@@ -60,14 +60,27 @@
 
 enum intel_mid_timer_options intel_mid_timer_options;
 
+/* intel_mid_ops to store sub arch ops */
+struct intel_mid_ops *intel_mid_ops;
+/* getter function for sub arch ops*/
+static void *(*get_intel_mid_ops[])(void) = INTEL_MID_OPS_INIT;
 enum intel_mid_cpu_type __intel_mid_cpu_chip;
 EXPORT_SYMBOL_GPL(__intel_mid_cpu_chip);
 
+static void intel_mid_power_off(void)
+{
+};
+
 static void intel_mid_reboot(void)
 {
 	intel_scu_ipc_simple_command(IPCMSG_COLD_BOOT, 0);
 }
 
+static unsigned long __init intel_mid_calibrate_tsc(void)
+{
+	return 0;
+}
+
 static void __init intel_mid_time_init(void)
 {
 	sfi_table_parse(SFI_SIG_MTMR, NULL, NULL, sfi_parse_mtmr);
@@ -92,13 +105,33 @@ static void __init intel_mid_time_init(void)
 
 static void intel_mid_arch_setup(void)
 {
-	if (boot_cpu_data.x86 == 6 && boot_cpu_data.x86_model == 0x27)
-		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
-	else {
+	if (boot_cpu_data.x86 != 6) {
 		pr_err("Unknown Intel MID CPU (%d:%d), default to Penwell\n",
 			boot_cpu_data.x86, boot_cpu_data.x86_model);
 		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
+		goto out;
+	}
+
+	switch (boot_cpu_data.x86_model) {
+	case 0x35:
+		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_CLOVERVIEW;
+		break;
+	case 0x27:
+	default:
+		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
+		break;
 	}
+
+	if (__intel_mid_cpu_chip < MAX_CPU_OPS(get_intel_mid_ops))
+		intel_mid_ops = get_intel_mid_ops[__intel_mid_cpu_chip]();
+	else {
+		intel_mid_ops = get_intel_mid_ops[INTEL_MID_CPU_CHIP_PENWELL]();
+		pr_info("ARCH: Uknown SoC, assuming PENWELL!\n");
+	}
+
+out:
+	if (intel_mid_ops->arch_setup)
+		intel_mid_ops->arch_setup();
 }
 
 /* MID systems don't have i8042 controller */
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +5 -2
index 519beb7..9ebce04 100644
@@ -11,5 +11,8 @@
 
 
 /* __attribute__((weak)) makes these declarations overridable */
-extern void intel_mid_power_off(void) __attribute__((weak));
-extern unsigned long __init intel_mid_calibrate_tsc(void) __attribute__((weak));
+/* For every CPU addition a new get_<cpuname>_ops interface needs
+ * to be added.
+ */
+extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
+extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
arch/x86/platform/intel-mid/mfld.c +28 -4
index c7ff83c..4f7884e 100644
@@ -15,11 +15,19 @@
 #include <asm/intel-mid.h>
 #include <asm/intel_mid_vrtc.h>
 
-void intel_mid_power_off(void)
+#include "intel_mid_weak_decls.h"
+
+static void penwell_arch_setup(void);
+/* penwell arch ops */
+static struct intel_mid_ops penwell_ops = {
+	.arch_setup = penwell_arch_setup,
+};
+
+static void mfld_power_off(void)
 {
 }
 
-unsigned long __init intel_mid_calibrate_tsc(void)
+static unsigned long __init mfld_calibrate_tsc(void)
 {
 	unsigned long fast_calibrate;
 	u32 lo, hi, ratio, fsb;
@@ -35,9 +43,9 @@ unsigned long __init intel_mid_calibrate_tsc(void)
 	}
 	rdmsr(MSR_FSB_FREQ, lo, hi);
 	if ((lo & 0x7) == 0x7)
-		fsb = PENWELL_FSB_FREQ_83SKU;
+		fsb = FSB_FREQ_83SKU;
 	else
-		fsb = PENWELL_FSB_FREQ_100SKU;
+		fsb = FSB_FREQ_100SKU;
 	fast_calibrate = ratio * fsb;
 	pr_debug("read penwell tsc %lu khz\n", fast_calibrate);
 	lapic_timer_frequency = fsb * 1000 / HZ;
@@ -49,3 +57,19 @@ unsigned long __init intel_mid_calibrate_tsc(void)
 
 	return 0;
 }
+
+static void __init penwell_arch_setup()
+{
+	x86_platform.calibrate_tsc = mfld_calibrate_tsc;
+	pm_power_off = mfld_power_off;
+}
+
+void * __cpuinit get_penwell_ops()
+{
+	return &penwell_ops;
+}
+
+void * __cpuinit get_cloverview_ops()
+{
+	return &penwell_ops;
+}
#5 lore raw

PATCH v2 3/4 x86: intel-mid: add Merrifield platform support

In reply to David Cohen

This code was partially based on Mark Brown's previous work.

Cc Mark F. Brown <[email protected]>
Cc Kuppuswamy Sathyanarayanan <[email protected]>
---
 arch/x86/include/asm/intel-mid.h                   |   2 +
 arch/x86/pci/intel_mid_pci.c                       |   6 +-
 arch/x86/platform/intel-mid/Makefile               |   2 +-
 arch/x86/platform/intel-mid/intel-mid.c            |   4 +
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h |   1 +
 arch/x86/platform/intel-mid/mrfl.c                 | 103 +++++++++++++++++++++
 arch/x86/platform/intel-mid/sfi.c                  |  34 +++++--
 7 files changed, 144 insertions(+), 8 deletions(-)
 create mode 100644 arch/x86/platform/intel-mid/mrfl.c
arch/x86/include/asm/intel-mid.h +2 -0
index f8a831431fe0..e34e097b6f9d 100644
@@ -52,6 +52,7 @@ enum intel_mid_cpu_type {
 	/* 1 was Moorestown */
 	INTEL_MID_CPU_CHIP_PENWELL = 2,
 	INTEL_MID_CPU_CHIP_CLOVERVIEW,
+	INTEL_MID_CPU_CHIP_TANGIER,
 };
 
 extern enum intel_mid_cpu_type __intel_mid_cpu_chip;
@@ -82,6 +83,7 @@ struct intel_mid_ops {
 #define INTEL_MID_OPS_INIT {\
 	DECLARE_INTEL_MID_OPS_INIT(penwell, INTEL_MID_CPU_CHIP_PENWELL), \
 	DECLARE_INTEL_MID_OPS_INIT(cloverview, INTEL_MID_CPU_CHIP_CLOVERVIEW), \
+	DECLARE_INTEL_MID_OPS_INIT(tangier, INTEL_MID_CPU_CHIP_TANGIER) \
 };
 
 #ifdef CONFIG_X86_INTEL_MID
arch/x86/pci/intel_mid_pci.c +5 -1
index 51384ca727ad..84b9d672843d 100644
@@ -31,6 +31,7 @@
 #include <asm/pci_x86.h>
 #include <asm/hw_irq.h>
 #include <asm/io_apic.h>
+#include <asm/intel-mid.h>
 
 #define PCIE_CAP_OFFSET	0x100
 
@@ -219,7 +220,10 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 	irq_attr.ioapic = mp_find_ioapic(dev->irq);
 	irq_attr.ioapic_pin = dev->irq;
 	irq_attr.trigger = 1; /* level */
-	irq_attr.polarity = 1; /* active low */
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER)
+		irq_attr.polarity = 0; /* active high */
+	else
+		irq_attr.polarity = 1; /* active low */
 	io_apic_set_pci_routing(&dev->dev, dev->irq, &irq_attr);
 
 	return 0;
arch/x86/platform/intel-mid/Makefile +1 -1
index 78a14ba0e0db..0a8ee703b9fa 100644
@@ -1,4 +1,4 @@
-obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o intel_mid_vrtc.o mfld.o
+obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o intel_mid_vrtc.o mfld.o mrfl.o
 obj-$(CONFIG_EARLY_PRINTK_INTEL_MID) += early_printk_intel_mid.o
 
 # SFI specific code
arch/x86/platform/intel-mid/intel-mid.c +4 -0
index 40955841bb32..1bbedc4b0f88 100644
@@ -116,6 +116,10 @@ static void intel_mid_arch_setup(void)
 	case 0x35:
 		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_CLOVERVIEW;
 		break;
+	case 0x3C:
+	case 0x4A:
+		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_TANGIER;
+		break;
 	case 0x27:
 	default:
 		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +1 -0
index 9ebce0447edf..a537ffc16299 100644
@@ -16,3 +16,4 @@
  */
 extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
 extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
+extern void * __init get_tangier_ops(void) __attribute__((weak));
arch/x86/platform/intel-mid/mrfl.c new file +103 -0

103 lines not on this page · raw message

arch/x86/platform/intel-mid/sfi.c +28 -6
index c84c1ca396bf..80a52288555c 100644
@@ -443,13 +443,35 @@ static int __init sfi_parse_devs(struct sfi_table_header *table)
 			 * so we have to enable them one by one here
 			 */
 			ioapic = mp_find_ioapic(irq);
-			irq_attr.ioapic = ioapic;
-			irq_attr.ioapic_pin = irq;
-			irq_attr.trigger = 1;
-			irq_attr.polarity = 1;
-			io_apic_set_pci_routing(NULL, irq, &irq_attr);
-		} else
+			if (ioapic >= 0) {
+				irq_attr.ioapic = ioapic;
+				irq_attr.ioapic_pin = irq;
+				irq_attr.trigger = 1;
+				if (intel_mid_identify_cpu() ==
+						INTEL_MID_CPU_CHIP_TANGIER) {
+					if (!strncmp(pentry->name,
+							"r69001-ts-i2c", 13))
+						/* active low */
+						irq_attr.polarity = 1;
+					else if (!strncmp(pentry->name,
+							"synaptics_3202", 14))
+						/* active low */
+						irq_attr.polarity = 1;
+					else if (irq == 41)
+						/* fast_int_1 */
+						irq_attr.polarity = 1;
+					else
+						/* active high */
+						irq_attr.polarity = 0;
+				} else {
+					/* PNW and CLV go with active low */
+					irq_attr.polarity = 1;
+				}
+				io_apic_set_pci_routing(NULL, irq, &irq_attr);
+			}
+		} else {
 			irq = 0; /* No irq */
+		}
 
 		dev = get_device_id(pentry->type, pentry->name);
 
Signature · 1 line
1.8.4.2
#6 lore raw

[tip:x86/intel-mid] x86, intel-mid: Add Merrifield platform support

x86, intel-mid: Add Merrifield platform support
Commit-ID:  bc20aa48bbb3068224a1c91f8332971fdb689fad
Gitweb:     http://git.kernel.org/tip/bc20aa48bbb3068224a1c91f8332971fdb689fad
Author:     David Cohen <[email protected]>
AuthorDate: Mon, 16 Dec 2013 12:07:38 -0800
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 15 Jan 2014 14:38:58 -0800

x86, intel-mid: Add Merrifield platform support

This code was partially based on Mark Brown's previous work.

---
 arch/x86/include/asm/intel-mid.h                   |   2 +
 arch/x86/pci/intel_mid_pci.c                       |   6 +-
 arch/x86/platform/intel-mid/Makefile               |   2 +-
 arch/x86/platform/intel-mid/intel-mid.c            |   4 +
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h |   1 +
 arch/x86/platform/intel-mid/mrfl.c                 | 103 +++++++++++++++++++++
 arch/x86/platform/intel-mid/sfi.c                  |  34 +++++--
 7 files changed, 144 insertions(+), 8 deletions(-)
arch/x86/include/asm/intel-mid.h +2 -0
index f8a8314..e34e097 100644
@@ -52,6 +52,7 @@ enum intel_mid_cpu_type {
 	/* 1 was Moorestown */
 	INTEL_MID_CPU_CHIP_PENWELL = 2,
 	INTEL_MID_CPU_CHIP_CLOVERVIEW,
+	INTEL_MID_CPU_CHIP_TANGIER,
 };
 
 extern enum intel_mid_cpu_type __intel_mid_cpu_chip;
@@ -82,6 +83,7 @@ struct intel_mid_ops {
 #define INTEL_MID_OPS_INIT {\
 	DECLARE_INTEL_MID_OPS_INIT(penwell, INTEL_MID_CPU_CHIP_PENWELL), \
 	DECLARE_INTEL_MID_OPS_INIT(cloverview, INTEL_MID_CPU_CHIP_CLOVERVIEW), \
+	DECLARE_INTEL_MID_OPS_INIT(tangier, INTEL_MID_CPU_CHIP_TANGIER) \
 };
 
 #ifdef CONFIG_X86_INTEL_MID
arch/x86/pci/intel_mid_pci.c +5 -1
index 51384ca..84b9d67 100644
@@ -31,6 +31,7 @@
 #include <asm/pci_x86.h>
 #include <asm/hw_irq.h>
 #include <asm/io_apic.h>
+#include <asm/intel-mid.h>
 
 #define PCIE_CAP_OFFSET	0x100
 
@@ -219,7 +220,10 @@ static int intel_mid_pci_irq_enable(struct pci_dev *dev)
 	irq_attr.ioapic = mp_find_ioapic(dev->irq);
 	irq_attr.ioapic_pin = dev->irq;
 	irq_attr.trigger = 1; /* level */
-	irq_attr.polarity = 1; /* active low */
+	if (intel_mid_identify_cpu() == INTEL_MID_CPU_CHIP_TANGIER)
+		irq_attr.polarity = 0; /* active high */
+	else
+		irq_attr.polarity = 1; /* active low */
 	io_apic_set_pci_routing(&dev->dev, dev->irq, &irq_attr);
 
 	return 0;
arch/x86/platform/intel-mid/Makefile +1 -1
index 78a14ba..0a8ee70 100644
@@ -1,4 +1,4 @@
-obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o intel_mid_vrtc.o mfld.o
+obj-$(CONFIG_X86_INTEL_MID) += intel-mid.o intel_mid_vrtc.o mfld.o mrfl.o
 obj-$(CONFIG_EARLY_PRINTK_INTEL_MID) += early_printk_intel_mid.o
 
 # SFI specific code
arch/x86/platform/intel-mid/intel-mid.c +4 -0
index 4095584..1bbedc4 100644
@@ -116,6 +116,10 @@ static void intel_mid_arch_setup(void)
 	case 0x35:
 		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_CLOVERVIEW;
 		break;
+	case 0x3C:
+	case 0x4A:
+		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_TANGIER;
+		break;
 	case 0x27:
 	default:
 		__intel_mid_cpu_chip = INTEL_MID_CPU_CHIP_PENWELL;
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +1 -0
index 9ebce04..a537ffc 100644
@@ -16,3 +16,4 @@
  */
 extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
 extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
+extern void * __init get_tangier_ops(void) __attribute__((weak));
arch/x86/platform/intel-mid/mrfl.c new file +103 -0

103 lines not on this page · raw message

arch/x86/platform/intel-mid/sfi.c +28 -6
index c84c1ca..80a5228 100644
@@ -443,13 +443,35 @@ static int __init sfi_parse_devs(struct sfi_table_header *table)
 			 * so we have to enable them one by one here
 			 */
 			ioapic = mp_find_ioapic(irq);
-			irq_attr.ioapic = ioapic;
-			irq_attr.ioapic_pin = irq;
-			irq_attr.trigger = 1;
-			irq_attr.polarity = 1;
-			io_apic_set_pci_routing(NULL, irq, &irq_attr);
-		} else
+			if (ioapic >= 0) {
+				irq_attr.ioapic = ioapic;
+				irq_attr.ioapic_pin = irq;
+				irq_attr.trigger = 1;
+				if (intel_mid_identify_cpu() ==
+						INTEL_MID_CPU_CHIP_TANGIER) {
+					if (!strncmp(pentry->name,
+							"r69001-ts-i2c", 13))
+						/* active low */
+						irq_attr.polarity = 1;
+					else if (!strncmp(pentry->name,
+							"synaptics_3202", 14))
+						/* active low */
+						irq_attr.polarity = 1;
+					else if (irq == 41)
+						/* fast_int_1 */
+						irq_attr.polarity = 1;
+					else
+						/* active high */
+						irq_attr.polarity = 0;
+				} else {
+					/* PNW and CLV go with active low */
+					irq_attr.polarity = 1;
+				}
+				io_apic_set_pci_routing(NULL, irq, &irq_attr);
+			}
+		} else {
 			irq = 0; /* No irq */
+		}
 
 		dev = get_device_id(pentry->type, pentry->name);
 
maintainer
#7 lore raw

Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support

In reply to David Cohen
Quoted 6 lines · David Cohen

This code was partially based on Mark Brown's previous work.

Cc Mark F. Brown <[email protected]>
Cc Kuppuswamy Sathyanarayanan <[email protected]>

I know this has already been merged to Linus' tree, but it looks funny to me.

arch/x86/platform/intel-mid/intel_mid_weak_decls.h line 19

Quoted 7 lines
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +1 -0
@@ -16,3 +16,4 @@
  */
 extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
 extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
+extern void * __init get_tangier_ops(void) __attribute__((weak));

We should use "__weak" instead of the gcc-specific "__attribute__((weak))".

I don't think it's a good idea to use __weak on a declaration in a header file. If there are ever multiple definitions of the symbol, they are *all* made weak symbols, and one is chosen based on link order, which is error-prone. I only see one definition now, but the whole point of weak is to allow multiple definitions, so this looks like a problem waiting to happen. See 10629d711ed, for example.

It look me a bit to figure out that these get_*_ops() functions are used by INTEL_MID_OPS_INIT, which constructs the name using a macro, so grep/cscope/etc. don't see any users. A comment pointing to INTEL_MID_OPS_INIT would be helpful.

What's the reason for making these symbols weak? Normally we use weak to make a generic default version of a function, while allowing architectures to replace the default with their own version if necessary. But I don't see that happening here. Maybe I'm just missing it, like I missed the uses of get_tangier_ops(), et al.

Bjorn

#8 lore raw

Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support

Hi Bjorn,

Quoted 33 lines · Bjorn Helgaas
Quoted 6 lines · David Cohen

This code was partially based on Mark Brown's previous work.

Cc Mark F. Brown <[email protected]>
Cc Kuppuswamy Sathyanarayanan <[email protected]>

I know this has already been merged to Linus' tree, but it looks funny to me.

arch/x86/platform/intel-mid/intel_mid_weak_decls.h line 19

Quoted 7 lines
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +1 -0
@@ -16,3 +16,4 @@
  */
 extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
 extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
+extern void * __init get_tangier_ops(void) __attribute__((weak));

We should use "__weak" instead of the gcc-specific "__attribute__((weak))".

I don't think it's a good idea to use __weak on a declaration in a header file. If there are ever multiple definitions of the symbol, they are *all* made weak symbols, and one is chosen based on link order, which is error-prone. I only see one definition now, but the whole point of weak is to allow multiple definitions, so this looks like a problem waiting to happen. See 10629d711ed, for example.

It look me a bit to figure out that these get_*_ops() functions are used by INTEL_MID_OPS_INIT, which constructs the name using a macro, so grep/cscope/etc. don't see any users. A comment pointing to INTEL_MID_OPS_INIT would be helpful.

Quoted 5 lines

What's the reason for making these symbols weak? Normally we use weak to make a generic default version of a function, while allowing architectures to replace the default with their own version if necessary. But I don't see that happening here. Maybe I'm just missing it, like I missed the uses of get_tangier_ops(), et al.

Intel mid was implemented in such way that we should select which soc to be used in compilation time. Depending on the selection, mfld.c or mrfl.c could not be compiled then some symbols wouldn't be available.

But IMHO this is a bad legacy design that exists in there, so I started to rework it as you can see in this commit:

commit 4cb9b00f42e07830310319a07e6c91413ee8153e

Author: David Cohen <[email protected]>
Date:   Mon Dec 16 17:37:26 2013 -0800

x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

I'm sending more patches soon and getting rid of intel_mid_weak_decls.h file is in my TODO list.

Br, David

maintainer
#9 lore raw

Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support

Quoted 50 lines · David Cohen

Hi Bjorn,

Quoted 33 lines · Bjorn Helgaas
Quoted 6 lines · David Cohen

This code was partially based on Mark Brown's previous work.

Cc Mark F. Brown <[email protected]>
Cc Kuppuswamy Sathyanarayanan <[email protected]>

I know this has already been merged to Linus' tree, but it looks funny to me.

arch/x86/platform/intel-mid/intel_mid_weak_decls.h line 19

Quoted 7 lines
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +1 -0
@@ -16,3 +16,4 @@
  */
 extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
 extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
+extern void * __init get_tangier_ops(void) __attribute__((weak));

We should use "__weak" instead of the gcc-specific "__attribute__((weak))".

I don't think it's a good idea to use __weak on a declaration in a header file. If there are ever multiple definitions of the symbol, they are *all* made weak symbols, and one is chosen based on link order, which is error-prone. I only see one definition now, but the whole point of weak is to allow multiple definitions, so this looks like a problem waiting to happen. See 10629d711ed, for example.

It look me a bit to figure out that these get_*_ops() functions are used by INTEL_MID_OPS_INIT, which constructs the name using a macro, so grep/cscope/etc. don't see any users. A comment pointing to INTEL_MID_OPS_INIT would be helpful.

Quoted 5 lines

What's the reason for making these symbols weak? Normally we use weak to make a generic default version of a function, while allowing architectures to replace the default with their own version if necessary. But I don't see that happening here. Maybe I'm just missing it, like I missed the uses of get_tangier_ops(), et al.

Intel mid was implemented in such way that we should select which soc to be used in compilation time. Depending on the selection, mfld.c or mrfl.c could not be compiled then some symbols wouldn't be available.

But IMHO this is a bad legacy design that exists in there, so I started to rework it as you can see in this commit:

commit 4cb9b00f42e07830310319a07e6c91413ee8153e

Quoted 8 lines
Author: David Cohen <[email protected]>
Date:   Mon Dec 16 17:37:26 2013 -0800

x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

I'm sending more patches soon and getting rid of intel_mid_weak_decls.h file is in my TODO list.

Sounds good. While you're looking at it, I have similar questions about ipc_device_handler() and msic_generic_platform_data(). It's not clear to me why they should be weak.

Bjorn

#10 lore raw

Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support

Quoted 58 lines · Bjorn Helgaas
Quoted 56 lines · David Cohen

Hi Bjorn,

Quoted 33 lines · Bjorn Helgaas
Quoted 6 lines · David Cohen

This code was partially based on Mark Brown's previous work.

Cc Mark F. Brown <[email protected]>
Cc Kuppuswamy Sathyanarayanan <[email protected]>

I know this has already been merged to Linus' tree, but it looks funny to me.

arch/x86/platform/intel-mid/intel_mid_weak_decls.h line 19

Quoted 7 lines
arch/x86/platform/intel-mid/intel_mid_weak_decls.h +1 -0
@@ -16,3 +16,4 @@
  */
 extern void * __cpuinit get_penwell_ops(void) __attribute__((weak));
 extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak));
+extern void * __init get_tangier_ops(void) __attribute__((weak));

We should use "__weak" instead of the gcc-specific "__attribute__((weak))".

I don't think it's a good idea to use __weak on a declaration in a header file. If there are ever multiple definitions of the symbol, they are *all* made weak symbols, and one is chosen based on link order, which is error-prone. I only see one definition now, but the whole point of weak is to allow multiple definitions, so this looks like a problem waiting to happen. See 10629d711ed, for example.

It look me a bit to figure out that these get_*_ops() functions are used by INTEL_MID_OPS_INIT, which constructs the name using a macro, so grep/cscope/etc. don't see any users. A comment pointing to INTEL_MID_OPS_INIT would be helpful.

Quoted 5 lines

What's the reason for making these symbols weak? Normally we use weak to make a generic default version of a function, while allowing architectures to replace the default with their own version if necessary. But I don't see that happening here. Maybe I'm just missing it, like I missed the uses of get_tangier_ops(), et al.

Intel mid was implemented in such way that we should select which soc to be used in compilation time. Depending on the selection, mfld.c or mrfl.c could not be compiled then some symbols wouldn't be available.

But IMHO this is a bad legacy design that exists in there, so I started to rework it as you can see in this commit:

commit 4cb9b00f42e07830310319a07e6c91413ee8153e

Author: David Cohen <[email protected]>
Date:   Mon Dec 16 17:37:26 2013 -0800

x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

Quoted 6 lines
Quoted 2 lines

I'm sending more patches soon and getting rid of intel_mid_weak_decls.h file is in my TODO list.

Sounds good. While you're looking at it, I have similar questions about ipc_device_handler() and msic_generic_platform_data(). It's not clear to me why they should be weak.

I'm afraid that's gargabe I missed. It supposed to be removed already.

The original upstreamed patch set needed it, since all platform data were gathered in a board.c file and some of them could not be compiled. You can see it here:
http://us.generation-nt.com/answer/patch-v2-00-10-rework-arch-x86-platform-mrst-intel-mid-help-212689892.html

But I reworked this approach and added a sfi_device() macro to let compiler to gather all the platform data, thus board.c file doesn't exist. It means it is not necessary anymore to be weak. I can send a patch right away fixing it.

Thanks for pointing that out.

Br, David

Quoted 2 lines

Bjorn

#11 lore raw

PATCH 1/1 x86: intel-mid: cleanup some platform code's header files

platform_ipc.h and platform_msic.h are wrongly declaring functions as external and with 'weak' attribute. This patch does a cleanup on those header files.

It should have no functional change.

---
 arch/x86/platform/intel-mid/device_libs/platform_ipc.h  | 5 +++--
 arch/x86/platform/intel-mid/device_libs/platform_msic.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)
arch/x86/platform/intel-mid/device_libs/platform_ipc.h +3 -2
index 8f568dd79605..79bb09d4f718 100644
@@ -12,6 +12,7 @@
 #ifndef _PLATFORM_IPC_H_
 #define _PLATFORM_IPC_H_
 
-extern void __init ipc_device_handler(struct sfi_device_table_entry *pentry,
-			struct devs_id *dev) __attribute__((weak));
+void __init
+ipc_device_handler(struct sfi_device_table_entry *pentry, struct devs_id *dev);
+
 #endif
arch/x86/platform/intel-mid/device_libs/platform_msic.h +2 -2
index 917eb56d77da..b7be1d041da2 100644
@@ -14,6 +14,6 @@
 
 extern struct intel_msic_platform_data msic_pdata;
 
-extern void *msic_generic_platform_data(void *info,
-			enum intel_msic_block block) __attribute__((weak));
+void *msic_generic_platform_data(void *info, enum intel_msic_block block);
+
 #endif
Signature · 1 line
1.8.4.2
#12 lore raw

[tip:x86/intel-mid] x86, intel-mid: Cleanup some platform code' s header files

x86, intel-mid: Cleanup some platform code's header files
Commit-ID:  790ed7421a973d9773dda8b4e5300c3f7f6fbcf7
Gitweb:     http://git.kernel.org/tip/790ed7421a973d9773dda8b4e5300c3f7f6fbcf7
Author:     David Cohen <[email protected]>
AuthorDate: Tue, 28 Jan 2014 15:09:27 -0800
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Tue, 28 Jan 2014 15:13:40 -0800

x86, intel-mid: Cleanup some platform code's header files

platform_ipc.h and platform_msic.h are wrongly declaring functions as external and with 'weak' attribute. This patch does a cleanup on those header files.

It should have no functional change.

---
 arch/x86/platform/intel-mid/device_libs/platform_ipc.h  | 5 +++--
 arch/x86/platform/intel-mid/device_libs/platform_msic.h | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)
arch/x86/platform/intel-mid/device_libs/platform_ipc.h +3 -2
index 8f568dd..79bb09d 100644
@@ -12,6 +12,7 @@
 #ifndef _PLATFORM_IPC_H_
 #define _PLATFORM_IPC_H_
 
-extern void __init ipc_device_handler(struct sfi_device_table_entry *pentry,
-			struct devs_id *dev) __attribute__((weak));
+void __init
+ipc_device_handler(struct sfi_device_table_entry *pentry, struct devs_id *dev);
+
 #endif
arch/x86/platform/intel-mid/device_libs/platform_msic.h +2 -2
index 917eb56..b7be1d0 100644
@@ -14,6 +14,6 @@
 
 extern struct intel_msic_platform_data msic_pdata;
 
-extern void *msic_generic_platform_data(void *info,
-			enum intel_msic_block block) __attribute__((weak));
+void *msic_generic_platform_data(void *info, enum intel_msic_block block);
+
 #endif
#13 lore raw

PATCH v2 4/4 x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

In reply to David Cohen

We want to support all Intel Mid platforms with a single config selection. This patch removes deprecated CONFIG_X86_MDFLD and X86_WANT_INTEL_MID options in favor of having CONFIG_X86_INTEL_MID only.

---
 arch/x86/Kconfig | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)
arch/x86/Kconfig +5 -24
index e903c71f7e69..b6a344fb01ae 100644
@@ -437,42 +437,23 @@ config X86_INTEL_CE
 	  This option compiles in support for the CE4100 SOC for settop
 	  boxes and media devices.
 
-config X86_WANT_INTEL_MID
+config X86_INTEL_MID
 	bool "Intel MID platform support"
 	depends on X86_32
 	depends on X86_EXTENDED_PLATFORM
-	---help---
-	  Select to build a kernel capable of supporting Intel MID platform
-	  systems which do not have the PCI legacy interfaces (Moorestown,
-	  Medfield). If you are building for a PC class system say N here.
-
-if X86_WANT_INTEL_MID
-
-config X86_INTEL_MID
-	bool
-
-config X86_MDFLD
-       bool "Medfield MID platform"
 	depends on PCI
 	depends on PCI_GOANY
 	depends on X86_IO_APIC
-	select X86_INTEL_MID
 	select SFI
+	select I2C
 	select DW_APB_TIMER
 	select APB_TIMER
-	select I2C
-	select SPI
 	select INTEL_SCU_IPC
-	select X86_PLATFORM_DEVICES
 	select MFD_INTEL_MSIC
 	---help---
-	  Medfield is Intel's Low Power Intel Architecture (LPIA) based Moblin
-	  Internet Device(MID) platform. 
-	  Unlike standard x86 PCs, Medfield does not have many legacy devices
-	  nor standard legacy replacement devices/features. e.g. Medfield does
-	  not contain i8259, i8254, HPET, legacy BIOS, most of the io ports.
-
-endif
+	  Select to build a kernel capable of supporting Intel MID platform
+	  systems which do not have the PCI legacy interfaces (Moorestown,
+	  Medfield). If you are building for a PC class system say N here.
 
 config X86_INTEL_LPSS
 	bool "Intel Low Power Subsystem Support"
Signature · 1 line
1.8.4.2
maintainer
#14 lore raw

Re: [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

Quoted 48 lines · David Cohen

We want to support all Intel Mid platforms with a single config selection. This patch removes deprecated CONFIG_X86_MDFLD and X86_WANT_INTEL_MID options in favor of having CONFIG_X86_INTEL_MID only.

---
 arch/x86/Kconfig | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)
arch/x86/Kconfig +2 -17
index e903c71f7e69..b6a344fb01ae 100644
@@ -437,42 +437,23 @@ config X86_INTEL_CE
          This option compiles in support for the CE4100 SOC for settop
          boxes and media devices.
 
-config X86_WANT_INTEL_MID
+config X86_INTEL_MID
        bool "Intel MID platform support"
        depends on X86_32
        depends on X86_EXTENDED_PLATFORM
-       ---help---
-         Select to build a kernel capable of supporting Intel MID platform
-         systems which do not have the PCI legacy interfaces (Moorestown,
-         Medfield). If you are building for a PC class system say N here.
-
-if X86_WANT_INTEL_MID
-
-config X86_INTEL_MID
-       bool
-
-config X86_MDFLD
-       bool "Medfield MID platform"
        depends on PCI
        depends on PCI_GOANY
        depends on X86_IO_APIC
-       select X86_INTEL_MID
        select SFI
+       select I2C
        select DW_APB_TIMER
        select APB_TIMER
-       select I2C
-       select SPI
        select INTEL_SCU_IPC
-       select X86_PLATFORM_DEVICES
        select MFD_INTEL_MSIC
        ---help---

arch/x86/Kconfig config X86_INTEL_CE · line 456

Quoted 10 lines
arch/x86/Kconfig +3 -7
-         Medfield is Intel's Low Power Intel Architecture (LPIA) based Moblin
-         Internet Device(MID) platform.
-         Unlike standard x86 PCs, Medfield does not have many legacy devices
-         nor standard legacy replacement devices/features. e.g. Medfield does
-         not contain i8259, i8254, HPET, legacy BIOS, most of the io ports.
-
-endif
+         Select to build a kernel capable of supporting Intel MID platform
+         systems which do not have the PCI legacy interfaces (Moorestown,
+         Medfield). If you are building for a PC class system say N here.

It'd be nice to know what "MID" means -- from Googling, I guess "Mobile Internet Device."

Quoted 5 lines
arch/x86/Kconfig +0 -1
 config X86_INTEL_LPSS
        bool "Intel Low Power Subsystem Support"
--

1.8.4.2

#15 lore raw

Re: [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

Hi, Bjorn,

[snip]

Quoted 2 lines

It'd be nice to know what "MID" means -- from Googling, I guess "Mobile Internet Device."

Yes, that's correct. Wikipedia explains it:
http://en.wikipedia.org/wiki/Mobile_Internet_device

I'll prepare a patch adding intel mid to Documentation/ as soon as I have time for it.

Br, David Cohen

#16 lore raw

PATCH v2 4/4 x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

In reply to David Cohen

We want to support all Intel Mid platforms with a single config selection. This patch removes deprecated CONFIG_X86_MDFLD and X86_WANT_INTEL_MID options in favor of having CONFIG_X86_INTEL_MID only.

---
 arch/x86/Kconfig | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)
arch/x86/Kconfig +7 -23
index e903c71f7e69..f998be2d2901 100644
@@ -437,42 +437,26 @@ config X86_INTEL_CE
 	  This option compiles in support for the CE4100 SOC for settop
 	  boxes and media devices.
 
-config X86_WANT_INTEL_MID
+config X86_INTEL_MID
 	bool "Intel MID platform support"
 	depends on X86_32
 	depends on X86_EXTENDED_PLATFORM
-	---help---
-	  Select to build a kernel capable of supporting Intel MID platform
-	  systems which do not have the PCI legacy interfaces (Moorestown,
-	  Medfield). If you are building for a PC class system say N here.
-
-if X86_WANT_INTEL_MID
-
-config X86_INTEL_MID
-	bool
-
-config X86_MDFLD
-       bool "Medfield MID platform"
 	depends on PCI
 	depends on PCI_GOANY
 	depends on X86_IO_APIC
-	select X86_INTEL_MID
 	select SFI
+	select I2C
 	select DW_APB_TIMER
 	select APB_TIMER
-	select I2C
-	select SPI
 	select INTEL_SCU_IPC
-	select X86_PLATFORM_DEVICES
 	select MFD_INTEL_MSIC
 	---help---
-	  Medfield is Intel's Low Power Intel Architecture (LPIA) based Moblin
-	  Internet Device(MID) platform. 
-	  Unlike standard x86 PCs, Medfield does not have many legacy devices
-	  nor standard legacy replacement devices/features. e.g. Medfield does
-	  not contain i8259, i8254, HPET, legacy BIOS, most of the io ports.
+	  Select to build a kernel capable of supporting Intel MID (Mobile
+	  Internet Device) platform systems which do not have the PCI legacy
+	  interfaces. If you are building for a PC class system say N here.
 
-endif
+	  Intel MID platforms are based on an Intel processor and chipset which
+	  consume less power than most of the x86 derivatives.
 
 config X86_INTEL_LPSS
 	bool "Intel Low Power Subsystem Support"
Signature · 1 line
1.8.4.2
#17 lore raw

PATCH 1/2 x86: intel-mid: return proper error code from get_gpio_by_name()

get_gpio_by_name() should return an error code instead of hardcoded -1.

---
 arch/x86/platform/intel-mid/sfi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/platform/intel-mid/sfi.c +1 -1
index 80a52288555c..c67b9a2f48a1 100644
@@ -224,7 +224,7 @@ int get_gpio_by_name(const char *name)
 		if (!strncmp(name, pentry->pin_name, SFI_NAME_LEN))
 			return pentry->pin_no;
 	}
-	return -1;
+	return -EINVAL;
 }
 
 void __init intel_scu_device_register(struct platform_device *pdev)
Signature · 1 line
1.8.4.2
#18 lore raw

PATCH 2/2 x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

---
 arch/x86/platform/intel-mid/sfi.c | 6 ++++++
 1 file changed, 6 insertions(+)
arch/x86/platform/intel-mid/sfi.c +6 -0
index c67b9a2f48a1..438306ebed05 100644
@@ -337,6 +337,8 @@ static void __init sfi_handle_ipc_dev(struct sfi_device_table_entry *pentry,
 	pr_debug("IPC bus, name = %16.16s, irq = 0x%2x\n",
 		pentry->name, pentry->irq);
 	pdata = intel_mid_sfi_get_pdata(dev, pentry);
+	if (IS_ERR(pdata))
+		return;
 
 	pdev = platform_device_alloc(pentry->name, 0);
 	if (pdev == NULL) {
@@ -370,6 +372,8 @@ static void __init sfi_handle_spi_dev(struct sfi_device_table_entry *pentry,
 		spi_info.chip_select);
 
 	pdata = intel_mid_sfi_get_pdata(dev, &spi_info);
+	if (IS_ERR(pdata))
+		return;
 
 	spi_info.platform_data = pdata;
 	if (dev->delay)
@@ -395,6 +399,8 @@ static void __init sfi_handle_i2c_dev(struct sfi_device_table_entry *pentry,
 		i2c_info.addr);
 	pdata = intel_mid_sfi_get_pdata(dev, &i2c_info);
 	i2c_info.platform_data = pdata;
+	if (IS_ERR(pdata))
+		return;
 
 	if (dev->delay)
 		intel_scu_i2c_device_register(pentry->host_num, &i2c_info);
Signature · 1 line
1.8.4.2
#19 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks,

Ingo

#20 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Hi Ingo,

Quoted 11 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and

Despite the bug exists and the patch is fixing it, it is really unlikely to happen regularly unless the hw has wrong FW. OTOH it is pretty useful during development of platform code to spot and fix errors and typos faster. I don't think there's need to go to 3.13. It can be applied for next version after the patch set I sent it in reply to.

Quoted 5 lines

also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

Quoted 8 lines

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

I'll amend the description.

Br, David

Quoted 4 lines

Thanks,

Ingo

#21 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

In reply to Ingo Molnar

Hi Ingo,

Quoted 16 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

Quoted 8 lines

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Br, David Cohen

Quoted 4 lines

Thanks,

Ingo

#22 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Quoted 24 lines · David Cohen

Hi Ingo,

Quoted 21 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

Quoted 7 lines
Quoted 3 lines

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Hm, in what way does the new changelog address my first request:

Quoted 1 line
Quoted 1 line

- first describe the symptoms of the bug - how does a user notice?

They are all phrased as bug fixes, yet _none_ of the three changelogs appears to describe specific symptoms on specific systems - they all seem to talk in the abstract, with no specific connection to reality.

That really makes it harder for patches to get into the (way too narrow) attention span of maintainersm, while phrasing it like this:

'If an Intel-MID system boots in a specific SFI environment then it
will hang on bootup without this fix.'

or:

'Existing Intel-MID hardware will run faster with this patch.'

will certainly wake up maintainers like a good coffee in the morning.

If a patch is a cleanup with no known bug fix effects then say so in the title and the changelog.

Thanks,

Ingo

#23 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Quoted 49 lines · Ingo Molnar
Quoted 24 lines · David Cohen

Hi Ingo,

Quoted 21 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

Quoted 7 lines
Quoted 3 lines

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Hm, in what way does the new changelog address my first request:

Quoted 1 line
Quoted 1 line

- first describe the symptoms of the bug - how does a user notice?

They are all phrased as bug fixes, yet _none_ of the three changelogs appears to describe specific symptoms on specific systems - they all seem to talk in the abstract, with no specific connection to reality.

That really makes it harder for patches to get into the (way too narrow) attention span of maintainersm, while phrasing it like this:

'If an Intel-MID system boots in a specific SFI environment then it
will hang on bootup without this fix.'

Quoted 8 lines

or:

'Existing Intel-MID hardware will run faster with this patch.'

will certainly wake up maintainers like a good coffee in the morning.

If a patch is a cleanup with no known bug fix effects then say so in the title and the changelog.

Fair enough.
These patches are fixing a potential bug that exists in current kernel, but I triggered with patches in my development tree that depends on this one to be refactored first:
https://patchwork.kernel.org/patch/3109791/

I tried to describe the potential bug, but it lacks the real use case as you pointed out. I'll resend the patches in a way to trigger and describe the situation without dependiing on non-upstreamed patches yet. And I'll hurry up to publish my intel mid devel tree as well.

I hope the new patch set tastes like good morning Brazilian coffee :)

Br, David Cohen

Quoted 4 lines

Thanks,

Ingo

#24 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Hi Ingo and hpa,

Quoted 65 lines · David Cohen
Quoted 49 lines · Ingo Molnar
Quoted 24 lines · David Cohen

Hi Ingo,

Quoted 21 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

Quoted 7 lines
Quoted 3 lines

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Hm, in what way does the new changelog address my first request:

Quoted 1 line
Quoted 1 line

- first describe the symptoms of the bug - how does a user notice?

They are all phrased as bug fixes, yet _none_ of the three changelogs appears to describe specific symptoms on specific systems - they all seem to talk in the abstract, with no specific connection to reality.

That really makes it harder for patches to get into the (way too narrow) attention span of maintainersm, while phrasing it like this:

'If an Intel-MID system boots in a specific SFI environment then it
will hang on bootup without this fix.'

Quoted 8 lines

or:

'Existing Intel-MID hardware will run faster with this patch.'

will certainly wake up maintainers like a good coffee in the morning.

If a patch is a cleanup with no known bug fix effects then say so in the title and the changelog.

Fair enough.
These patches are fixing a potential bug that exists in current kernel, but I triggered with patches in my development tree that depends on this one to be refactored first:
https://patchwork.kernel.org/patch/3109791/

Quoted 6 lines

I tried to describe the potential bug, but it lacks the real use case as you pointed out. I'll resend the patches in a way to trigger and describe the situation without dependiing on non-upstreamed patches yet. And I'll hurry up to publish my intel mid devel tree as well.

I hope the new patch set tastes like good morning Brazilian coffee :)

In order to show a practical error case fixed by this patch set using current legacy platform code, I need to get them working first. But it turns out legacy platform code (for Moorestown and Medfield) aren't in a good shape at all. I found few cases of obsolete platform data being returned from platform code (intel mid was orphan for too long on upstream).

I'll have to append new patches to this set "[PATCH v2 0/3] x86:
intel-mid: handle platform code error in better way", so it won't be a simple fix of patch description.

In order to not block the rest of my patches on thread "[PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID", please consider to apply them first (maybe for 3.14 if possible).

When I resend these patches here, we can consider apply them on 3.14-rcX (as they are bug fixes) or just postpone them to >3.14.

Thanks,

David Cohen

Quoted 7 lines

Br, David Cohen

Quoted 4 lines

Thanks,

Ingo

#25 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Quoted 82 lines · David Cohen

Hi Ingo and hpa,

Quoted 65 lines · David Cohen
Quoted 49 lines · Ingo Molnar
Quoted 24 lines · David Cohen

Hi Ingo,

Quoted 21 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

Quoted 7 lines
Quoted 3 lines

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Hm, in what way does the new changelog address my first request:

Quoted 1 line
Quoted 1 line

- first describe the symptoms of the bug - how does a user notice?

They are all phrased as bug fixes, yet _none_ of the three changelogs appears to describe specific symptoms on specific systems - they all seem to talk in the abstract, with no specific connection to reality.

That really makes it harder for patches to get into the (way too narrow) attention span of maintainersm, while phrasing it like this:

'If an Intel-MID system boots in a specific SFI environment then it
will hang on bootup without this fix.'

Quoted 8 lines

or:

'Existing Intel-MID hardware will run faster with this patch.'

will certainly wake up maintainers like a good coffee in the morning.

If a patch is a cleanup with no known bug fix effects then say so in the title and the changelog.

Fair enough.
These patches are fixing a potential bug that exists in current kernel, but I triggered with patches in my development tree that depends on this one to be refactored first:
https://patchwork.kernel.org/patch/3109791/

Quoted 6 lines

I tried to describe the potential bug, but it lacks the real use case as you pointed out. I'll resend the patches in a way to trigger and describe the situation without dependiing on non-upstreamed patches yet. And I'll hurry up to publish my intel mid devel tree as well.

I hope the new patch set tastes like good morning Brazilian coffee :)

In order to show a practical error case fixed by this patch set using current legacy platform code, I need to get them working first. But it turns out legacy platform code (for Moorestown and Medfield) aren't in a good shape at all. I found few cases of obsolete platform data being returned from platform code (intel mid was orphan for too long on upstream).

Quoted 3 lines

I'll have to append new patches to this set "[PATCH v2 0/3] x86:
intel-mid: handle platform code error in better way", so it won't be a simple fix of patch description.

Great, more fixes to the code is the best kind of fix to a changelog.

Quoted 3 lines

In order to not block the rest of my patches on thread "[PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID", please consider to apply them first (maybe for 3.14 if possible).

Sure, those look fine to me, but please don't forget about these fixes either.

Quoted 2 lines

When I resend these patches here, we can consider apply them on 3.14-rcX (as they are bug fixes) or just postpone them to >3.14.

Please send them ASAP, don't wait for v3.14 -rc's. We'll handle the logistics. Sending those 3 fixes with an improved changelog would be a good start.

Thanks,

Ingo

#26 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Quoted 85 lines · Ingo Molnar
Quoted 82 lines · David Cohen

Hi Ingo and hpa,

Quoted 65 lines · David Cohen
Quoted 49 lines · Ingo Molnar
Quoted 24 lines · David Cohen

Hi Ingo,

Quoted 21 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

Quoted 7 lines
Quoted 3 lines

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Hm, in what way does the new changelog address my first request:

Quoted 1 line
Quoted 1 line

- first describe the symptoms of the bug - how does a user notice?

They are all phrased as bug fixes, yet _none_ of the three changelogs appears to describe specific symptoms on specific systems - they all seem to talk in the abstract, with no specific connection to reality.

That really makes it harder for patches to get into the (way too narrow) attention span of maintainersm, while phrasing it like this:

'If an Intel-MID system boots in a specific SFI environment then it
will hang on bootup without this fix.'

Quoted 8 lines

or:

'Existing Intel-MID hardware will run faster with this patch.'

will certainly wake up maintainers like a good coffee in the morning.

If a patch is a cleanup with no known bug fix effects then say so in the title and the changelog.

Fair enough.
These patches are fixing a potential bug that exists in current kernel, but I triggered with patches in my development tree that depends on this one to be refactored first:
https://patchwork.kernel.org/patch/3109791/

Quoted 6 lines

I tried to describe the potential bug, but it lacks the real use case as you pointed out. I'll resend the patches in a way to trigger and describe the situation without dependiing on non-upstreamed patches yet. And I'll hurry up to publish my intel mid devel tree as well.

I hope the new patch set tastes like good morning Brazilian coffee :)

In order to show a practical error case fixed by this patch set using current legacy platform code, I need to get them working first. But it turns out legacy platform code (for Moorestown and Medfield) aren't in a good shape at all. I found few cases of obsolete platform data being returned from platform code (intel mid was orphan for too long on upstream).

Quoted 5 lines
Quoted 3 lines

I'll have to append new patches to this set "[PATCH v2 0/3] x86:
intel-mid: handle platform code error in better way", so it won't be a simple fix of patch description.

Great, more fixes to the code is the best kind of fix to a changelog.

IMHO it's better than say "considering this driver works, it would fail this way because of SFI doing that thing..."

But thinking twice, I'm tempted to say these patches simply don't do functional changes right now. Than I can handle fixing the platform code and the new driver's upstreaming that depends on this fix perhaps on 3.15.

Quoted 7 lines
Quoted 4 lines

In order to not block the rest of my patches on thread "[PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID", please consider to apply them first (maybe for 3.14 if possible).

Sure, those look fine to me, but please don't forget about these fixes either.

Thanks.

Quoted 7 lines
Quoted 3 lines

When I resend these patches here, we can consider apply them on 3.14-rcX (as they are bug fixes) or just postpone them to >3.14.

Please send them ASAP, don't wait for v3.14 -rc's. We'll handle the logistics. Sending those 3 fixes with an improved changelog would be a good start.

I'm currently stuck with full day trainings for the rest of the week in my job. I can resend ASAP if assuming this is a non-functional change at this moment.

Br, David Cohen

Quoted 4 lines

Thanks,

Ingo

#27 lore raw

Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

Quoted 113 lines · David Cohen
Quoted 85 lines · Ingo Molnar
Quoted 82 lines · David Cohen

Hi Ingo and hpa,

Quoted 65 lines · David Cohen
Quoted 49 lines · Ingo Molnar
Quoted 24 lines · David Cohen

Hi Ingo,

Quoted 21 lines · Ingo Molnar
Quoted 5 lines · David Cohen

Prevent sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata.

Is this bug triggering in practice? If not then please say so in the changelog. If yes then is this patch desired for v3.13 merging and also please fix the changelog to conform to the standard changelog style:

- first describe the symptoms of the bug - how does a user notice?

- then describe how the code behaves today and how that is causing the bug

- and then only describe how it's fixed.

Quoted 7 lines
Quoted 3 lines

The first item is the most important one - while developers (naturally) tend to concentrate on the least important point, the last one.

Thanks for the feedback :)
This new patch set was done in reply to your comment:
https://lkml.org/lkml/2013/12/20/517

Hm, in what way does the new changelog address my first request:

Quoted 1 line
Quoted 1 line

- first describe the symptoms of the bug - how does a user notice?

They are all phrased as bug fixes, yet _none_ of the three changelogs appears to describe specific symptoms on specific systems - they all seem to talk in the abstract, with no specific connection to reality.

That really makes it harder for patches to get into the (way too narrow) attention span of maintainersm, while phrasing it like this:

'If an Intel-MID system boots in a specific SFI environment then it
will hang on bootup without this fix.'

Quoted 8 lines

or:

'Existing Intel-MID hardware will run faster with this patch.'

will certainly wake up maintainers like a good coffee in the morning.

If a patch is a cleanup with no known bug fix effects then say so in the title and the changelog.

Fair enough.
These patches are fixing a potential bug that exists in current kernel, but I triggered with patches in my development tree that depends on this one to be refactored first:
https://patchwork.kernel.org/patch/3109791/

Quoted 6 lines

I tried to describe the potential bug, but it lacks the real use case as you pointed out. I'll resend the patches in a way to trigger and describe the situation without dependiing on non-upstreamed patches yet. And I'll hurry up to publish my intel mid devel tree as well.

I hope the new patch set tastes like good morning Brazilian coffee :)

In order to show a practical error case fixed by this patch set using current legacy platform code, I need to get them working first. But it turns out legacy platform code (for Moorestown and Medfield) aren't in a good shape at all. I found few cases of obsolete platform data being returned from platform code (intel mid was orphan for too long on upstream).

Quoted 5 lines
Quoted 3 lines

I'll have to append new patches to this set "[PATCH v2 0/3] x86:
intel-mid: handle platform code error in better way", so it won't be a simple fix of patch description.

Great, more fixes to the code is the best kind of fix to a changelog.

IMHO it's better than say "considering this driver works, it would fail this way because of SFI doing that thing..."

But thinking twice, I'm tempted to say these patches simply don't do functional changes right now. Than I can handle fixing the platform code and the new driver's upstreaming that depends on this fix perhaps on 3.15.

Quoted 7 lines
Quoted 4 lines

In order to not block the rest of my patches on thread "[PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID", please consider to apply them first (maybe for 3.14 if possible).

Sure, those look fine to me, but please don't forget about these fixes either.

Thanks.

Quoted 4 lines
Quoted 3 lines

When I resend these patches here, we can consider apply them on 3.14-rcX (as they are bug fixes) or just postpone them to >3.14.

Quoted 7 lines
Quoted 3 lines

Please send them ASAP, don't wait for v3.14 -rc's. We'll handle the logistics. Sending those 3 fixes with an improved changelog would be a good start.

I'm currently stuck with full day trainings for the rest of the week in my job. I can resend ASAP if assuming this is a non-functional change at this moment.

I resent this patch set here:
https://lkml.org/lkml/2014/1/16/498

It is non-functional now (explicitly said on changelogs). I hope it's all good this time.

Br, David Cohen

#28 lore raw

v2 cover x86: intel-mid: handle platform code error in better way

In reply to David Cohen

Hi,

These patches are meant to improve how Intel MID detects error in its platform code. Currently it's ambiguous when platform code returns NULL due to error or due to lack of platform data in valid situations. With these patches, errors will be handled by ERR_PTR()/PTR_ERR() macros.

Br, David Cohen

---
David Cohen (3):
x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
x86: intel-mid: platform code should return error when failing
x86: intel-mid: return proper error code from get_gpio_by_name()

 arch/x86/platform/intel-mid/device_libs/platform_emc1403.c   | 6 ++++--
 arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_lis331.c    | 6 ++++--
 arch/x86/platform/intel-mid/device_libs/platform_max7315.c   | 6 +++---
 arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c   | 4 ++--
 arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c  | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c  | 6 ++++++
 arch/x86/platform/intel-mid/device_libs/platform_tca6416.c   | 6 +++---
 arch/x86/platform/intel-mid/sfi.c                            | 8 +++++++-
 11 files changed, 33 insertions(+), 17 deletions(-)
Signature · 1 line
1.8.4.2
#29 lore raw

PATCH v2 1/3 x86: intel-mid: sfi_handle_*_dev() should check for pdata error code

When Intel MID finds a match between SFI table from FW and registered SFI devices, it will execute platform initialization code, acquire platform data and register platform device to probe the driver. But if platform initialization code fails, Intel MID will still register platform device even though it's likely it won't work properly. This patch handles this error condition in more graceful way.

Since 'NULL' is a valid return value, this patch makes sfi_handle_*_dev() functions to use IS_ERR() to validate returned pdata. Then, it prevents sfi_handle_*_dev() to register device in case intel_mid_sfi_get_pdata() failed to execute.

---
 arch/x86/platform/intel-mid/sfi.c | 6 ++++++
 1 file changed, 6 insertions(+)
arch/x86/platform/intel-mid/sfi.c +6 -0
index 80a52288555c..1d5d20b088fa 100644
@@ -337,6 +337,8 @@ static void __init sfi_handle_ipc_dev(struct sfi_device_table_entry *pentry,
 	pr_debug("IPC bus, name = %16.16s, irq = 0x%2x\n",
 		pentry->name, pentry->irq);
 	pdata = intel_mid_sfi_get_pdata(dev, pentry);
+	if (IS_ERR(pdata))
+		return;
 
 	pdev = platform_device_alloc(pentry->name, 0);
 	if (pdev == NULL) {
@@ -370,6 +372,8 @@ static void __init sfi_handle_spi_dev(struct sfi_device_table_entry *pentry,
 		spi_info.chip_select);
 
 	pdata = intel_mid_sfi_get_pdata(dev, &spi_info);
+	if (IS_ERR(pdata))
+		return;
 
 	spi_info.platform_data = pdata;
 	if (dev->delay)
@@ -395,6 +399,8 @@ static void __init sfi_handle_i2c_dev(struct sfi_device_table_entry *pentry,
 		i2c_info.addr);
 	pdata = intel_mid_sfi_get_pdata(dev, &i2c_info);
 	i2c_info.platform_data = pdata;
+	if (IS_ERR(pdata))
+		return;
 
 	if (dev->delay)
 		intel_scu_i2c_device_register(pentry->host_num, &i2c_info);
Signature · 1 line
1.8.4.2
#30 lore raw

PATCH v2 2/3 x86: intel-mid: platform code should return error when failing

In reply to David Cohen

All Intel MID platform code are responsible for:
- execute platform initialization code
- return platform data (when there's one)

But currently it's ambiguous the error condition during platform initialization and the lack of platform data. In both cases platform code will return NULL.

This patch fixes such situation. From now on NULL is a valid case when there is no platform data and ERR_PTR() is used for error conditions.

---
 arch/x86/platform/intel-mid/device_libs/platform_emc1403.c   | 6 ++++--
 arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_lis331.c    | 6 ++++--
 arch/x86/platform/intel-mid/device_libs/platform_max7315.c   | 6 +++---
 arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c   | 4 ++--
 arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c  | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c | 2 +-
 arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c  | 6 ++++++
 arch/x86/platform/intel-mid/device_libs/platform_tca6416.c   | 6 +++---
 10 files changed, 26 insertions(+), 16 deletions(-)
arch/x86/platform/intel-mid/device_libs/platform_emc1403.c +4 -2
index 0d942c1d26d5..58933daa606d 100644
@@ -22,8 +22,10 @@ static void __init *emc1403_platform_data(void *info)
 	int intr = get_gpio_by_name("thermal_int");
 	int intr2nd = get_gpio_by_name("thermal_alert");
 
-	if (intr == -1 || intr2nd == -1)
-		return NULL;
+	if (intr < 0)
+		return ERR_PTR(intr);
+	if (intr2nd < 0)
+		return ERR_PTR(intr2nd);
 
 	i2c_info->irq = intr + INTEL_MID_IRQ_OFFSET;
 	intr2nd_pdata = intr2nd + INTEL_MID_IRQ_OFFSET;
arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c +1 -1
index a013a4834bbe..dccae6b0413f 100644
@@ -66,7 +66,7 @@ static int __init pb_keys_init(void)
 		gb[i].gpio = get_gpio_by_name(gb[i].desc);
 		pr_debug("info[%2d]: name = %s, gpio = %d\n", i, gb[i].desc,
 					gb[i].gpio);
-		if (gb[i].gpio == -1)
+		if (gb[i].gpio < 0)
 			continue;
 
 		if (i != good)
arch/x86/platform/intel-mid/device_libs/platform_lis331.c +4 -2
index 15278c11f714..506856c418a6 100644
@@ -21,8 +21,10 @@ static void __init *lis331dl_platform_data(void *info)
 	int intr = get_gpio_by_name("accel_int");
 	int intr2nd = get_gpio_by_name("accel_2");
 
-	if (intr == -1 || intr2nd == -1)
-		return NULL;
+	if (intr < 0)
+		return ERR_PTR(intr);
+	if (intr2nd < 0)
+		return ERR_PTR(intr2nd);
 
 	i2c_info->irq = intr + INTEL_MID_IRQ_OFFSET;
 	intr2nd_pdata = intr2nd + INTEL_MID_IRQ_OFFSET;
arch/x86/platform/intel-mid/device_libs/platform_max7315.c +3 -3
index 94ade10024ae..d25ec6d4b606 100644
@@ -31,7 +31,7 @@ static void __init *max7315_platform_data(void *info)
 	if (nr == MAX7315_NUM) {
 		pr_err("too many max7315s, we only support %d\n",
 				MAX7315_NUM);
-		return NULL;
+		return ERR_PTR(-EINVAL);
 	}
 	/* we have several max7315 on the board, we only need load several
 	 * instances of the same pca953x driver to cover them
@@ -48,8 +48,8 @@ static void __init *max7315_platform_data(void *info)
 	gpio_base = get_gpio_by_name(base_pin_name);
 	intr = get_gpio_by_name(intr_pin_name);
 
-	if (gpio_base == -1)
-		return NULL;
+	if (gpio_base < 0)
+		return ERR_PTR(gpio_base);
 	max7315->gpio_base = gpio_base;
 	if (intr != -1) {
 		i2c_info->irq = intr + INTEL_MID_IRQ_OFFSET;
arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c +2 -2
index dd28d63c84fb..c0a8fe7791a0 100644
@@ -19,8 +19,8 @@ static void *mpu3050_platform_data(void *info)
 	struct i2c_board_info *i2c_info = info;
 	int intr = get_gpio_by_name("mpu3050_int");
 
-	if (intr == -1)
-		return NULL;
+	if (intr < 0)
+		return ERR_PTR(intr);
 
 	i2c_info->irq = intr + INTEL_MID_IRQ_OFFSET;
 	return NULL;
arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c +1 -1
index 2a4f7b1dd917..a3843c43ae70 100644
@@ -29,7 +29,7 @@ static void __init *msic_gpio_platform_data(void *info)
 	int gpio = get_gpio_by_name("msic_gpio_base");
 
 	if (gpio < 0)
-		return NULL;
+		return ERR_PTR(gpio);
 
 	msic_gpio_pdata.gpio_base = gpio;
 	msic_pdata.gpio = &msic_gpio_pdata;
arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c +1 -1
index 6497111ddb54..65aa98aee28f 100644
@@ -30,7 +30,7 @@ static void __init *msic_ocd_platform_data(void *info)
 	gpio = get_gpio_by_name("ocd_gpio");
 
 	if (gpio < 0)
-		return NULL;
+		return ERR_PTR(gpio);
 
 	msic_ocd_pdata.gpio = gpio;
 	msic_pdata.ocd = &msic_ocd_pdata;
arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c +1 -1
index d87182a09263..65c2a9a19db4 100644
@@ -26,7 +26,7 @@ static void __init *pmic_gpio_platform_data(void *info)
 	static struct intel_pmic_gpio_platform_data pmic_gpio_pdata;
 	int gpio_base = get_gpio_by_name("pmic_gpio_base");
 
-	if (gpio_base == -1)
+	if (gpio_base < 0)
 		gpio_base = 64;
 	pmic_gpio_pdata.gpio_base = gpio_base;
 	pmic_gpio_pdata.irq_base = gpio_base + INTEL_MID_IRQ_OFFSET;
arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c +6 -0
index 740fc757050c..53fb3ec7cf5d 100644
@@ -21,8 +21,14 @@ static void *tc35876x_platform_data(void *data)
 
 	/* gpio pins set to -1 will not be used by the driver */
 	pdata.gpio_bridge_reset = get_gpio_by_name("LCMB_RXEN");
+	if (pdata.gpio_bridge_reset < 0)
+		return ERR_PTR(pdata.gpio_bridge_reset);
 	pdata.gpio_panel_bl_en = get_gpio_by_name("6S6P_BL_EN");
+	if (pdata.gpio_panel_bl_en < 0)
+		return ERR_PTR(pdata.gpio_panel_bl_en);
 	pdata.gpio_panel_vadd = get_gpio_by_name("EN_VREG_LCD_V3P3");
+	if (pdata.gpio_panel_vadd < 0)
+		return ERR_PTR(pdata.gpio_panel_vadd);
 
 	return &pdata;
 }
arch/x86/platform/intel-mid/device_libs/platform_tca6416.c +3 -3
index 22881c9a6737..e331b38871da 100644
@@ -34,10 +34,10 @@ static void *tca6416_platform_data(void *info)
 	gpio_base = get_gpio_by_name(base_pin_name);
 	intr = get_gpio_by_name(intr_pin_name);
 
-	if (gpio_base == -1)
-		return NULL;
+	if (gpio_base < 0)
+		return ERR_PTR(gpio_base);
 	tca6416.gpio_base = gpio_base;
-	if (intr != -1) {
+	if (intr >= 0) {
 		i2c_info->irq = intr + INTEL_MID_IRQ_OFFSET;
 		tca6416.irq_base = gpio_base + INTEL_MID_IRQ_OFFSET;
 	} else {
Signature · 1 line
1.8.4.2
#31 lore raw

PATCH v2 3/3 x86: intel-mid: return proper error code from get_gpio_by_name()

In reply to David Cohen

get_gpio_by_name() should return an error code instead of hardcoded -1.

---
 arch/x86/platform/intel-mid/sfi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/platform/intel-mid/sfi.c +1 -1
index 1d5d20b088fa..438306ebed05 100644
@@ -224,7 +224,7 @@ int get_gpio_by_name(const char *name)
 		if (!strncmp(name, pentry->pin_name, SFI_NAME_LEN))
 			return pentry->pin_no;
 	}
-	return -1;
+	return -EINVAL;
 }
 
 void __init intel_scu_device_register(struct platform_device *pdev)
Signature · 1 line
1.8.4.2
#32 lore raw

[tip:x86/intel-mid] x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

In reply to David Cohen
x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs
Commit-ID:  4cb9b00f42e07830310319a07e6c91413ee8153e
Gitweb:     http://git.kernel.org/tip/4cb9b00f42e07830310319a07e6c91413ee8153e
Author:     David Cohen <[email protected]>
AuthorDate: Mon, 16 Dec 2013 17:37:26 -0800
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Wed, 15 Jan 2014 14:38:58 -0800

x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs

We want to support all Intel MID (Mobile Internet Device) platforms with a single config selection. This patch removes deprecated CONFIG_X86_MDFLD and X86_WANT_INTEL_MID options in favor of having CONFIG_X86_INTEL_MID only.

---
 arch/x86/Kconfig | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)
arch/x86/Kconfig +7 -23
index 0952ecd..edcf80e 100644
@@ -438,42 +438,26 @@ config X86_INTEL_CE
 	  This option compiles in support for the CE4100 SOC for settop
 	  boxes and media devices.
 
-config X86_WANT_INTEL_MID
+config X86_INTEL_MID
 	bool "Intel MID platform support"
 	depends on X86_32
 	depends on X86_EXTENDED_PLATFORM
-	---help---
-	  Select to build a kernel capable of supporting Intel MID platform
-	  systems which do not have the PCI legacy interfaces (Moorestown,
-	  Medfield). If you are building for a PC class system say N here.
-
-if X86_WANT_INTEL_MID
-
-config X86_INTEL_MID
-	bool
-
-config X86_MDFLD
-       bool "Medfield MID platform"
 	depends on PCI
 	depends on PCI_GOANY
 	depends on X86_IO_APIC
-	select X86_INTEL_MID
 	select SFI
+	select I2C
 	select DW_APB_TIMER
 	select APB_TIMER
-	select I2C
-	select SPI
 	select INTEL_SCU_IPC
-	select X86_PLATFORM_DEVICES
 	select MFD_INTEL_MSIC
 	---help---
-	  Medfield is Intel's Low Power Intel Architecture (LPIA) based Moblin
-	  Internet Device(MID) platform. 
-	  Unlike standard x86 PCs, Medfield does not have many legacy devices
-	  nor standard legacy replacement devices/features. e.g. Medfield does
-	  not contain i8259, i8254, HPET, legacy BIOS, most of the io ports.
+	  Select to build a kernel capable of supporting Intel MID (Mobile
+	  Internet Device) platform systems which do not have the PCI legacy
+	  interfaces. If you are building for a PC class system say N here.
 
-endif
+	  Intel MID platforms are based on an Intel processor and chipset which
+	  consume less power than most of the x86 derivatives.
 
 config X86_INTEL_LPSS
 	bool "Intel Low Power Subsystem Support"
#33 lore raw
In reply to David Cohen
Quoted 6 lines · David Cohen

Hi,

I've a bunch of Intel MID patches under review but it seems they are becoming old and start to need changes.
I gathered an up-to-date version of all of them in this single patch set.

Quoted 8 lines

This series implements support of Clovertrail and Merrifield to Intel MID. It also removes the unwanted X86_MDFLD option since *all* Intel MID platforms should be supported through CONFIG_X86_INTEL_MID option.

This makes obsolete the following patches:
https://lkml.org/lkml/2013/10/18/480
https://lkml.org/lkml/2013/10/22/533
http://lkml.indiana.edu/hypermail/linux/kernel/1311.1/02937.html

Ping :)
Any comments on this whole patch set?

Br, David Cohen

Quoted 29 lines

Br, David

---
David Cohen (3):
x86: intel-mid: move Medfield code out of intel-mid.c core file
x86: intel-mid: add Merrifield platform support
x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID
configs

Kuppuswamy Sathyanarayanan (1):
x86: intel-mid: add Clovertrail platform support

 arch/x86/Kconfig                                   |  29 +-----
 arch/x86/include/asm/intel-mid.h                   |  48 +++++++++-
 arch/x86/pci/intel_mid_pci.c                       |   6 +-
 arch/x86/platform/intel-mid/Makefile               |   4 +-
 arch/x86/platform/intel-mid/intel-mid.c            |  64 +++++++------
 arch/x86/platform/intel-mid/intel_mid_weak_decls.h |  19 ++++
 arch/x86/platform/intel-mid/mfld.c                 |  75 +++++++++++++++
 arch/x86/platform/intel-mid/mrfl.c                 | 103 +++++++++++++++++++++
 arch/x86/platform/intel-mid/sfi.c                  |  34 +++++--
 9 files changed, 317 insertions(+), 65 deletions(-)
 create mode 100644 arch/x86/platform/intel-mid/intel_mid_weak_decls.h
 create mode 100644 arch/x86/platform/intel-mid/mfld.c
 create mode 100644 arch/x86/platform/intel-mid/mrfl.c
Signature · 1 line
1.8.4.2
#34 lore raw
Quoted 11 lines · David Cohen
Quoted 10 lines · David Cohen

Hi,

I've a bunch of Intel MID patches under review but it seems they are becoming old and start to need changes.
I gathered an up-to-date version of all of them in this single patch set.

This series implements support of Clovertrail and Merrifield to Intel MID. It also removes the unwanted X86_MDFLD option since *all* Intel MID platforms should be supported through CONFIG_X86_INTEL_MID option.

Quoted 8 lines
Quoted 4 lines

This makes obsolete the following patches:
https://lkml.org/lkml/2013/10/18/480
https://lkml.org/lkml/2013/10/22/533
http://lkml.indiana.edu/hypermail/linux/kernel/1311.1/02937.html

Ping :)
Any comments on this whole patch set?

You got some feedback from Bjorn and Ingo... mostly description-related, but that is important enough.

Other than that we should queue it up ASAP.

-hpa

#35 lore raw
Quoted 17 lines · H. Peter Anvin
Quoted 16 lines · David Cohen
Quoted 14 lines · David Cohen

Hi,

I've a bunch of Intel MID patches under review but it seems they are becoming old and start to need changes.
I gathered an up-to-date version of all of them in this single patch set.

This series implements support of Clovertrail and Merrifield to Intel MID. It also removes the unwanted X86_MDFLD option since *all* Intel MID platforms should be supported through CONFIG_X86_INTEL_MID option.

This makes obsolete the following patches:
https://lkml.org/lkml/2013/10/18/480
https://lkml.org/lkml/2013/10/22/533
http://lkml.indiana.edu/hypermail/linux/kernel/1311.1/02937.html

Quoted 6 lines
Quoted 3 lines

Ping :)
Any comments on this whole patch set?

You got some feedback from Bjorn and Ingo... mostly description-related, but that is important enough.

Hm. I believe I sent new patch versions based on those comments. But I guess I failed to spot my new patches were my reply :(

Br, David

Quoted 5 lines

Other than that we should queue it up ASAP.

-hpa