linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID
@ 2013-12-16 20:07 David Cohen
  2013-12-16 20:07 ` [PATCH v2 1/4] x86: intel-mid: move Medfield code out of intel-mid.c core file David Cohen
                   ` (5 more replies)
  0 siblings, 6 replies; 36+ messages in thread
From: David Cohen @ 2013-12-16 20:07 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: bhelgaas, david.a.cohen, linux-kernel, linux-pci

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

-- 
1.8.4.2


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 1/4] x86: intel-mid: move Medfield code out of intel-mid.c core file
  2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
@ 2013-12-16 20:07 ` David Cohen
  2014-01-15 22:42   ` [tip:x86/intel-mid] x86, intel-mid: Move " tip-bot for David Cohen
  2013-12-16 20:07 ` [PATCH v2 2/4] x86: intel-mid: add Clovertrail platform support David Cohen
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2013-12-16 20:07 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bhelgaas, david.a.cohen, linux-kernel, linux-pci,
	Kuppuswamy Sathyanarayanan

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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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

diff --git a/arch/x86/platform/intel-mid/Makefile b/arch/x86/platform/intel-mid/Makefile
index 01cc29ea5ff7..78a14ba0e0db 100644
--- a/arch/x86/platform/intel-mid/Makefile
+++ b/arch/x86/platform/intel-mid/Makefile
@@ -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/
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index f90e290f689f..527d6d50643d 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -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);
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
new file mode 100644
index 000000000000..519beb75ba4c
--- /dev/null
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -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));
diff --git a/arch/x86/platform/intel-mid/mfld.c b/arch/x86/platform/intel-mid/mfld.c
new file mode 100644
index 000000000000..c7ff83c4576d
--- /dev/null
+++ b/arch/x86/platform/intel-mid/mfld.c
@@ -0,0 +1,51 @@
+/*
+ * mfld.c: Intel Medfield platform setup code
+ *
+ * (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.
+ */
+
+#include <linux/init.h>
+
+#include <asm/apic.h>
+#include <asm/intel-mid.h>
+#include <asm/intel_mid_vrtc.h>
+
+void intel_mid_power_off(void)
+{
+}
+
+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;
+}
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 2/4] x86: intel-mid: add Clovertrail platform support
  2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
  2013-12-16 20:07 ` [PATCH v2 1/4] x86: intel-mid: move Medfield code out of intel-mid.c core file David Cohen
@ 2013-12-16 20:07 ` David Cohen
  2014-01-15 22:42   ` [tip:x86/intel-mid] x86, intel-mid: Add " tip-bot for Kuppuswamy Sathyanarayanan
  2013-12-16 20:07 ` [PATCH v2 3/4] x86: intel-mid: add Merrifield " David Cohen
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2013-12-16 20:07 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bhelgaas, david.a.cohen, linux-kernel, linux-pci,
	Kuppuswamy Sathyanarayanan, Fei Yang

From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Fei Yang <fei.yang@intel.com>
---
 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(-)

diff --git a/arch/x86/include/asm/intel-mid.h b/arch/x86/include/asm/intel-mid.h
index 459769d39263..f8a831431fe0 100644
--- a/arch/x86/include/asm/intel-mid.h
+++ b/arch/x86/include/asm/intel-mid.h
@@ -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
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 527d6d50643d..40955841bb32 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -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 */
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 519beb75ba4c..9ebce0447edf 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -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));
diff --git a/arch/x86/platform/intel-mid/mfld.c b/arch/x86/platform/intel-mid/mfld.c
index c7ff83c4576d..4f7884eebc14 100644
--- a/arch/x86/platform/intel-mid/mfld.c
+++ b/arch/x86/platform/intel-mid/mfld.c
@@ -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;
+}
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support
  2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
  2013-12-16 20:07 ` [PATCH v2 1/4] x86: intel-mid: move Medfield code out of intel-mid.c core file David Cohen
  2013-12-16 20:07 ` [PATCH v2 2/4] x86: intel-mid: add Clovertrail platform support David Cohen
@ 2013-12-16 20:07 ` David Cohen
  2014-01-15 22:43   ` [tip:x86/intel-mid] x86, intel-mid: Add " tip-bot for David Cohen
  2014-01-28  0:52   ` [PATCH v2 3/4] x86: intel-mid: add " Bjorn Helgaas
  2013-12-16 20:07 ` [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs David Cohen
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 36+ messages in thread
From: David Cohen @ 2013-12-16 20:07 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86
  Cc: bhelgaas, david.a.cohen, linux-kernel, linux-pci, Fei Yang,
	Mark F. Brown, Kuppuswamy Sathyanarayanan

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

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Fei Yang <fei.yang@intel.com>
Cc: Mark F. Brown <mark.f.brown@intel.com>
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
---
 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

diff --git a/arch/x86/include/asm/intel-mid.h b/arch/x86/include/asm/intel-mid.h
index f8a831431fe0..e34e097b6f9d 100644
--- a/arch/x86/include/asm/intel-mid.h
+++ b/arch/x86/include/asm/intel-mid.h
@@ -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
diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index 51384ca727ad..84b9d672843d 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/Makefile b/arch/x86/platform/intel-mid/Makefile
index 78a14ba0e0db..0a8ee703b9fa 100644
--- a/arch/x86/platform/intel-mid/Makefile
+++ b/arch/x86/platform/intel-mid/Makefile
@@ -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
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 40955841bb32..1bbedc4b0f88 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 9ebce0447edf..a537ffc16299 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -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));
diff --git a/arch/x86/platform/intel-mid/mrfl.c b/arch/x86/platform/intel-mid/mrfl.c
new file mode 100644
index 000000000000..09d10159e7b7
--- /dev/null
+++ b/arch/x86/platform/intel-mid/mrfl.c
@@ -0,0 +1,103 @@
+/*
+ * mrfl.c: Intel Merrifield platform specific setup code
+ *
+ * (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.
+ */
+
+#include <linux/init.h>
+
+#include <asm/apic.h>
+#include <asm/intel-mid.h>
+
+#include "intel_mid_weak_decls.h"
+
+static unsigned long __init tangier_calibrate_tsc(void)
+{
+	unsigned long fast_calibrate;
+	u32 lo, hi, ratio, fsb, bus_freq;
+
+	/* *********************** */
+	/* Compute TSC:Ratio * FSB */
+	/* *********************** */
+
+	/* Compute Ratio */
+	rdmsr(MSR_PLATFORM_INFO, lo, hi);
+	pr_debug("IA32 PLATFORM_INFO is 0x%x : %x\n", hi, lo);
+
+	ratio = (lo >> 8) & 0xFF;
+	pr_debug("ratio is %d\n", ratio);
+	if (!ratio) {
+		pr_err("Read a zero ratio, force tsc ratio to 4 ...\n");
+		ratio = 4;
+	}
+
+	/* Compute FSB */
+	rdmsr(MSR_FSB_FREQ, lo, hi);
+	pr_debug("Actual FSB frequency detected by SOC 0x%x : %x\n",
+			hi, lo);
+
+	bus_freq = lo & 0x7;
+	pr_debug("bus_freq = 0x%x\n", bus_freq);
+
+	if (bus_freq == 0)
+		fsb = FSB_FREQ_100SKU;
+	else if (bus_freq == 1)
+		fsb = FSB_FREQ_100SKU;
+	else if (bus_freq == 2)
+		fsb = FSB_FREQ_133SKU;
+	else if (bus_freq == 3)
+		fsb = FSB_FREQ_167SKU;
+	else if (bus_freq == 4)
+		fsb = FSB_FREQ_83SKU;
+	else if (bus_freq == 5)
+		fsb = FSB_FREQ_400SKU;
+	else if (bus_freq == 6)
+		fsb = FSB_FREQ_267SKU;
+	else if (bus_freq == 7)
+		fsb = FSB_FREQ_333SKU;
+	else {
+		BUG();
+		pr_err("Invalid bus_freq! Setting to minimal value!\n");
+		fsb = FSB_FREQ_100SKU;
+	}
+
+	/* TSC = FSB Freq * Resolved HFM Ratio */
+	fast_calibrate = ratio * fsb;
+	pr_debug("calculate tangier tsc %lu KHz\n", fast_calibrate);
+
+	/* ************************************ */
+	/* Calculate Local APIC Timer Frequency */
+	/* ************************************ */
+	lapic_timer_frequency = (fsb * 1000) / HZ;
+
+	pr_debug("Setting lapic_timer_frequency = %d\n",
+			lapic_timer_frequency);
+
+	/* 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 tangier_arch_setup(void)
+{
+	x86_platform.calibrate_tsc = tangier_calibrate_tsc;
+}
+
+/* tangier arch ops */
+static struct intel_mid_ops tangier_ops = {
+	.arch_setup = tangier_arch_setup,
+};
+
+void * __cpuinit get_tangier_ops()
+{
+	return &tangier_ops;
+}
diff --git a/arch/x86/platform/intel-mid/sfi.c b/arch/x86/platform/intel-mid/sfi.c
index c84c1ca396bf..80a52288555c 100644
--- a/arch/x86/platform/intel-mid/sfi.c
+++ b/arch/x86/platform/intel-mid/sfi.c
@@ -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);
 
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs
  2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
                   ` (2 preceding siblings ...)
  2013-12-16 20:07 ` [PATCH v2 3/4] x86: intel-mid: add Merrifield " David Cohen
@ 2013-12-16 20:07 ` David Cohen
  2013-12-16 20:47   ` Bjorn Helgaas
  2013-12-17  1:37 ` [PATCH v2.1 " David Cohen
  2014-01-14 22:44 ` [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
  5 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2013-12-16 20:07 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: bhelgaas, david.a.cohen, linux-kernel, linux-pci

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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/Kconfig | 29 +++++------------------------
 1 file changed, 5 insertions(+), 24 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e903c71f7e69..b6a344fb01ae 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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"
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs
  2013-12-16 20:07 ` [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs David Cohen
@ 2013-12-16 20:47   ` Bjorn Helgaas
  2013-12-16 21:31     ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: Bjorn Helgaas @ 2013-12-16 20:47 UTC (permalink / raw)
  To: David Cohen
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-pci

On Mon, Dec 16, 2013 at 1:07 PM, David Cohen
<david.a.cohen@linux.intel.com> wrote:
> 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.
>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> ---
>  arch/x86/Kconfig | 29 +++++------------------------
>  1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index e903c71f7e69..b6a344fb01ae 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -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.

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

>  config X86_INTEL_LPSS
>         bool "Intel Low Power Subsystem Support"
> --
> 1.8.4.2
>

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs
  2013-12-16 20:47   ` Bjorn Helgaas
@ 2013-12-16 21:31     ` David Cohen
  0 siblings, 0 replies; 36+ messages in thread
From: David Cohen @ 2013-12-16 21:31 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-pci

Hi, Bjorn,

[snip]

> 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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2.1 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs
  2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
                   ` (3 preceding siblings ...)
  2013-12-16 20:07 ` [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs David Cohen
@ 2013-12-17  1:37 ` David Cohen
  2013-12-20  5:42   ` [PATCH 1/2] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
  2014-01-15 22:43   ` [tip:x86/intel-mid] x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs tip-bot for David Cohen
  2014-01-14 22:44 ` [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
  5 siblings, 2 replies; 36+ messages in thread
From: David Cohen @ 2013-12-17  1:37 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: bhelgaas, david.a.cohen, linux-kernel, linux-pci

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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/Kconfig | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e903c71f7e69..f998be2d2901 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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"
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 1/2] x86: intel-mid: return proper error code from get_gpio_by_name()
  2013-12-17  1:37 ` [PATCH v2.1 " David Cohen
@ 2013-12-20  5:42   ` David Cohen
  2013-12-20  5:42     ` [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
  2013-12-21  1:15     ` [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way David Cohen
  2014-01-15 22:43   ` [tip:x86/intel-mid] x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs tip-bot for David Cohen
  1 sibling, 2 replies; 36+ messages in thread
From: David Cohen @ 2013-12-20  5:42 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, David Cohen

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

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/sfi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/intel-mid/sfi.c b/arch/x86/platform/intel-mid/sfi.c
index 80a52288555c..c67b9a2f48a1 100644
--- a/arch/x86/platform/intel-mid/sfi.c
+++ b/arch/x86/platform/intel-mid/sfi.c
@@ -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)
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2013-12-20  5:42   ` [PATCH 1/2] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
@ 2013-12-20  5:42     ` David Cohen
  2013-12-20  8:49       ` Ingo Molnar
  2013-12-21  1:15     ` [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way David Cohen
  1 sibling, 1 reply; 36+ messages in thread
From: David Cohen @ 2013-12-20  5:42 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, 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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/sfi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/platform/intel-mid/sfi.c b/arch/x86/platform/intel-mid/sfi.c
index c67b9a2f48a1..438306ebed05 100644
--- a/arch/x86/platform/intel-mid/sfi.c
+++ b/arch/x86/platform/intel-mid/sfi.c
@@ -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);
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2013-12-20  5:42     ` [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
@ 2013-12-20  8:49       ` Ingo Molnar
  2013-12-20 17:40         ` David Cohen
  2014-01-15  0:21         ` David Cohen
  0 siblings, 2 replies; 36+ messages in thread
From: Ingo Molnar @ 2013-12-20  8:49 UTC (permalink / raw)
  To: David Cohen; +Cc: tglx, mingo, hpa, x86, linux-kernel


* David Cohen <david.a.cohen@linux.intel.com> wrote:

> 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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2013-12-20  8:49       ` Ingo Molnar
@ 2013-12-20 17:40         ` David Cohen
  2014-01-15  0:21         ` David Cohen
  1 sibling, 0 replies; 36+ messages in thread
From: David Cohen @ 2013-12-20 17:40 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: tglx, mingo, hpa, x86, linux-kernel

Hi Ingo,

On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> 
> * David Cohen <david.a.cohen@linux.intel.com> wrote:
> 
> > 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.

> 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.

I'll amend the description.

Br, David

> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way
  2013-12-20  5:42   ` [PATCH 1/2] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
  2013-12-20  5:42     ` [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
@ 2013-12-21  1:15     ` David Cohen
  2013-12-21  1:15       ` [PATCH v2 1/3] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
                         ` (2 more replies)
  1 sibling, 3 replies; 36+ messages in thread
From: David Cohen @ 2013-12-21  1:15 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, david.a.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(-)

-- 
1.8.4.2


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v2 1/3] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2013-12-21  1:15     ` [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way David Cohen
@ 2013-12-21  1:15       ` David Cohen
  2013-12-21  1:15       ` [PATCH v2 2/3] x86: intel-mid: platform code should return error when failing David Cohen
  2013-12-21  1:15       ` [PATCH v2 3/3] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
  2 siblings, 0 replies; 36+ messages in thread
From: David Cohen @ 2013-12-21  1:15 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, david.a.cohen

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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/sfi.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/platform/intel-mid/sfi.c b/arch/x86/platform/intel-mid/sfi.c
index 80a52288555c..1d5d20b088fa 100644
--- a/arch/x86/platform/intel-mid/sfi.c
+++ b/arch/x86/platform/intel-mid/sfi.c
@@ -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);
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 2/3] x86: intel-mid: platform code should return error when failing
  2013-12-21  1:15     ` [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way David Cohen
  2013-12-21  1:15       ` [PATCH v2 1/3] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
@ 2013-12-21  1:15       ` David Cohen
  2013-12-21  1:15       ` [PATCH v2 3/3] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
  2 siblings, 0 replies; 36+ messages in thread
From: David Cohen @ 2013-12-21  1:15 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, david.a.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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_emc1403.c b/arch/x86/platform/intel-mid/device_libs/platform_emc1403.c
index 0d942c1d26d5..58933daa606d 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_emc1403.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_emc1403.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c b/arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c
index a013a4834bbe..dccae6b0413f 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_gpio_keys.c
@@ -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)
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_lis331.c b/arch/x86/platform/intel-mid/device_libs/platform_lis331.c
index 15278c11f714..506856c418a6 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_lis331.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_lis331.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
index 94ade10024ae..d25ec6d4b606 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c b/arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c
index dd28d63c84fb..c0a8fe7791a0 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_mpu3050.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c b/arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c
index 2a4f7b1dd917..a3843c43ae70 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_msic_gpio.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c b/arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c
index 6497111ddb54..65aa98aee28f 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_msic_ocd.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c b/arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c
index d87182a09263..65c2a9a19db4 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_pmic_gpio.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
index 740fc757050c..53fb3ec7cf5d 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_tc35876x.c
@@ -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;
 }
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_tca6416.c b/arch/x86/platform/intel-mid/device_libs/platform_tca6416.c
index 22881c9a6737..e331b38871da 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_tca6416.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_tca6416.c
@@ -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 {
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v2 3/3] x86: intel-mid: return proper error code from get_gpio_by_name()
  2013-12-21  1:15     ` [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way David Cohen
  2013-12-21  1:15       ` [PATCH v2 1/3] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
  2013-12-21  1:15       ` [PATCH v2 2/3] x86: intel-mid: platform code should return error when failing David Cohen
@ 2013-12-21  1:15       ` David Cohen
  2 siblings, 0 replies; 36+ messages in thread
From: David Cohen @ 2013-12-21  1:15 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: linux-kernel, david.a.cohen

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

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
---
 arch/x86/platform/intel-mid/sfi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/platform/intel-mid/sfi.c b/arch/x86/platform/intel-mid/sfi.c
index 1d5d20b088fa..438306ebed05 100644
--- a/arch/x86/platform/intel-mid/sfi.c
+++ b/arch/x86/platform/intel-mid/sfi.c
@@ -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)
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID
  2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
                   ` (4 preceding siblings ...)
  2013-12-17  1:37 ` [PATCH v2.1 " David Cohen
@ 2014-01-14 22:44 ` David Cohen
  2014-01-14 23:52   ` H. Peter Anvin
  5 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-14 22:44 UTC (permalink / raw)
  To: tglx, mingo, hpa, x86; +Cc: bhelgaas, linux-kernel, linux-pci

On Mon, Dec 16, 2013 at 12:07:35PM -0800, David Cohen wrote:
> 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

Ping :)
Any comments on this whole patch set?

Br, David Cohen

> 
> 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
> 
> -- 
> 1.8.4.2

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID
  2014-01-14 22:44 ` [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
@ 2014-01-14 23:52   ` H. Peter Anvin
  2014-01-15  0:13     ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: H. Peter Anvin @ 2014-01-14 23:52 UTC (permalink / raw)
  To: David Cohen, tglx, mingo, x86; +Cc: bhelgaas, linux-kernel, linux-pci

On 01/14/2014 02:44 PM, David Cohen wrote:
> On Mon, Dec 16, 2013 at 12:07:35PM -0800, David Cohen wrote:
>> 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
> 
> 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



^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID
  2014-01-14 23:52   ` H. Peter Anvin
@ 2014-01-15  0:13     ` David Cohen
  0 siblings, 0 replies; 36+ messages in thread
From: David Cohen @ 2014-01-15  0:13 UTC (permalink / raw)
  To: H. Peter Anvin; +Cc: tglx, mingo, x86, bhelgaas, linux-kernel, linux-pci

On Tue, Jan 14, 2014 at 03:52:01PM -0800, H. Peter Anvin wrote:
> On 01/14/2014 02:44 PM, David Cohen wrote:
> > On Mon, Dec 16, 2013 at 12:07:35PM -0800, David Cohen wrote:
> >> 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
> > 
> > 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

> 
> Other than that we should queue it up ASAP.
> 
> 	-hpa
> 

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2013-12-20  8:49       ` Ingo Molnar
  2013-12-20 17:40         ` David Cohen
@ 2014-01-15  0:21         ` David Cohen
  2014-01-15  6:58           ` Ingo Molnar
  1 sibling, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-15  0:21 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: tglx, mingo, hpa, x86, linux-kernel

Hi Ingo,

On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> 
> * David Cohen <david.a.cohen@linux.intel.com> wrote:
> 
> > 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 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

> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2014-01-15  0:21         ` David Cohen
@ 2014-01-15  6:58           ` Ingo Molnar
  2014-01-15 17:39             ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: Ingo Molnar @ 2014-01-15  6:58 UTC (permalink / raw)
  To: David Cohen; +Cc: tglx, mingo, hpa, x86, linux-kernel


* David Cohen <david.a.cohen@linux.intel.com> wrote:

> Hi Ingo,
> 
> On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> > 
> > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > 
> > > 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 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:

> >  - 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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2014-01-15  6:58           ` Ingo Molnar
@ 2014-01-15 17:39             ` David Cohen
  2014-01-15 22:26               ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-15 17:39 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: tglx, mingo, hpa, x86, linux-kernel

On Wed, Jan 15, 2014 at 07:58:37AM +0100, Ingo Molnar wrote:
> 
> * David Cohen <david.a.cohen@linux.intel.com> wrote:
> 
> > Hi Ingo,
> > 
> > On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> > > 
> > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > 
> > > > 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 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:
> 
> > >  - 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.

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

> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2014-01-15 17:39             ` David Cohen
@ 2014-01-15 22:26               ` David Cohen
  2014-01-16  9:50                 ` Ingo Molnar
  0 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-15 22:26 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: tglx, mingo, hpa, x86, linux-kernel

Hi Ingo and hpa,

On Wed, Jan 15, 2014 at 09:39:52AM -0800, David Cohen wrote:
> On Wed, Jan 15, 2014 at 07:58:37AM +0100, Ingo Molnar wrote:
> > 
> > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > 
> > > Hi Ingo,
> > > 
> > > On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> > > > 
> > > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > > 
> > > > > 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 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:
> > 
> > > >  - 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.
> 
> 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 :)

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

> 
> Br, David Cohen
> 
> > 
> > Thanks,
> > 
> > 	Ingo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [tip:x86/intel-mid] x86, intel-mid: Move Medfield code out of intel-mid.c core file
  2013-12-16 20:07 ` [PATCH v2 1/4] x86: intel-mid: move Medfield code out of intel-mid.c core file David Cohen
@ 2014-01-15 22:42   ` tip-bot for David Cohen
  0 siblings, 0 replies; 36+ messages in thread
From: tip-bot for David Cohen @ 2014-01-15 22:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, sathyanarayanan.kuppuswamy,
	david.a.cohen, tglx, hpa

Commit-ID:  ecd6910db979bc40ac19f0e71e027132fc906068
Gitweb:     http://git.kernel.org/tip/ecd6910db979bc40ac19f0e71e027132fc906068
Author:     David Cohen <david.a.cohen@linux.intel.com>
AuthorDate: Mon, 16 Dec 2013 12:07:36 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Link: http://lkml.kernel.org/r/1387224459-25746-2-git-send-email-david.a.cohen@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/platform/intel-mid/Makefile b/arch/x86/platform/intel-mid/Makefile
index 01cc29e..78a14ba 100644
--- a/arch/x86/platform/intel-mid/Makefile
+++ b/arch/x86/platform/intel-mid/Makefile
@@ -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/
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index f90e290..527d6d5 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -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);
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
new file mode 100644
index 0000000..519beb7
--- /dev/null
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -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));
diff --git a/arch/x86/platform/intel-mid/mfld.c b/arch/x86/platform/intel-mid/mfld.c
new file mode 100644
index 0000000..c7ff83c
--- /dev/null
+++ b/arch/x86/platform/intel-mid/mfld.c
@@ -0,0 +1,51 @@
+/*
+ * mfld.c: Intel Medfield platform setup code
+ *
+ * (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.
+ */
+
+#include <linux/init.h>
+
+#include <asm/apic.h>
+#include <asm/intel-mid.h>
+#include <asm/intel_mid_vrtc.h>
+
+void intel_mid_power_off(void)
+{
+}
+
+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;
+}

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [tip:x86/intel-mid] x86, intel-mid: Add Clovertrail platform support
  2013-12-16 20:07 ` [PATCH v2 2/4] x86: intel-mid: add Clovertrail platform support David Cohen
@ 2014-01-15 22:42   ` tip-bot for Kuppuswamy Sathyanarayanan
  0 siblings, 0 replies; 36+ messages in thread
From: tip-bot for Kuppuswamy Sathyanarayanan @ 2014-01-15 22:42 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, fei.yang, david.a.cohen,
	sathyanarayanan.kuppuswamy, tglx, hpa

Commit-ID:  85611e3febe78955a519f5f9eb47b941525c8c76
Gitweb:     http://git.kernel.org/tip/85611e3febe78955a519f5f9eb47b941525c8c76
Author:     Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
AuthorDate: Mon, 16 Dec 2013 12:07:37 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Link: http://lkml.kernel.org/r/1387224459-25746-3-git-send-email-david.a.cohen@linux.intel.com
Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: Fei Yang <fei.yang@intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/include/asm/intel-mid.h b/arch/x86/include/asm/intel-mid.h
index 459769d..f8a8314 100644
--- a/arch/x86/include/asm/intel-mid.h
+++ b/arch/x86/include/asm/intel-mid.h
@@ -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
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 527d6d5..4095584 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -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 */
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 519beb7..9ebce04 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -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));
diff --git a/arch/x86/platform/intel-mid/mfld.c b/arch/x86/platform/intel-mid/mfld.c
index c7ff83c..4f7884e 100644
--- a/arch/x86/platform/intel-mid/mfld.c
+++ b/arch/x86/platform/intel-mid/mfld.c
@@ -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;
+}

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [tip:x86/intel-mid] x86, intel-mid: Add Merrifield platform support
  2013-12-16 20:07 ` [PATCH v2 3/4] x86: intel-mid: add Merrifield " David Cohen
@ 2014-01-15 22:43   ` tip-bot for David Cohen
  2014-01-28  0:52   ` [PATCH v2 3/4] x86: intel-mid: add " Bjorn Helgaas
  1 sibling, 0 replies; 36+ messages in thread
From: tip-bot for David Cohen @ 2014-01-15 22:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, fei.yang, sathyanarayanan.kuppuswamy,
	david.a.cohen, mark.f.brown, tglx, hpa

Commit-ID:  bc20aa48bbb3068224a1c91f8332971fdb689fad
Gitweb:     http://git.kernel.org/tip/bc20aa48bbb3068224a1c91f8332971fdb689fad
Author:     David Cohen <david.a.cohen@linux.intel.com>
AuthorDate: Mon, 16 Dec 2013 12:07:38 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Link: http://lkml.kernel.org/r/1387224459-25746-4-git-send-email-david.a.cohen@linux.intel.com
Signed-off-by: Fei Yang <fei.yang@intel.com>
Cc: Mark F. Brown <mark.f.brown@intel.com>
Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/include/asm/intel-mid.h b/arch/x86/include/asm/intel-mid.h
index f8a8314..e34e097 100644
--- a/arch/x86/include/asm/intel-mid.h
+++ b/arch/x86/include/asm/intel-mid.h
@@ -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
diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index 51384ca..84b9d67 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/Makefile b/arch/x86/platform/intel-mid/Makefile
index 78a14ba..0a8ee70 100644
--- a/arch/x86/platform/intel-mid/Makefile
+++ b/arch/x86/platform/intel-mid/Makefile
@@ -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
diff --git a/arch/x86/platform/intel-mid/intel-mid.c b/arch/x86/platform/intel-mid/intel-mid.c
index 4095584..1bbedc4 100644
--- a/arch/x86/platform/intel-mid/intel-mid.c
+++ b/arch/x86/platform/intel-mid/intel-mid.c
@@ -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;
diff --git a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
index 9ebce04..a537ffc 100644
--- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
+++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
@@ -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));
diff --git a/arch/x86/platform/intel-mid/mrfl.c b/arch/x86/platform/intel-mid/mrfl.c
new file mode 100644
index 0000000..09d1015
--- /dev/null
+++ b/arch/x86/platform/intel-mid/mrfl.c
@@ -0,0 +1,103 @@
+/*
+ * mrfl.c: Intel Merrifield platform specific setup code
+ *
+ * (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.
+ */
+
+#include <linux/init.h>
+
+#include <asm/apic.h>
+#include <asm/intel-mid.h>
+
+#include "intel_mid_weak_decls.h"
+
+static unsigned long __init tangier_calibrate_tsc(void)
+{
+	unsigned long fast_calibrate;
+	u32 lo, hi, ratio, fsb, bus_freq;
+
+	/* *********************** */
+	/* Compute TSC:Ratio * FSB */
+	/* *********************** */
+
+	/* Compute Ratio */
+	rdmsr(MSR_PLATFORM_INFO, lo, hi);
+	pr_debug("IA32 PLATFORM_INFO is 0x%x : %x\n", hi, lo);
+
+	ratio = (lo >> 8) & 0xFF;
+	pr_debug("ratio is %d\n", ratio);
+	if (!ratio) {
+		pr_err("Read a zero ratio, force tsc ratio to 4 ...\n");
+		ratio = 4;
+	}
+
+	/* Compute FSB */
+	rdmsr(MSR_FSB_FREQ, lo, hi);
+	pr_debug("Actual FSB frequency detected by SOC 0x%x : %x\n",
+			hi, lo);
+
+	bus_freq = lo & 0x7;
+	pr_debug("bus_freq = 0x%x\n", bus_freq);
+
+	if (bus_freq == 0)
+		fsb = FSB_FREQ_100SKU;
+	else if (bus_freq == 1)
+		fsb = FSB_FREQ_100SKU;
+	else if (bus_freq == 2)
+		fsb = FSB_FREQ_133SKU;
+	else if (bus_freq == 3)
+		fsb = FSB_FREQ_167SKU;
+	else if (bus_freq == 4)
+		fsb = FSB_FREQ_83SKU;
+	else if (bus_freq == 5)
+		fsb = FSB_FREQ_400SKU;
+	else if (bus_freq == 6)
+		fsb = FSB_FREQ_267SKU;
+	else if (bus_freq == 7)
+		fsb = FSB_FREQ_333SKU;
+	else {
+		BUG();
+		pr_err("Invalid bus_freq! Setting to minimal value!\n");
+		fsb = FSB_FREQ_100SKU;
+	}
+
+	/* TSC = FSB Freq * Resolved HFM Ratio */
+	fast_calibrate = ratio * fsb;
+	pr_debug("calculate tangier tsc %lu KHz\n", fast_calibrate);
+
+	/* ************************************ */
+	/* Calculate Local APIC Timer Frequency */
+	/* ************************************ */
+	lapic_timer_frequency = (fsb * 1000) / HZ;
+
+	pr_debug("Setting lapic_timer_frequency = %d\n",
+			lapic_timer_frequency);
+
+	/* 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 tangier_arch_setup(void)
+{
+	x86_platform.calibrate_tsc = tangier_calibrate_tsc;
+}
+
+/* tangier arch ops */
+static struct intel_mid_ops tangier_ops = {
+	.arch_setup = tangier_arch_setup,
+};
+
+void * __cpuinit get_tangier_ops()
+{
+	return &tangier_ops;
+}
diff --git a/arch/x86/platform/intel-mid/sfi.c b/arch/x86/platform/intel-mid/sfi.c
index c84c1ca..80a5228 100644
--- a/arch/x86/platform/intel-mid/sfi.c
+++ b/arch/x86/platform/intel-mid/sfi.c
@@ -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);
 

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [tip:x86/intel-mid] x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs
  2013-12-17  1:37 ` [PATCH v2.1 " David Cohen
  2013-12-20  5:42   ` [PATCH 1/2] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
@ 2014-01-15 22:43   ` tip-bot for David Cohen
  1 sibling, 0 replies; 36+ messages in thread
From: tip-bot for David Cohen @ 2014-01-15 22:43 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: linux-kernel, hpa, mingo, david.a.cohen, tglx, hpa

Commit-ID:  4cb9b00f42e07830310319a07e6c91413ee8153e
Gitweb:     http://git.kernel.org/tip/4cb9b00f42e07830310319a07e6c91413ee8153e
Author:     David Cohen <david.a.cohen@linux.intel.com>
AuthorDate: Mon, 16 Dec 2013 17:37:26 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Link: http://lkml.kernel.org/r/1387244246-20714-1-git-send-email-david.a.cohen@linux.intel.com
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 arch/x86/Kconfig | 30 +++++++-----------------------
 1 file changed, 7 insertions(+), 23 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0952ecd..edcf80e 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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"

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2014-01-15 22:26               ` David Cohen
@ 2014-01-16  9:50                 ` Ingo Molnar
  2014-01-16 17:23                   ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: Ingo Molnar @ 2014-01-16  9:50 UTC (permalink / raw)
  To: David Cohen; +Cc: tglx, mingo, hpa, x86, linux-kernel


* David Cohen <david.a.cohen@linux.intel.com> wrote:

> Hi Ingo and hpa,
> 
> On Wed, Jan 15, 2014 at 09:39:52AM -0800, David Cohen wrote:
> > On Wed, Jan 15, 2014 at 07:58:37AM +0100, Ingo Molnar wrote:
> > > 
> > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > 
> > > > Hi Ingo,
> > > > 
> > > > On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> > > > > 
> > > > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > > > 
> > > > > > 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 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:
> > > 
> > > > >  - 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.
> > 
> > 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 :)
> 
> 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.

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

> 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.

> 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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2014-01-16  9:50                 ` Ingo Molnar
@ 2014-01-16 17:23                   ` David Cohen
  2014-01-16 23:35                     ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-16 17:23 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: tglx, mingo, hpa, x86, linux-kernel

On Thu, Jan 16, 2014 at 10:50:06AM +0100, Ingo Molnar wrote:
> 
> * David Cohen <david.a.cohen@linux.intel.com> wrote:
> 
> > Hi Ingo and hpa,
> > 
> > On Wed, Jan 15, 2014 at 09:39:52AM -0800, David Cohen wrote:
> > > On Wed, Jan 15, 2014 at 07:58:37AM +0100, Ingo Molnar wrote:
> > > > 
> > > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > > 
> > > > > Hi Ingo,
> > > > > 
> > > > > On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> > > > > > 
> > > > > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > > > > 
> > > > > > > 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 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:
> > > > 
> > > > > >  - 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.
> > > 
> > > 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 :)
> > 
> > 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.
> 
> 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.

> 
> > 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.

> 
> > 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

> 
> Thanks,
> 
> 	Ingo

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code
  2014-01-16 17:23                   ` David Cohen
@ 2014-01-16 23:35                     ` David Cohen
  0 siblings, 0 replies; 36+ messages in thread
From: David Cohen @ 2014-01-16 23:35 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: tglx, mingo, hpa, x86, linux-kernel

On Thu, Jan 16, 2014 at 09:23:54AM -0800, David Cohen wrote:
> On Thu, Jan 16, 2014 at 10:50:06AM +0100, Ingo Molnar wrote:
> > 
> > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > 
> > > Hi Ingo and hpa,
> > > 
> > > On Wed, Jan 15, 2014 at 09:39:52AM -0800, David Cohen wrote:
> > > > On Wed, Jan 15, 2014 at 07:58:37AM +0100, Ingo Molnar wrote:
> > > > > 
> > > > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > > > 
> > > > > > Hi Ingo,
> > > > > > 
> > > > > > On Fri, Dec 20, 2013 at 09:49:53AM +0100, Ingo Molnar wrote:
> > > > > > > 
> > > > > > > * David Cohen <david.a.cohen@linux.intel.com> wrote:
> > > > > > > 
> > > > > > > > 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 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:
> > > > > 
> > > > > > >  - 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.
> > > > 
> > > > 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 :)
> > > 
> > > 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.
> > 
> > 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.
> 
> > 
> > > 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.
> 
> > 
> > > 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.

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


^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support
  2013-12-16 20:07 ` [PATCH v2 3/4] x86: intel-mid: add Merrifield " David Cohen
  2014-01-15 22:43   ` [tip:x86/intel-mid] x86, intel-mid: Add " tip-bot for David Cohen
@ 2014-01-28  0:52   ` Bjorn Helgaas
  2014-01-28  1:30     ` David Cohen
  1 sibling, 1 reply; 36+ messages in thread
From: Bjorn Helgaas @ 2014-01-28  0:52 UTC (permalink / raw)
  To: David Cohen
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-pci, Fei Yang, Mark F. Brown, Kuppuswamy Sathyanarayanan

On Mon, Dec 16, 2013 at 1:07 PM, David Cohen
<david.a.cohen@linux.intel.com> wrote:
> This code was partially based on Mark Brown's previous work.
>
> Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> Signed-off-by: Fei Yang <fei.yang@intel.com>
> Cc: Mark F. Brown <mark.f.brown@intel.com>
> Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>

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

> --- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> +++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> @@ -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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support
  2014-01-28  0:52   ` [PATCH v2 3/4] x86: intel-mid: add " Bjorn Helgaas
@ 2014-01-28  1:30     ` David Cohen
  2014-01-28 18:40       ` Bjorn Helgaas
  0 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-28  1:30 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-pci, Fei Yang, Mark F. Brown, Kuppuswamy Sathyanarayanan

Hi Bjorn,

On Mon, Jan 27, 2014 at 05:52:30PM -0700, Bjorn Helgaas wrote:
> On Mon, Dec 16, 2013 at 1:07 PM, David Cohen
> <david.a.cohen@linux.intel.com> wrote:
> > This code was partially based on Mark Brown's previous work.
> >
> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> > Signed-off-by: Fei Yang <fei.yang@intel.com>
> > Cc: Mark F. Brown <mark.f.brown@intel.com>
> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> I know this has already been merged to Linus' tree, but it looks funny to me.
> 
> > --- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> > +++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> > @@ -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.

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 <david.a.cohen@linux.intel.com>
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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support
  2014-01-28  1:30     ` David Cohen
@ 2014-01-28 18:40       ` Bjorn Helgaas
  2014-01-28 19:35         ` David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: Bjorn Helgaas @ 2014-01-28 18:40 UTC (permalink / raw)
  To: David Cohen
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-pci, Fei Yang, Mark F. Brown, Kuppuswamy Sathyanarayanan

On Mon, Jan 27, 2014 at 6:30 PM, David Cohen
<david.a.cohen@linux.intel.com> wrote:
> Hi Bjorn,
>
> On Mon, Jan 27, 2014 at 05:52:30PM -0700, Bjorn Helgaas wrote:
>> On Mon, Dec 16, 2013 at 1:07 PM, David Cohen
>> <david.a.cohen@linux.intel.com> wrote:
>> > This code was partially based on Mark Brown's previous work.
>> >
>> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
>> > Signed-off-by: Fei Yang <fei.yang@intel.com>
>> > Cc: Mark F. Brown <mark.f.brown@intel.com>
>> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
>>
>> I know this has already been merged to Linus' tree, but it looks funny to me.
>>
>> > --- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
>> > +++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
>> > @@ -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.
>
> 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 <david.a.cohen@linux.intel.com>
> 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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v2 3/4] x86: intel-mid: add Merrifield platform support
  2014-01-28 18:40       ` Bjorn Helgaas
@ 2014-01-28 19:35         ` David Cohen
  2014-01-28 23:09           ` [PATCH] x86: intel-mid: cleanup some platform code's header files David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-28 19:35 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86, linux-kernel,
	linux-pci, Fei Yang, Mark F. Brown, Kuppuswamy Sathyanarayanan

On Tue, Jan 28, 2014 at 11:40:57AM -0700, Bjorn Helgaas wrote:
> On Mon, Jan 27, 2014 at 6:30 PM, David Cohen
> <david.a.cohen@linux.intel.com> wrote:
> > Hi Bjorn,
> >
> > On Mon, Jan 27, 2014 at 05:52:30PM -0700, Bjorn Helgaas wrote:
> >> On Mon, Dec 16, 2013 at 1:07 PM, David Cohen
> >> <david.a.cohen@linux.intel.com> wrote:
> >> > This code was partially based on Mark Brown's previous work.
> >> >
> >> > Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
> >> > Signed-off-by: Fei Yang <fei.yang@intel.com>
> >> > Cc: Mark F. Brown <mark.f.brown@intel.com>
> >> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
> >>
> >> I know this has already been merged to Linus' tree, but it looks funny to me.
> >>
> >> > --- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> >> > +++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h
> >> > @@ -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.
> >
> > 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 <david.a.cohen@linux.intel.com>
> > 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.

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

> 
> Bjorn

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH] x86: intel-mid: cleanup some platform code's header files
  2014-01-28 19:35         ` David Cohen
@ 2014-01-28 23:09           ` David Cohen
  2014-01-28 23:15             ` [tip:x86/intel-mid] x86, intel-mid: Cleanup some platform code' s " tip-bot for David Cohen
  0 siblings, 1 reply; 36+ messages in thread
From: David Cohen @ 2014-01-28 23:09 UTC (permalink / raw)
  To: hpa, mingo, tglx, x86; +Cc: linux-kernel, David Cohen, Bjorn Helgaas

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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
---
 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(-)

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_ipc.h b/arch/x86/platform/intel-mid/device_libs/platform_ipc.h
index 8f568dd79605..79bb09d4f718 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_ipc.h
+++ b/arch/x86/platform/intel-mid/device_libs/platform_ipc.h
@@ -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
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_msic.h b/arch/x86/platform/intel-mid/device_libs/platform_msic.h
index 917eb56d77da..b7be1d041da2 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_msic.h
+++ b/arch/x86/platform/intel-mid/device_libs/platform_msic.h
@@ -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
-- 
1.8.4.2


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [tip:x86/intel-mid] x86, intel-mid: Cleanup some platform code' s header files
  2014-01-28 23:09           ` [PATCH] x86: intel-mid: cleanup some platform code's header files David Cohen
@ 2014-01-28 23:15             ` tip-bot for David Cohen
  0 siblings, 0 replies; 36+ messages in thread
From: tip-bot for David Cohen @ 2014-01-28 23:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, bhelgaas, david.a.cohen, tglx, hpa

Commit-ID:  790ed7421a973d9773dda8b4e5300c3f7f6fbcf7
Gitweb:     http://git.kernel.org/tip/790ed7421a973d9773dda8b4e5300c3f7f6fbcf7
Author:     David Cohen <david.a.cohen@linux.intel.com>
AuthorDate: Tue, 28 Jan 2014 15:09:27 -0800
Committer:  H. Peter Anvin <hpa@linux.intel.com>
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.

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Link: http://lkml.kernel.org/r/1390950567-12821-1-git-send-email-david.a.cohen@linux.intel.com
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: H. Peter Anvin <hpa@linux.intel.com>
---
 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(-)

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_ipc.h b/arch/x86/platform/intel-mid/device_libs/platform_ipc.h
index 8f568dd..79bb09d 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_ipc.h
+++ b/arch/x86/platform/intel-mid/device_libs/platform_ipc.h
@@ -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
diff --git a/arch/x86/platform/intel-mid/device_libs/platform_msic.h b/arch/x86/platform/intel-mid/device_libs/platform_msic.h
index 917eb56..b7be1d0 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_msic.h
+++ b/arch/x86/platform/intel-mid/device_libs/platform_msic.h
@@ -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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2014-01-28 23:15 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-16 20:07 [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
2013-12-16 20:07 ` [PATCH v2 1/4] x86: intel-mid: move Medfield code out of intel-mid.c core file David Cohen
2014-01-15 22:42   ` [tip:x86/intel-mid] x86, intel-mid: Move " tip-bot for David Cohen
2013-12-16 20:07 ` [PATCH v2 2/4] x86: intel-mid: add Clovertrail platform support David Cohen
2014-01-15 22:42   ` [tip:x86/intel-mid] x86, intel-mid: Add " tip-bot for Kuppuswamy Sathyanarayanan
2013-12-16 20:07 ` [PATCH v2 3/4] x86: intel-mid: add Merrifield " David Cohen
2014-01-15 22:43   ` [tip:x86/intel-mid] x86, intel-mid: Add " tip-bot for David Cohen
2014-01-28  0:52   ` [PATCH v2 3/4] x86: intel-mid: add " Bjorn Helgaas
2014-01-28  1:30     ` David Cohen
2014-01-28 18:40       ` Bjorn Helgaas
2014-01-28 19:35         ` David Cohen
2014-01-28 23:09           ` [PATCH] x86: intel-mid: cleanup some platform code's header files David Cohen
2014-01-28 23:15             ` [tip:x86/intel-mid] x86, intel-mid: Cleanup some platform code' s " tip-bot for David Cohen
2013-12-16 20:07 ` [PATCH v2 4/4] x86: intel-mid: remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs David Cohen
2013-12-16 20:47   ` Bjorn Helgaas
2013-12-16 21:31     ` David Cohen
2013-12-17  1:37 ` [PATCH v2.1 " David Cohen
2013-12-20  5:42   ` [PATCH 1/2] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
2013-12-20  5:42     ` [PATCH 2/2] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
2013-12-20  8:49       ` Ingo Molnar
2013-12-20 17:40         ` David Cohen
2014-01-15  0:21         ` David Cohen
2014-01-15  6:58           ` Ingo Molnar
2014-01-15 17:39             ` David Cohen
2014-01-15 22:26               ` David Cohen
2014-01-16  9:50                 ` Ingo Molnar
2014-01-16 17:23                   ` David Cohen
2014-01-16 23:35                     ` David Cohen
2013-12-21  1:15     ` [PATCH v2 0/3] x86: intel-mid: handle platform code error in better way David Cohen
2013-12-21  1:15       ` [PATCH v2 1/3] x86: intel-mid: sfi_handle_*_dev() should check for pdata error code David Cohen
2013-12-21  1:15       ` [PATCH v2 2/3] x86: intel-mid: platform code should return error when failing David Cohen
2013-12-21  1:15       ` [PATCH v2 3/3] x86: intel-mid: return proper error code from get_gpio_by_name() David Cohen
2014-01-15 22:43   ` [tip:x86/intel-mid] x86, intel-mid: Remove deprecated X86_MDFLD and X86_WANT_INTEL_MID configs tip-bot for David Cohen
2014-01-14 22:44 ` [PATCH v2 0/4] Add Clovertrail and Merrifeld support to Intel MID David Cohen
2014-01-14 23:52   ` H. Peter Anvin
2014-01-15  0:13     ` David Cohen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).