RE: [PATCH/RFC V2 10/16] scsi: ufs: add UFS power management support
-Original Message-
From: linux-scsi-ow...@vger.kernel.org
[mailto:linux-scsi-ow...@vger.kernel.org] On Behalf Of Dolev Raviv
Sent: Thursday, August 14, 2014 9:31 PM
To: james.bottom...@hansenpartnership.com; h...@infradead.org
Cc: linux-s...@vger.kernel.org; linux-scsi-ow...@vger.kernel.org;
linux-arm-msm@vger.kernel.org; santos...@gmail.com; Subhash Jadavani;
Dolev Raviv; Sujit Reddy Thumma
Subject: [PATCH/RFC V2 10/16] scsi: ufs: add UFS power management support
From: Subhash Jadavani subha...@codeaurora.org
This patch adds support for UFS device and UniPro link power management
during runtime/system PM.
Main idea is to define multiple UFS low power levels based on UFS device and
UFS link power states. This would allow any specific platform or pci driver to
choose the best suited low power level during runtime and system suspend
based on their power goals.
bkops handlig:
To put the UFS device in sleep state when bkops is disabled, first query the
bkops status from the device and enable bkops on device only if device needs
time to perform the bkops.
START_STOP handling:
Before sending START_STOP_UNIT to the device well-known logical unit
(w-lun) to make sure that the device w-lun unit attention condition is
cleared.
Write protection:
UFS device specification allows LUs to be write protected, either permanently
or power on write protected. If any LU is power on write protected and if the
card is power cycled (by powering off VCCQ and/or VCC rails), LU's write
protect status would be lost. So this means those LUs can be written now. To
ensures that UFS device is power cycled only if the power on protect is not
set
for any of the LUs, check if power on write protect is set and if device is in
sleep/power-off state link in inactive state (Hibern8 or OFF state).
If none of the Logical Units on UFS device is power on write protected then
all
UFS device power rails (VCC, VCCQ VCCQ2) can be turned off if UFS device is
in power-off state and UFS link is in OFF state. But current implementation
would disable all device power rails even if UFS link is not in OFF state.
Signed-off-by: Subhash Jadavani subha...@codeaurora.org
Signed-off-by: Dolev Raviv dra...@codeaurora.org
Signed-off-by: Sujit Reddy Thumma sthu...@codeaurora.org
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index
bcc3a7f..2a82959 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -129,6 +129,7 @@ enum {
/* Flag idn for Query Requests*/
enum flag_idn {
QUERY_FLAG_IDN_FDEVICEINIT = 0x01,
+ QUERY_FLAG_IDN_PWR_ON_WPE = 0x03,
QUERY_FLAG_IDN_BKOPS_EN = 0x04,
};
@@ -194,6 +195,18 @@ enum unit_desc_param {
UNIT_DESC_PARAM_LARGE_UNIT_SIZE_M1 = 0x22,
};
+/*
+ * Logical Unit Write Protect
+ * 00h: LU not write protected
+ * 01h: LU write protected when fPowerOnWPEn =1
+ * 02h: LU permanently write protected when fPermanentWPEn =1 */ enum
+ufs_lu_wp_type {
+ UFS_LU_NO_WP= 0x00,
+ UFS_LU_POWER_ON_WP = 0x01,
+ UFS_LU_PERM_WP = 0x02,
+};
+
/* bActiveICCLevel parameter current units */ enum {
UFSHCD_NANO_AMP = 0,
@@ -226,11 +239,12 @@ enum {
};
/* Background operation status */
-enum {
+enum bkops_status {
BKOPS_STATUS_NO_OP = 0x0,
BKOPS_STATUS_NON_CRITICAL= 0x1,
BKOPS_STATUS_PERF_IMPACT = 0x2,
BKOPS_STATUS_CRITICAL= 0x3,
+ BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL,
};
/* UTP QUERY Transaction Specific Fields OpCode */ @@ -291,6 +305,14 @@
enum {
UPIU_TASK_MANAGEMENT_FUNC_FAILED= 0x05,
UPIU_INCORRECT_LOGICAL_UNIT_NO = 0x09,
};
+
+/* UFS device power modes */
+enum ufs_dev_pwr_mode {
+ UFS_ACTIVE_PWR_MODE = 1,
+ UFS_SLEEP_PWR_MODE = 2,
+ UFS_POWERDOWN_PWR_MODE = 3,
+};
+
/**
* struct utp_upiu_header - UPIU header structure
* @dword_0: UPIU header DW-0
@@ -437,6 +459,12 @@ struct ufs_query_res {
#define UFS_VREG_VCCQ2_MIN_UV 165 /* uV */
#define UFS_VREG_VCCQ2_MAX_UV 195 /* uV */
+/*
+ * VCCQ VCCQ2 current requirement when UFS device is in sleep state
+ * and link is in Hibern8 state.
+ */
+#define UFS_VREG_LPM_LOAD_UA 1000 /* uA */
+
struct ufs_vreg {
struct regulator *reg;
const char *name;
@@ -453,4 +481,10 @@ struct ufs_vreg_info {
struct ufs_vreg *vccq2;
};
+struct ufs_dev_info {
+ bool f_power_on_wp_en;
+ /* Keeps information if any of the LU is power on write protected */
+ bool is_lu_power_on_wp;
+};
+
#endif /* End of Header */
diff --git a/drivers/scsi/ufs/ufshcd-pci.c b/drivers/scsi/ufs/ufshcd-pci.c
index
1aac2ef..b8f7774 100644
--- a/drivers/scsi/ufs/ufshcd-pci.c
+++ b/drivers/scsi/ufs/ufshcd-pci.c
@@ -43,34 +43,24 @@
* @pdev: pointer to PCI device
Re: [PATCH v3 2/6] pinctrl: Introduce pinctrl driver for Qualcomm SSBI PMIC's
Hi Bjorn,
Two things which I noticed while trying out this driver to drive a reset
line.
1 gpio numbering for pinconf vs gpio are not consistent, they differ by
an offset of 1.
For example to control GPIO43 I had to do something like this in pinconf:
wlan_default_gpios: wlan-gpios {
pios {
pins = gpio43;
function = normal;
bias-disable;
power-source = PM8921_GPIO_S4;
};
};
for same pin for gpio I had to do:
reset-gpio = pm8921_gpio 42 GPIO_ACTIVE_LOW;
This offset by 1 is bit confusing and can easily lead to errors which
are hard to find.
I think this should be easy to fix..
2 Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set
to enable gpio mode. without this bit driver does not work for output pins.
here is the patch which fixes this:
From 5a5c5171a3371cf38ccd157922ea140bfd0253d5 Mon Sep 17 00:00:00 2001
From: Srinivas Kandagatla srinivas.kandaga...@linaro.org
Date: Wed, 20 Aug 2014 07:18:03 +0100
Subject: [PATCH] pinctrl:qcom:ssbi: Enable gpio mode
Looking back at 3.4 kernel driver, it looks like the gpio enable bit is
missing in this driver.
This patch is just a hack to get the wlan reset line working.
Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org
---
drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
b/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
index 27e5ec9..2633ea1 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
@@ -49,6 +49,7 @@
#define SSBI_REG_ADDR_GPIO_BASE0x150
#define SSBI_REG_ADDR_GPIO(n) (SSBI_REG_ADDR_GPIO_BASE + n)
+#definePM8XXX_GPIO_MODE_ENABLE BIT(0)
#define PM8XXX_GPIO_WRITE BIT(7)
#define PM8XXX_MAX_GPIOS 44
@@ -493,7 +494,8 @@ static int pm8xxx_gpio_config_set(struct pinctrl_dev
*pctldev,
}
if (banks BIT(0))
- pm8xxx_gpio_write(pctrl, offset, 0, pin-power_source 1);
+ pm8xxx_gpio_write(pctrl, offset, 0, pin-power_source 1 |
+ PM8XXX_GPIO_MODE_ENABLE);
if (banks BIT(1)) {
val = pin-direction 2;
--
2.0.2
with this change am able to reset WLAN chip on IFC6410 which is
connected to gpio43 of pmic.
thanks,
srini
On 11/08/14 16:40, Ivan T. Ivanov wrote:
From: Bjorn Andersson bjorn.anders...@sonymobile.com
This introduces a pinctrl, pinconf, pinmux and gpio driver for the gpio
block found in pm8018, pm8038, pm8058, pm8917 and pm8921 pmics from
Qualcomm.
Signed-off-by: Bjorn Andersson bjorn.anders...@sonymobile.com
Signed-off-by: Ivan T. Ivanov iiva...@mm-sol.com
---
drivers/pinctrl/qcom/Kconfig | 11 +
drivers/pinctrl/qcom/Makefile| 1 +
drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c | 870 +++
3 files changed, 882 insertions(+)
create mode 100644 drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
diff --git a/drivers/pinctrl/qcom/Kconfig b/drivers/pinctrl/qcom/Kconfig
index d160a71..6bd4e4d 100644
--- a/drivers/pinctrl/qcom/Kconfig
+++ b/drivers/pinctrl/qcom/Kconfig
@@ -39,4 +39,15 @@ config PINCTRL_MSM8X74
This is the pinctrl, pinmux, pinconf and gpiolib driver for the
Qualcomm TLMM block found in the Qualcomm 8974 platform.
+config PINCTRL_SSBI_PMIC
+ tristate Qualcomm SSBI PMIC pin controller driver
+ depends on GPIOLIB OF
+ select PINCONF
+ select PINMUX
+ select GENERIC_PINCONF
+ help
+ This is the pinctrl, pinmux, pinconf and gpiolib driver for the
+ Qualcomm GPIO blocks found in the pm8018, pm8038, pm8058, pm8917 and
+ pm8921 pmics from Qualcomm.
+
endif
diff --git a/drivers/pinctrl/qcom/Makefile b/drivers/pinctrl/qcom/Makefile
index 2a02602..8faa2ca 100644
--- a/drivers/pinctrl/qcom/Makefile
+++ b/drivers/pinctrl/qcom/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_PINCTRL_APQ8064) += pinctrl-apq8064.o
obj-$(CONFIG_PINCTRL_IPQ8064) += pinctrl-ipq8064.o
obj-$(CONFIG_PINCTRL_MSM8960) += pinctrl-msm8960.o
obj-$(CONFIG_PINCTRL_MSM8X74) += pinctrl-msm8x74.o
+obj-$(CONFIG_PINCTRL_SSBI_PMIC) += pinctrl-ssbi-pmic.o
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
b/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
new file mode 100644
index 000..9a1b443
--- /dev/null
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c
@@ -0,0 +1,870 @@
+/*
+ * Copyright (c) 2014, Sony Mobile Communications AB.
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even
Re: [PATCH v3 2/2] ksm: provide support to use deferrable timers for scanner thread
Hi Hugh, + unsigned long enable; + int err; + + err = kstrtoul(buf, 10,enable); + if (err 0) + return err; + if (enable= 1) + return -EINVAL; I haven't studied the patch itself, I'm still worrying about the concept. But this caught my eye just before hitting Send: I don't think we need a tunable which only accepts the value 0 ;) Okay. I can correct this to accept any non-zero value. Is that okay ? I missed that to reply earlier. This was suggested by Andrew. And I think that is okay as displaying any non-zero value to user via this knob may not be completely right. + use_deferrable_timer = enable; -- Chintan Pandya QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] ARM: apq8064: Add pinmux and i2c pinctrl nodes
This patch adds pinmux and i2c pinctrl DT node for IFC6410 board.
It also adds necessary DT support for i2c eeprom which is present on
IFC6410.
Tested on IFC6410 board.
Signed-off-by: Kiran Padwal kiran.pad...@smartplayin.com
---
Chages since v1:
- Renamed pinmux phandle qcom_pinmux to tlmm_pinmux.
- Updated pinmux interrupt.
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 29 ++
arch/arm/boot/dts/qcom-apq8064.dtsi| 59
2 files changed, 88 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index 7c2441d..d52ac3c 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -5,6 +5,15 @@
compatible = qcom,apq8064-ifc6410, qcom,apq8064;
soc {
+ pinmux@80 {
+ i2c1_pins: i2c1_pinmux {
+ mux {
+ pins = gpio20, gpio21;
+ function = gsbi1;
+ };
+ };
+ };
+
gsbi@1660 {
status = ok;
qcom,mode = GSBI_PROT_I2C_UART;
@@ -12,5 +21,25 @@
status = ok;
};
};
+
+ gsbi1: gsbi@1244 {
+ qcom,mode = GSBI_PROT_I2C;
+ status = ok;
+
+ i2c1: i2c@1246 {
+ status = ok;
+
+ clock-frequency = 20;
+
+ pinctrl-0 = i2c1_pins;
+ pinctrl-names = default;
+
+ eeprom: eeprom@52 {
+ compatible = atmel,24c128;
+ reg = 0x52;
+ pagesize = 32;
+ };
+ };
+ };
};
};
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 92bf793..bb2ccde 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -70,6 +70,17 @@
ranges;
compatible = simple-bus;
+ tlmm_pinmux: pinmux@80 {
+ compatible = qcom,apq8064-pinctrl;
+ reg = 0x80 0x4000;
+
+ gpio-controller;
+ #gpio-cells = 2;
+ interrupt-controller;
+ #interrupt-cells = 2;
+ interrupts = 0 16 0x4;
+ };
+
intc: interrupt-controller@200 {
compatible = qcom,msm-qgic2;
interrupt-controller;
@@ -133,6 +144,54 @@
regulator;
};
+ gsbi1: gsbi@1244 {
+ compatible = qcom,gsbi-v1.0.0;
+ reg = 0x1244 0x100;
+ clocks = gcc GSBI1_H_CLK;
+ clock-names = iface;
+ #address-cells = 1;
+ #size-cells = 1;
+ ranges;
+ status = disabled;
+
+ i2c@1246 {
+ compatible = qcom,i2c-qup-v1.1.1;
+ reg = 0x1246 0x1000;
+ interrupts = 0 194 0;
+
+ clocks = gcc GSBI1_QUP_CLK, gcc
GSBI1_H_CLK;
+ clock-names = core, iface;
+ status = disabled;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ };
+ };
+
+ gsbi2: gsbi@1248 {
+ compatible = qcom,gsbi-v1.0.0;
+ reg = 0x1248 0x100;
+ clocks = gcc GSBI2_H_CLK;
+ clock-names = iface;
+ #address-cells = 1;
+ #size-cells = 1;
+ ranges;
+ status = disabled;
+
+ i2c@124a {
+ compatible = qcom,i2c-qup-v1.1.1;
+ reg = 0x124a 0x1000;
+ interrupts = 0 196 0;
+
+ clocks = gcc GSBI2_QUP_CLK, gcc
GSBI2_H_CLK;
+ clock-names = core, iface;
+ status = disabled;
+
+ #address-cells = 1;
+ #size-cells = 0;
+ };
+ };
+
gsbi7: gsbi@1660 {
[PATCH v4 1/2] timer: provide an api for deferrable timeout
schedule_timeout wakes up the CPU from IDLE state. For some use cases it
is not desirable, hence introduce a convenient API
(schedule_timeout_deferrable_interruptible) on similar pattern which uses
a deferrable timer.
Signed-off-by: Chintan Pandya cpan...@codeaurora.org
Cc: Thomas Gleixner t...@linutronix.de
Cc: John Stultz john.stu...@linaro.org
Cc: Peter Zijlstra pet...@infradead.org
Cc: Ingo Molnar mi...@redhat.com
Cc: Hugh Dickins hu...@google.com
---
Changes:
V3--V4:
- No change
V2--V3:
- Big comment moved from static function to exported function
- Using __setup_timer_on_stack for better readability
V2:
- this patch has been newly introduced in patch v2
include/linux/sched.h | 2 ++
kernel/time/timer.c | 73 +++
2 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 5c2c885..13fe361 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -376,6 +376,8 @@ extern int in_sched_functions(unsigned long addr);
#defineMAX_SCHEDULE_TIMEOUTLONG_MAX
extern signed long schedule_timeout(signed long timeout);
extern signed long schedule_timeout_interruptible(signed long timeout);
+extern signed long
+schedule_timeout_deferrable_interruptible(signed long timeout);
extern signed long schedule_timeout_killable(signed long timeout);
extern signed long schedule_timeout_uninterruptible(signed long timeout);
asmlinkage void schedule(void);
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index aca5dfe..f4c4082 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1431,33 +1431,8 @@ static void process_timeout(unsigned long __data)
wake_up_process((struct task_struct *)__data);
}
-/**
- * schedule_timeout - sleep until timeout
- * @timeout: timeout value in jiffies
- *
- * Make the current task sleep until @timeout jiffies have
- * elapsed. The routine will return immediately unless
- * the current task state has been set (see set_current_state()).
- *
- * You can set the task state as follows -
- *
- * %TASK_UNINTERRUPTIBLE - at least @timeout jiffies are guaranteed to
- * pass before the routine returns. The routine will return 0
- *
- * %TASK_INTERRUPTIBLE - the routine may return early if a signal is
- * delivered to the current task. In this case the remaining time
- * in jiffies will be returned, or 0 if the timer expired in time
- *
- * The current task state is guaranteed to be TASK_RUNNING when this
- * routine returns.
- *
- * Specifying a @timeout value of %MAX_SCHEDULE_TIMEOUT will schedule
- * the CPU away without a bound on the timeout. In this case the return
- * value will be %MAX_SCHEDULE_TIMEOUT.
- *
- * In all cases the return value is guaranteed to be non-negative.
- */
-signed long __sched schedule_timeout(signed long timeout)
+static signed long
+__sched __schedule_timeout(signed long timeout, unsigned long flag)
{
struct timer_list timer;
unsigned long expire;
@@ -1493,7 +1468,9 @@ signed long __sched schedule_timeout(signed long timeout)
expire = timeout + jiffies;
- setup_timer_on_stack(timer, process_timeout, (unsigned long)current);
+ __setup_timer_on_stack(timer, process_timeout, (unsigned long)current,
+ flag);
+
__mod_timer(timer, expire, false, TIMER_NOT_PINNED);
schedule();
del_singleshot_timer_sync(timer);
@@ -1506,12 +1483,52 @@ signed long __sched schedule_timeout(signed long
timeout)
out:
return timeout 0 ? 0 : timeout;
}
+
+/**
+ * schedule_timeout - sleep until timeout
+ * @timeout: timeout value in jiffies
+ *
+ * Make the current task sleep until @timeout jiffies have
+ * elapsed. The routine will return immediately unless
+ * the current task state has been set (see set_current_state()).
+ *
+ * You can set the task state as follows -
+ *
+ * %TASK_UNINTERRUPTIBLE - at least @timeout jiffies are guaranteed to
+ * pass before the routine returns. The routine will return 0
+ *
+ * %TASK_INTERRUPTIBLE - the routine may return early if a signal is
+ * delivered to the current task. In this case the remaining time
+ * in jiffies will be returned, or 0 if the timer expired in time
+ *
+ * The current task state is guaranteed to be TASK_RUNNING when this
+ * routine returns.
+ *
+ * Specifying a @timeout value of %MAX_SCHEDULE_TIMEOUT will schedule
+ * the CPU away without a bound on the timeout. In this case the return
+ * value will be %MAX_SCHEDULE_TIMEOUT.
+ *
+ * In all cases the return value is guaranteed to be non-negative.
+ */
+signed long __sched schedule_timeout(signed long timeout)
+{
+ return __schedule_timeout(timeout, 0);
+}
EXPORT_SYMBOL(schedule_timeout);
/*
* We can use __set_current_state() here because schedule_timeout() calls
* schedule() unconditionally.
*/
+
+signed long
+__sched schedule_timeout_deferrable_interruptible(signed long
[PATCH v4 2/2] ksm: provide support to use deferrable timers for scanner thread
KSM thread to scan pages is scheduled on definite timeout. That wakes up
CPU from idle state and hence may affect the power consumption. Provide
an optional support to use deferrable timer which suites low-power
use-cases.
Typically, on our setup we observed, 10% less power consumption with some
use-cases in which CPU goes to power collapse frequently. For example,
playing audio on Soc which has HW based Audio encoder/decoder, CPU
remains idle for longer duration of time. This idle state will save
significant CPU power consumption if KSM don't wakes them up
periodically.
Note that, deferrable timers won't be deferred if any CPU is active and
not in IDLE state.
By default, deferrable timers is enabled. To disable deferrable timers,
$ echo 0 /sys/kernel/mm/ksm/deferrable_timer
Signed-off-by: Chintan Pandya cpan...@codeaurora.org
Cc: Thomas Gleixner t...@linutronix.de
Cc: John Stultz john.stu...@linaro.org
Cc: Peter Zijlstra pet...@infradead.org
Cc: Ingo Molnar mi...@redhat.com
Cc: Hugh Dickins hu...@google.com
---
Changes:
V3--V4:
- Use deferrable timers by default
V2--V3:
- Handled error case properly
- Corrected indentation in Documentation
- Fixed build failure
- Removed left over process_timeout()
V1--V2:
- allowing only valid values to be updated as use_deferrable_timer
- using only 'deferrable' and not 'deferred'
- moved out schedule_timeout code for deferrable timer into timer.c
Documentation/vm/ksm.txt | 6 ++
mm/ksm.c | 36 ++--
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/Documentation/vm/ksm.txt b/Documentation/vm/ksm.txt
index f34a8ee..23e26c3 100644
--- a/Documentation/vm/ksm.txt
+++ b/Documentation/vm/ksm.txt
@@ -87,6 +87,12 @@ pages_sharing- how many more sites are sharing them i.e.
how much saved
pages_unshared - how many pages unique but repeatedly checked for merging
pages_volatile - how many pages changing too fast to be placed in a tree
full_scans - how many times all mergeable areas have been scanned
+deferrable_timer - whether to use deferrable timers or not
+ e.g. echo 1 /sys/kernel/mm/ksm/deferrable_timer
+ Default: 1 (means, we are using deferrable timers. Users
+ might want to clear deferrable_timer option if they want
+ ksm thread to wakeup CPU to carryout ksm activities thus
+ loosing on battery while gaining on memory savings.)
A high ratio of pages_sharing to pages_shared indicates good sharing, but
a high ratio of pages_unshared to pages_sharing indicates wasted effort.
diff --git a/mm/ksm.c b/mm/ksm.c
index fb75902..af90e30 100644
--- a/mm/ksm.c
+++ b/mm/ksm.c
@@ -223,6 +223,9 @@ static unsigned int ksm_thread_pages_to_scan = 100;
/* Milliseconds ksmd should sleep between batches */
static unsigned int ksm_thread_sleep_millisecs = 20;
+/* Boolean to indicate whether to use deferrable timer or not */
+static bool use_deferrable_timer = 1;
+
#ifdef CONFIG_NUMA
/* Zeroed when merging across nodes is not allowed */
static unsigned int ksm_merge_across_nodes = 1;
@@ -1725,8 +1728,13 @@ static int ksm_scan_thread(void *nothing)
try_to_freeze();
if (ksmd_should_run()) {
- schedule_timeout_interruptible(
- msecs_to_jiffies(ksm_thread_sleep_millisecs));
+ signed long to;
+
+ to = msecs_to_jiffies(ksm_thread_sleep_millisecs);
+ if (use_deferrable_timer)
+ schedule_timeout_deferrable_interruptible(to);
+ else
+ schedule_timeout_interruptible(to);
} else {
wait_event_freezable(ksm_thread_wait,
ksmd_should_run() || kthread_should_stop());
@@ -2175,6 +2183,29 @@ static ssize_t run_store(struct kobject *kobj, struct
kobj_attribute *attr,
}
KSM_ATTR(run);
+static ssize_t deferrable_timer_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ return snprintf(buf, 8, %d\n, use_deferrable_timer);
+}
+
+static ssize_t deferrable_timer_store(struct kobject *kobj,
+struct kobj_attribute *attr,
+const char *buf, size_t count)
+{
+ unsigned long enable;
+ int err;
+
+ err = kstrtoul(buf, 10, enable);
+ if (err 0)
+ return err;
+ if (enable = 1)
+ return -EINVAL;
+ use_deferrable_timer = enable;
+ return count;
+}
+KSM_ATTR(deferrable_timer);
+
#ifdef CONFIG_NUMA
static ssize_t merge_across_nodes_show(struct kobject *kobj,
struct kobj_attribute *attr, char *buf)
@@ -2287,6 +2318,7 @@ static struct
Re: [RFC 09/10] scsi: sd: Avoid sending medium write commands if device is write protected
Christoph == Christoph Hellwig h...@infradead.org writes: Christoph This looks reasonable to me. I agree. Reviewed-by: Martin K. Petersen martin.peter...@oracle.com -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/1] iommu-api: Add map_sg/unmap_sg functions
On Tue, Aug 19, 2014 at 10:52:46PM +0200, Laurent Pinchart wrote: On Tuesday 19 August 2014 11:40:24 Olav Haugan wrote: On 8/19/2014 9:11 AM, Laurent Pinchart wrote: On Tuesday 19 August 2014 13:59:54 Joerg Roedel wrote: On Mon, Aug 18, 2014 at 03:47:56PM -0700, Olav Haugan wrote: If the alignment is not correct then iommu_map() will return error. Not sure what other option we have here (and why make it different behavior than iommu_map which just return error when it is not aligned properly). I don't think we want to force any kind of alignment automatically. I would rather have the API tell me I am doing something wrong than having the function aligning the values and possibly undermap or overmap. But sg-offset is an offset into the page (at least it is used that way in the DMA-API and since you do 'page_len = s-offset + s-length' you use it the same way). So when you pass iova + offset the result will no longer be page-aligned. You should force sg-offset == 0 and sg-length to be page-aligned instead. This makes more sense because the IOMMU-API works on (io)-page granularity and not on arbitrary phys-addr ranges like the DMA-API. Yes, I am aware of that. However, several people prefer this than passing in scatterlist. It is not very convenient to pass a scatterlist in some use cases. Someone mentioned a use case where they would have to create a dummy sg list and populate it with the iova just to do an unmap. I believe we would have to do this also. There is no use for sglist when unmapping. However, would like to keep separate API from iommu_unmap() to keep the API function names symmetric (map_sg/unmap_sg). Keeping it symetric is not more complicated, the caller just needs to keep the sg-list used for mapping around. I prefer the unmap_sg call to work in sg-lists too. Do we have a use case where the unmap_sg() implementation would be different than a plain iommu_unmap() call ? If not I'd rather remove unmap_sg() completely. I thought that was why we added the default fallback and set all the drivers to point to these fallback functions. Several people wanted this so that we don't have to have NULL-check in these functions (and have the functions be simple inline functions). Okay, since you add these call-backs to all drivers I think I can live with not doing a pointer check here. I suggested doing a if (ops is not NULL) return ops(); else return default_ops(); to avoid modifying all drivers. I'm not sure why that wasn't received with much enthusiasm. Both Thierry R. and Konrad W. argued for modifying the drivers instead so I implemented what the majority wanted. :-) I'm not blaming you :-) I was just wondering what their rationale was. a). To be inline with the usage of this API. For this particular case the other functions (but maybe I missed some) follow the idea that they implement the ops-func for every operation and they don't have an fallback. b) Follow what x86 maintainers prefer (based on other API calls, such as x86_init or any other platform overrides). c). When there are new IOMMUs it has to take in-to account all of the function ops. If they fail to implement all of them the kernel crashes instead of working (but maybe working incorrectly). d). Lastly, we also want the bloat of kernel. If the compiler decides to roll in the fallback implementation for 'iommu_map_sg' in - it will needlessly expand the kernel wherever we use 'iommu_map_sg' call with a fallback implementation (which might not be called 99% of time). Having the little inline static just do 'func_ops-func' will stop the compiler from optimizing too much and convert this just to a simple function. -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 1/1] iommu-api: Add map_sg/unmap_sg functions
Hi Konrad, On Wednesday 20 August 2014 09:02:50 Konrad Rzeszutek Wilk wrote: On Tue, Aug 19, 2014 at 10:52:46PM +0200, Laurent Pinchart wrote: On Tuesday 19 August 2014 11:40:24 Olav Haugan wrote: On 8/19/2014 9:11 AM, Laurent Pinchart wrote: On Tuesday 19 August 2014 13:59:54 Joerg Roedel wrote: [snip] Okay, since you add these call-backs to all drivers I think I can live with not doing a pointer check here. I suggested doing a if (ops is not NULL) return ops(); else return default_ops(); to avoid modifying all drivers. I'm not sure why that wasn't received with much enthusiasm. Both Thierry R. and Konrad W. argued for modifying the drivers instead so I implemented what the majority wanted. :-) I'm not blaming you :-) I was just wondering what their rationale was. a). To be inline with the usage of this API. For this particular case the other functions (but maybe I missed some) follow the idea that they implement the ops-func for every operation and they don't have an fallback. That's because the other functions are mandatory, while this particular one is just an optimization and can be implemented generically. b) Follow what x86 maintainers prefer (based on other API calls, such as x86_init or any other platform overrides). c). When there are new IOMMUs it has to take in-to account all of the function ops. If they fail to implement all of them the kernel crashes instead of working (but maybe working incorrectly). I don't think that applies in this case, the generic implementation should work in all cases. d). Lastly, we also want the bloat of kernel. If the compiler decides to roll in the fallback implementation for 'iommu_map_sg' in - it will needlessly expand the kernel wherever we use 'iommu_map_sg' call with a fallback implementation (which might not be called 99% of time). Having the little inline static just do 'func_ops-func' will stop the compiler from optimizing too much and convert this just to a simple function. That's an argument I can buy. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] ARM: apq8064: Add pinmux and i2c pinctrl nodes
Hi,
Am 20.08.2014 14:02, schrieb Kiran Padwal:
This patch adds pinmux and i2c pinctrl DT node for IFC6410 board.
It also adds necessary DT support for i2c eeprom which is present on
IFC6410.
Tested on IFC6410 board.
Signed-off-by: Kiran Padwal kiran.pad...@smartplayin.com
---
Chages since v1:
- Renamed pinmux phandle qcom_pinmux to tlmm_pinmux.
- Updated pinmux interrupt.
arch/arm/boot/dts/qcom-apq8064-ifc6410.dts | 29 ++
arch/arm/boot/dts/qcom-apq8064.dtsi| 59
2 files changed, 88 insertions(+)
diff --git a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
index 7c2441d..d52ac3c 100644
--- a/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
+++ b/arch/arm/boot/dts/qcom-apq8064-ifc6410.dts
@@ -5,6 +5,15 @@
compatible = qcom,apq8064-ifc6410, qcom,apq8064;
soc {
+ pinmux@80 {
+ i2c1_pins: i2c1_pinmux {
Is there a requirement to have _pinmux in the subnode of the pinmux
node? Otherwise use just i2c1? I notice because the general convention
seems to be dashes rather than underscores for node names.
+ mux {
+ pins = gpio20, gpio21;
+ function = gsbi1;
+ };
+ };
+ };
+
gsbi@1660 {
status = ok;
qcom,mode = GSBI_PROT_I2C_UART;
@@ -12,5 +21,25 @@
status = ok;
};
};
+
+ gsbi1: gsbi@1244 {
+ qcom,mode = GSBI_PROT_I2C;
+ status = ok;
Usually the overridden status property goes first, as seen on the
previous gsbi node.
Further, its canonical value is okay (although in practice anything
other than disabled should work), so suggest you adopt it for your new
nodes.
Also, you already provide a label gsbi1 to this node in the .dtsi
below, so you don't need to do it here again.
Some architectures prefer in such a case that in the .dts the node is
referenced as gsbi1 {...}; below the root node in alphabetical order
rather than duplicating the full /soc/foo@bar hierarchy.
+
+ i2c1: i2c@1246 {
Suggest to move the i2c1 label to the .dtsi. Then optionally same could
be done here as outlined for gsbi1.
+ status = ok;
okay.
+
+ clock-frequency = 20;
+
+ pinctrl-0 = i2c1_pins;
+ pinctrl-names = default;
+
+ eeprom: eeprom@52 {
+ compatible = atmel,24c128;
+ reg = 0x52;
+ pagesize = 32;
+ };
+ };
+ };
};
};
diff --git a/arch/arm/boot/dts/qcom-apq8064.dtsi
b/arch/arm/boot/dts/qcom-apq8064.dtsi
index 92bf793..bb2ccde 100644
--- a/arch/arm/boot/dts/qcom-apq8064.dtsi
+++ b/arch/arm/boot/dts/qcom-apq8064.dtsi
@@ -70,6 +70,17 @@
ranges;
compatible = simple-bus;
+ tlmm_pinmux: pinmux@80 {
+ compatible = qcom,apq8064-pinctrl;
+ reg = 0x80 0x4000;
+
+ gpio-controller;
+ #gpio-cells = 2;
+ interrupt-controller;
+ #interrupt-cells = 2;
+ interrupts = 0 16 0x4;
s/0x4/IRQ_TYPE_LEVEL_HIGH/?
+ };
+
intc: interrupt-controller@200 {
compatible = qcom,msm-qgic2;
interrupt-controller;
@@ -133,6 +144,54 @@
regulator;
};
+ gsbi1: gsbi@1244 {
+ compatible = qcom,gsbi-v1.0.0;
+ reg = 0x1244 0x100;
+ clocks = gcc GSBI1_H_CLK;
+ clock-names = iface;
+ #address-cells = 1;
+ #size-cells = 1;
+ ranges;
+ status = disabled;
+
+ i2c@1246 {
+ compatible = qcom,i2c-qup-v1.1.1;
+ reg = 0x1246 0x1000;
+ interrupts = 0 194 0;
The trailing 0 might be IRQ_TYPE_NONE?
+
+ clocks = gcc GSBI1_QUP_CLK, gcc
GSBI1_H_CLK;
+ clock-names = core, iface;
+ status = disabled;
I'd guess this is redundant since the parent is already disabled?
+
+ #address-cells = 1;
+ #size-cells = 0;
+ };
Re: [PATCH v1 1/3] pinctrl: qcom: Add APQ8084 pinctrl support
On Tue, Aug 19, 2014 at 09:39:30PM -0700, Bjorn Andersson wrote: On Tue 19 Aug 10:22 PDT 2014, Georgi Djakov wrote: This patch adds support for the TLMM (Top-Level Mode Mux) block found in the APQ8084 platform. [...] + +#define NUM_GPIO_PINGROUPS 143 + I think this looks good overall, but in my APQ8084 documentation (80-NG550-2X Rev. B) there are 147 (0..146) gpio pins in the TLMM block. Any suggestion to why there's a discrepancy? We have some discrepancies internally on the documentation. However, in this case I'd expect the OEM documentation to be correct. So this needs to change to bjorn's version, as it appears to be the correct version. I have no idea why some pins are missing. -- sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: msm: Make phy_reset clk and reset line optional.
On Thu, Jul 17, 2014 at 09:16:40PM +0100, Srinivas Kandagatla wrote: This patch makes the phy reset clk and reset line optional as this clk is not available on boards like IFC6410 with APQ8064. phy-reset clk is only used as argument to the mach level callbacks, so this patch adds condition before clk_get calls so that the driver wouldn't fail on SOCs which do not have this support. Signed-off-by: Srinivas Kandagatla srinivas.kandaga...@linaro.org --- Hi Felipe, With this new patch now the error message is only printed if the SOC actually supports the phy reset clk, for SOCs like APQ8064 where there is no phy reset clock or the callback which takes it there is no point in doing a clk_get call in the first place. doesn't apply. Please rebase on top of v3.17-rc1 -- balbi signature.asc Description: Digital signature
Re: [PATCH v4] irqchip: gic: Allow gic_arch_extn hooks to call into scheduler
On 08/17/14 17:17, Nicolas Pitre wrote: On Sun, 17 Aug 2014, Russell King - ARM Linux wrote: I have no problem with changing gic_raise_softirq() to use a different lock, which gic_migrate_target(), and gic_set_affinity() can also use. There's no need for horrid locking here, because the only thing we're protecting is gic_map[] and the write to the register to trigger an IPI - and nothing using gic_arch_extn has any business knowing about SGIs. No need for these crappy sgi_map_lock() macros and all the ifdeffery. Those macros are there only to conditionalize the locking in gic_raise_softirq() because no locking what so ever is needed there when gic_migrate_target() is configured out. I suggested the macros to cut down on the #ifdefery in the code. Ok I can resend with the sgi lock around the gic_cpu_map updating code. Let's see how v5 goes. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] irqchip: gic: Allow gic_arch_extn hooks to call into scheduler
On 08/17/14 18:54, Jason Cooper wrote: Stephen, is the out of tree code that triggered this bound for mainline? It's bound for mainline eventually. We're actively working on enabling more low power modes and when that happens we'll need this patch. We can always carry this patch for now if you don't want to accept it, but I figured getting it mainlined reduces our carrying burden and also makes a minor improvement in sending IPIs when the BL switcher is used. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] irqchip: gic: Allow gic_arch_extn hooks to call into scheduler
Commit 1a6b69b6548c (ARM: gic: add CPU migration support, 2012-04-12) introduced an acquisition of the irq_controller_lock in gic_raise_softirq() which can lead to a spinlock recursion if the gic_arch_extn hooks call into the scheduler (via complete() or wake_up(), etc.). This happens because gic_arch_extn hooks are normally called with the irq_controller_lock held and calling into the scheduler may cause us to call smp_send_reschedule() which will grab the irq_controller_lock again. Here's an example from a vendor kernel (note that the gic_arch_extn hook code here isn't actually in mainline): BUG: spinlock recursion on CPU#0, swapper/0/1 lock: irq_controller_lock+0x0/0x18, .magic: dead4ead, .owner: sw er_cpu: 0 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.10-00430-g3d433c4e Call trace: [ffc87e1c] dump_backtrace+0x0/0x140 [ffc87f6c] show_stack+0x10/0x1c [ffc00064732c] dump_stack+0x74/0xc4 [ffc0006446c0] spin_dump+0x78/0x88 [ffc0006446f4] spin_bug+0x24/0x34 [ffcd47d0] do_raw_spin_lock+0x58/0x148 [ffc00064d398] _raw_spin_lock_irqsave+0x24/0x38 [ffc0002c9d7c] gic_raise_softirq+0x2c/0xbc [ffc8daa4] smp_send_reschedule+0x34/0x40 [ffcc1e94] try_to_wake_up+0x224/0x288 [ffcc1f4c] default_wake_function+0xc/0x18 [ffcceef0] __wake_up_common+0x50/0x8c [ffccef3c] __wake_up_locked+0x10/0x1c [ffccf734] complete+0x3c/0x5c [ffc0002f0e78] msm_mpm_enable_irq_exclusive+0x1b8/0x1c8 [ffc0002f0f58] __msm_mpm_enable_irq+0x4c/0x7c [ffc0002f0f94] msm_mpm_enable_irq+0xc/0x18 [ffc0002c9bb0] gic_unmask_irq+0x40/0x7c [ffcde5f4] irq_enable+0x2c/0x48 [ffcde65c] irq_startup+0x4c/0x74 [ffcdd2fc] __setup_irq+0x264/0x3f0 [ffcdd5e0] request_threaded_irq+0xcc/0x11c [ffcdf254] devm_request_threaded_irq+0x68/0xb4 [ffc000471520] msm_iommu_ctx_probe+0x124/0x2d4 [ffc000337374] platform_drv_probe+0x20/0x54 [ffc00033598c] driver_probe_device+0x158/0x340 [ffc000335c20] __driver_attach+0x60/0x90 [ffc000333c9c] bus_for_each_dev+0x6c/0x8c [ffc000335304] driver_attach+0x1c/0x28 [ffc000334f14] bus_add_driver+0x120/0x204 [ffc0003362e4] driver_register+0xbc/0x10c [ffc000337348] __platform_driver_register+0x5c/0x68 [ffc00094c478] msm_iommu_driver_init+0x54/0x7c [ffc813ec] do_one_initcall+0xa4/0x130 [ffc00091d928] kernel_init_freeable+0x138/0x1dc [ffc000642578] kernel_init+0xc/0xd4 We really just want to synchronize the sending of an SGI with the update of the gic_cpu_map[], so introduce a new SGI lock that we can use to synchronize the two code paths. To further optimize we don't take any locks at all if the BL switcher code isn't used. Cc: Nicolas Pitre n...@linaro.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Signed-off-by: Stephen Boyd sb...@codeaurora.org --- drivers/irqchip/irq-gic.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 4b959e606fe8..052762638b1c 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -80,6 +80,16 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock); #define NR_GIC_CPU_IF 8 static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly; +#ifdef CONFIG_BL_SWITCHER +/* Synchronize switching CPU interface and sending SGIs */ +static DEFINE_RAW_SPINLOCK(gic_sgi_lock); +#define sgi_map_lock(flags) raw_spin_lock_irqsave(gic_sgi_lock, flags) +#define sgi_map_unlock(flags) raw_spin_unlock_irqrestore(gic_sgi_lock, flags) +#else +#define sgi_map_lock(flags) (void)(flags) +#define sgi_map_unlock(flags) (void)(flags) +#endif + /* * Supported arch specific GIC irq extension. * Default make them NULL. @@ -605,7 +615,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0; - raw_spin_lock_irqsave(irq_controller_lock, flags); + sgi_map_lock(flags); /* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -620,7 +630,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map 16 | irq, gic_data_dist_base(gic_data[0]) + GIC_DIST_SOFTINT); - raw_spin_unlock_irqrestore(irq_controller_lock, flags); + sgi_map_unlock(flags); } #endif @@ -690,6 +700,7 @@ void gic_migrate_target(unsigned int new_cpu_id) ror_val = (cur_cpu_id - new_cpu_id) 31; raw_spin_lock(irq_controller_lock); + raw_spin_lock(gic_sgi_lock); /* Update the target interface for this logical CPU */ gic_cpu_map[cpu] = 1 new_cpu_id; @@ -709,6 +720,7 @@ void gic_migrate_target(unsigned int new_cpu_id) } } + raw_spin_unlock(gic_sgi_lock); raw_spin_unlock(irq_controller_lock); /* -- The Qualcomm Innovation Center, Inc. is a
Re: [PATCH v1 1/3] pinctrl: qcom: Add APQ8084 pinctrl support
On 20.08.14 18:42, Andy Gross wrote: On Tue, Aug 19, 2014 at 09:39:30PM -0700, Bjorn Andersson wrote: On Tue 19 Aug 10:22 PDT 2014, Georgi Djakov wrote: This patch adds support for the TLMM (Top-Level Mode Mux) block found in the APQ8084 platform. [...] + +#define NUM_GPIO_PINGROUPS 143 + I think this looks good overall, but in my APQ8084 documentation (80-NG550-2X Rev. B) there are 147 (0..146) gpio pins in the TLMM block. Any suggestion to why there's a discrepancy? We have some discrepancies internally on the documentation. However, in this case I'd expect the OEM documentation to be correct. So this needs to change to bjorn's version, as it appears to be the correct version. I have no idea why some pins are missing. Thank you for reviewing, Bjorn and Andy. I'll update and submit v2. BR, Georgi -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/3] pinctrl: qcom: Add APQ8084 pinctrl support
On Tue, Aug 19, 2014 at 08:22:14PM +0300, Georgi Djakov wrote:
This patch adds support for the TLMM (Top-Level Mode Mux) block found
in the APQ8084 platform.
Comment in-line
snip
+ PINCTRL_PIN(134, GPIO_134),
+ PINCTRL_PIN(135, GPIO_135),
+ PINCTRL_PIN(136, GPIO_136),
+ PINCTRL_PIN(137, GPIO_137),
+ PINCTRL_PIN(138, GPIO_138),
+ PINCTRL_PIN(139, GPIO_139),
+ PINCTRL_PIN(140, GPIO_140),
+ PINCTRL_PIN(141, GPIO_141),
+ PINCTRL_PIN(142, GPIO_142),
Add in the missing pins
+ PINCTRL_PIN(142, GPIO_143),
+ PINCTRL_PIN(142, GPIO_144),
+ PINCTRL_PIN(142, GPIO_145),
+ PINCTRL_PIN(142, GPIO_146),
+
+ PINCTRL_PIN(143, SDC1_CLK),
+ PINCTRL_PIN(144, SDC1_CMD),
+ PINCTRL_PIN(145, SDC1_DATA),
+ PINCTRL_PIN(146, SDC2_CLK),
+ PINCTRL_PIN(147, SDC2_CMD),
+ PINCTRL_PIN(148, SDC2_DATA),
Shift down to accomodate 4 pins
+ PINCTRL_PIN(147, SDC1_CLK),
+ PINCTRL_PIN(148, SDC1_CMD),
+ PINCTRL_PIN(149, SDC1_DATA),
+ PINCTRL_PIN(150, SDC2_CLK),
+ PINCTRL_PIN(151, SDC2_CMD),
+ PINCTRL_PIN(152, SDC2_DATA),
+};
+
+#define DECLARE_APQ_GPIO_PINS(pin) static const unsigned int
gpio##pin##_pins[] = { pin }
+
snip
+DECLARE_APQ_GPIO_PINS(138);
+DECLARE_APQ_GPIO_PINS(139);
+DECLARE_APQ_GPIO_PINS(140);
+DECLARE_APQ_GPIO_PINS(141);
+DECLARE_APQ_GPIO_PINS(142);
+DECLARE_APQ_GPIO_PINS(143);
+DECLARE_APQ_GPIO_PINS(144);
+DECLARE_APQ_GPIO_PINS(145);
+DECLARE_APQ_GPIO_PINS(146);
+
+static const unsigned int sdc1_clk_pins[] = { 143 };
+static const unsigned int sdc1_cmd_pins[] = { 144 };
+static const unsigned int sdc1_data_pins[] = { 145 };
+static const unsigned int sdc2_clk_pins[] = { 146 };
+static const unsigned int sdc2_cmd_pins[] = { 147 };
+static const unsigned int sdc2_data_pins[] = { 148 };
+static const unsigned int sdc1_clk_pins[] = { 147 };
+static const unsigned int sdc1_cmd_pins[] = { 148 };
+static const unsigned int sdc1_data_pins[] = { 149 };
+static const unsigned int sdc2_clk_pins[] = { 150 };
+static const unsigned int sdc2_cmd_pins[] = { 151 };
+static const unsigned int sdc2_data_pins[] = { 152 };
+
+#define FUNCTION(fname) \
+ [APQ_MUX_##fname] = { \
+ .name = #fname, \
snip
+ gpio64, gpio65, gpio66, gpio67, gpio68, gpio69, gpio70,
+ gpio71, gpio72, gpio73, gpio74, gpio75, gpio76, gpio77,
+ gpio78, gpio79, gpio80, gpio81, gpio82, gpio83, gpio84,
+ gpio85, gpio86, gpio87, gpio88, gpio89, gpio90, gpio91,
+ gpio92, gpio93, gpio94, gpio95, gpio96, gpio97, gpio98,
+ gpio99, gpio100, gpio101, gpio102, gpio103, gpio104,
+ gpio105, gpio106, gpio107, gpio108, gpio109, gpio110,
+ gpio111, gpio112, gpio113, gpio114, gpio115, gpio116,
+ gpio117, gpio118, gpio119, gpio120, gpio121, gpio122,
+ gpio123, gpio124, gpio125, gpio126, gpio127, gpio128,
+ gpio129, gpio130, gpio131, gpio132, gpio133, gpio134,
+ gpio135, gpio136, gpio137, gpio138, gpio139, gpio140,
+ gpio141, gpio142
Add in extra pins
snip
+ FUNCTION(cam_mclk3),
+ FUNCTION(cci_async),
+ FUNCTION(cci_async_in0),
+ FUNCTION(cci_i2c),
split into cci_i2c0 and cci_i2c1
+ FUNCTION(cci_timer0),
+ FUNCTION(cci_timer1),
+ FUNCTION(cci_timer2),
+ FUNCTION(cci_timer3),
+ FUNCTION(cci_timer4),
+ FUNCTION(dll_sdc10),
+ FUNCTION(dll_sdc11),
+ FUNCTION(dll_sdc20),
+ FUNCTION(dll_sdc21),
+ FUNCTION(edp_hot),
change this to edp_hpd
+ FUNCTION(edp_tpa),
+ FUNCTION(gcc_gp1),
+ FUNCTION(gcc_gp1_clk_b),
this isnt used
+ FUNCTION(gcc_gp2),
+ FUNCTION(gcc_gp2_clk_b),
this isnt used
+ FUNCTION(gcc_gp3),
+ FUNCTION(gcc_gp3_clk_b),
this isnt used
+ FUNCTION(gcc_obt),
+ FUNCTION(gcc_vtt),
+ FUNCTION(gp_mn),
+ FUNCTION(gp_pdm),
+ FUNCTION(gp_pdm_0a),
+ FUNCTION(gp_pdm_2a),
+ FUNCTION(gp_pdm_1b),
+ FUNCTION(gp_pdm_2b),
Drop a and b, there is no collision on the same pin, so unnecessary
+ FUNCTION(gp0_clk),
+ FUNCTION(gp1_clk),
+ FUNCTION(gpio),
+ FUNCTION(hdmi_cec),
+ FUNCTION(hdmi_ddc),
+ FUNCTION(hdmi_dtest),
+ FUNCTION(hdmi_hot),
change this to hdmi_hpd
+ FUNCTION(hdmi_rcv),
+ FUNCTION(hsic),
+ FUNCTION(ldo_en),
+ FUNCTION(ldo_update),
+ FUNCTION(mdp_vsync),
+ FUNCTION(pci_e0),
+ FUNCTION(pci_e0_rst),
+ FUNCTION(pci_e1),
+ FUNCTION(pci_e1_rst),
+ FUNCTION(pci_e1_rst_n),
+ FUNCTION(pci_e1_clkreq_n),
+ FUNCTION(pri_mi2s),
+ FUNCTION(qdss_cti),
+ FUNCTION(qdss_cti_trig_out_a),
+ FUNCTION(qdss_cti_trig_in_a),
+ FUNCTION(qdss_cti_trig_in_b),
+ FUNCTION(qdss_cti_trig_in_c),
+ FUNCTION(qdss_cti_trig_out_c),
Drop all the qdss
+ FUNCTION(qua_mi2s),
+ FUNCTION(sata_act),
+
Re: [PATCH v5] irqchip: gic: Allow gic_arch_extn hooks to call into scheduler
On Wed, 20 Aug 2014, Stephen Boyd wrote: Commit 1a6b69b6548c (ARM: gic: add CPU migration support, 2012-04-12) introduced an acquisition of the irq_controller_lock in gic_raise_softirq() which can lead to a spinlock recursion if the gic_arch_extn hooks call into the scheduler (via complete() or wake_up(), etc.). This happens because gic_arch_extn hooks are normally called with the irq_controller_lock held and calling into the scheduler may cause us to call smp_send_reschedule() which will grab the irq_controller_lock again. Here's an example from a vendor kernel (note that the gic_arch_extn hook code here isn't actually in mainline): BUG: spinlock recursion on CPU#0, swapper/0/1 Given the actual source of the bug is contested, you probably should only talk about the locking optimization which is the only undisputed reason for this patch. lock: irq_controller_lock+0x0/0x18, .magic: dead4ead, .owner: sw er_cpu: 0 CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.14.10-00430-g3d433c4e Call trace: [ffc87e1c] dump_backtrace+0x0/0x140 [ffc87f6c] show_stack+0x10/0x1c [ffc00064732c] dump_stack+0x74/0xc4 [ffc0006446c0] spin_dump+0x78/0x88 [ffc0006446f4] spin_bug+0x24/0x34 [ffcd47d0] do_raw_spin_lock+0x58/0x148 [ffc00064d398] _raw_spin_lock_irqsave+0x24/0x38 [ffc0002c9d7c] gic_raise_softirq+0x2c/0xbc [ffc8daa4] smp_send_reschedule+0x34/0x40 [ffcc1e94] try_to_wake_up+0x224/0x288 [ffcc1f4c] default_wake_function+0xc/0x18 [ffcceef0] __wake_up_common+0x50/0x8c [ffccef3c] __wake_up_locked+0x10/0x1c [ffccf734] complete+0x3c/0x5c [ffc0002f0e78] msm_mpm_enable_irq_exclusive+0x1b8/0x1c8 [ffc0002f0f58] __msm_mpm_enable_irq+0x4c/0x7c [ffc0002f0f94] msm_mpm_enable_irq+0xc/0x18 [ffc0002c9bb0] gic_unmask_irq+0x40/0x7c [ffcde5f4] irq_enable+0x2c/0x48 [ffcde65c] irq_startup+0x4c/0x74 [ffcdd2fc] __setup_irq+0x264/0x3f0 [ffcdd5e0] request_threaded_irq+0xcc/0x11c [ffcdf254] devm_request_threaded_irq+0x68/0xb4 [ffc000471520] msm_iommu_ctx_probe+0x124/0x2d4 [ffc000337374] platform_drv_probe+0x20/0x54 [ffc00033598c] driver_probe_device+0x158/0x340 [ffc000335c20] __driver_attach+0x60/0x90 [ffc000333c9c] bus_for_each_dev+0x6c/0x8c [ffc000335304] driver_attach+0x1c/0x28 [ffc000334f14] bus_add_driver+0x120/0x204 [ffc0003362e4] driver_register+0xbc/0x10c [ffc000337348] __platform_driver_register+0x5c/0x68 [ffc00094c478] msm_iommu_driver_init+0x54/0x7c [ffc813ec] do_one_initcall+0xa4/0x130 [ffc00091d928] kernel_init_freeable+0x138/0x1dc [ffc000642578] kernel_init+0xc/0xd4 We really just want to synchronize the sending of an SGI with the update of the gic_cpu_map[], so introduce a new SGI lock that we can use to synchronize the two code paths. To further optimize we don't take any locks at all if the BL switcher code isn't used. Cc: Nicolas Pitre n...@linaro.org Cc: Russell King - ARM Linux li...@arm.linux.org.uk Signed-off-by: Stephen Boyd sb...@codeaurora.org --- drivers/irqchip/irq-gic.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c index 4b959e606fe8..052762638b1c 100644 --- a/drivers/irqchip/irq-gic.c +++ b/drivers/irqchip/irq-gic.c @@ -80,6 +80,16 @@ static DEFINE_RAW_SPINLOCK(irq_controller_lock); #define NR_GIC_CPU_IF 8 static u8 gic_cpu_map[NR_GIC_CPU_IF] __read_mostly; +#ifdef CONFIG_BL_SWITCHER +/* Synchronize switching CPU interface and sending SGIs */ +static DEFINE_RAW_SPINLOCK(gic_sgi_lock); +#define sgi_map_lock(flags) raw_spin_lock_irqsave(gic_sgi_lock, flags) +#define sgi_map_unlock(flags) raw_spin_unlock_irqrestore(gic_sgi_lock, flags) +#else +#define sgi_map_lock(flags) (void)(flags) +#define sgi_map_unlock(flags) (void)(flags) +#endif + /* * Supported arch specific GIC irq extension. * Default make them NULL. @@ -605,7 +615,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) int cpu; unsigned long flags, map = 0; - raw_spin_lock_irqsave(irq_controller_lock, flags); + sgi_map_lock(flags); /* Convert our logical CPU mask into a physical one. */ for_each_cpu(cpu, mask) @@ -620,7 +630,7 @@ static void gic_raise_softirq(const struct cpumask *mask, unsigned int irq) /* this always happens on GIC0 */ writel_relaxed(map 16 | irq, gic_data_dist_base(gic_data[0]) + GIC_DIST_SOFTINT); - raw_spin_unlock_irqrestore(irq_controller_lock, flags); + sgi_map_unlock(flags); } #endif @@ -690,6 +700,7 @@ void gic_migrate_target(unsigned int new_cpu_id) ror_val = (cur_cpu_id - new_cpu_id) 31; raw_spin_lock(irq_controller_lock); + raw_spin_lock(gic_sgi_lock); /* Update the target interface for this
Re: [PATCH v3 2/6] pinctrl: Introduce pinctrl driver for Qualcomm SSBI PMIC's
On Wed 20 Aug 01:06 PDT 2014, Srinivas Kandagatla wrote: Hi Bjorn, Hi Srinivas, Thanks for the testing. I'm reworking the driver to incorporate yours, Linus' and Ivans feedback. Two things which I noticed while trying out this driver to drive a reset line. 1 gpio numbering for pinconf vs gpio are not consistent, they differ by an offset of 1. After scratching my head regarding this I now see that there's a off by one in most of the gpio functions. I've rewroked this to make more sense now. The gpio numbers has to start at 1, as all documentation is based on that. [...] 2 Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set to enable gpio mode. without this bit driver does not work for output pins. Thanks, I missed that. Unfortunately, setting that bit results in input not working - the interrupt bits are never set for gpios that have that bit set. I'm trying to figure out why this is the case before sending out the new version... Regards, Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] pinctrl: Device tree bindings for Qualcomm pm8xxx gpio block
On Mon 18 Aug 00:16 PDT 2014, Ivan T. Ivanov wrote: On Sat, 2014-08-16 at 16:24 +0100, Daniel wrote: @Ivan: sorry about the double post. Am 11.08.2014 um 16:40 schrieb Ivan T. Ivanov iiva...@mm-sol.com: [...] +#define PMIC_GPIO_PULL_UP_30 1 +#define PMIC_GPIO_PULL_UP_1P52 +#define PMIC_GPIO_PULL_UP_31P5 3 +#define PMIC_GPIO_PULL_UP_1P5_30 4 Looking at drivers/pinctrl/qcom/pinctrl-ssbi-pmic.c, shouldn't these defines start at 0? e.g. #define PMIC_GPIO_PULL_UP_30 0 Initially bias-pull-up was used to set this parameter. Zero value for bias-pull-up has special meaning ...the pin is connected to VDD So values in DTS have to have offset by one. Micro Amps are non-standard for pull-ups, thats why I have changed this to qcom,pull-up-strength, but I have made mistake in config_set function. Following patch should fix the issue. I will send updated version soon. The bias-pull-up is read as u32 and 0 means that it's not pull-up, therefor i shifted them all. Sorry about that. Now that we have this in a separate property there's no point in such trickery and we should make them follow the register values, i.e: #define PM8XXX_GPIO_BIAS_PU_30 0 #define PM8XXX_GPIO_BIAS_PU_1P5 1 #define PM8XXX_GPIO_BIAS_PU_31P52 #define PM8XXX_GPIO_BIAS_PU_1P5_30 3 I find it cleaner and we don't need the translation. However, I still cannot get any data from those 2 pins if I export them through /sys/class/gpio... Reading should work, but most other gpio operations was off by one it seems. I have corrected this (and other reported things) and will send out a new version soon. - pin-bias = arg - PM8XXX_GPIO_BIAS_PU_30; + pin-bias = arg - PMIC_GPIO_PULL_UP_30; If we just make it follow the register value (starting at 0) we just use arg straight off. Regards, Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/6] pinctrl: Introduce pinctrl driver for Qualcomm SSBI PMIC's
On Wed, Aug 20, 2014 at 2:28 PM, Bjorn Andersson bjorn.anders...@sonymobile.com wrote: On Wed 20 Aug 01:06 PDT 2014, Srinivas Kandagatla wrote: 2 Looking back at v3.4 kernel, for gpio modes, BIT(0) of bank 0 is set to enable gpio mode. without this bit driver does not work for output pins. Thanks, I missed that. Unfortunately, setting that bit results in input not working - the interrupt bits are never set for gpios that have that bit set. I'm trying to figure out why this is the case before sending out the new version... With help from Andy Gross this is now corrected as well, turned out that BIT(0) in bank0 controls if the direction is considered. As I was tricked by the multiple levels of indirection in the codeaurora version I got these wrong. Will send out an updated version shortly. Regards, Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] pinctrl: Device tree bindings for Qualcomm pm8xxx gpio block
On Mon 11 Aug 08:40 PDT 2014, Ivan T. Ivanov wrote: [...] diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.txt [...] +SUBNODES: [...] +- function: + Usage: required + Value type: string + Definition: Specify the alternative function to be configured for the + specified pins. Valid values are: + normal, + paired, + func1, + func2, + dtest1, + dtest2, + dtest3, + dtest4 + I still think it looks better with real functions, but I rather go with this than discussing it forever. +- qcom,pull-up-strength: + Usage: optional + Value type: u32 + Definition: Specifies the strength to use for pull up, if selected. + Valid values are; as defined in + dt-bindings/pinctrl/qcom,pmic-gpio.h: + 1: 30uA (PMIC_GPIO_PULL_UP_30) + 2: 1.5uA(PMIC_GPIO_PULL_UP_1P5) + 3: 31.5uA (PMIC_GPIO_PULL_UP_31P5) + 4: 1.5uA + 30uA boost (PMIC_GPIO_PULL_UP_1P5_30) + If this property is ommited 30uA strength will be used if + pull up is selected I would prefer if we decrement this one step, as it will follow the register values of both the ssbi and spmi based pmics. [...] + +- power-source: + Usage: optional + Value type: u32 + Definition: Selects the power source for the specified pins. Valid + power sources are defined per chip in + dt-bindings/pinctrl/qcom,pmic-gpio.h + _GPIO_L6, _GPIO_L5... After implementing this my only concern is that debugfs output is not as useful anymore; saying VIN2 instead of S4. But I can live with this. diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h [...] + +#define PM8038_GPIO1_2_LPG_DRV PMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO3_5V_BOOST_EN PMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO4_SSBI_ALT_CLKPMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO5_6_EXT_REG_ENPMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO10_11_EXT_REG_EN PMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO6_7_CLK PMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO9_BAT_ALRM_OUTPMIC_GPIO_FUNC_FUNC1 +#define PM8038_GPIO6_12_KYPD_DRV PMIC_GPIO_FUNC_FUNC2 + +#define PM8058_GPIO7_8_MP3_CLK PMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO7_8_BCLK_19P2MHZ PMIC_GPIO_FUNC_FUNC2 +#define PM8058_GPIO9_26_KYPD_DRV PMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO21_23_UART_TX PMIC_GPIO_FUNC_FUNC2 +#define PM8058_GPIO24_26_LPG_DRV PMIC_GPIO_FUNC_FUNC2 +#define PM8058_GPIO33_BCLK_19P2MHZ PMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO34_35_MP3_CLK PMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO36_BCLK_19P2MHZ PMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO37_UPL_OUTPMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO37_UART_M_RX PMIC_GPIO_FUNC_FUNC2 +#define PM8058_GPIO38_XO_SLEEP_CLK PMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO38_39_CLK_32KHZ PMIC_GPIO_FUNC_FUNC2 +#define PM8058_GPIO39_MP3_CLKPMIC_GPIO_FUNC_FUNC1 +#define PM8058_GPIO40_EXT_BB_EN PMIC_GPIO_FUNC_FUNC1 + +#define PM8917_GPIO9_18_KEYP_DRV PMIC_GPIO_FUNC_FUNC1 +#define PM8917_GPIO20_BAT_ALRM_OUT PMIC_GPIO_FUNC_FUNC1 +#define PM8917_GPIO21_23_UART_TX PMIC_GPIO_FUNC_FUNC2 +#define PM8917_GPIO25_26_EXT_REG_EN PMIC_GPIO_FUNC_FUNC1 +#define PM8917_GPIO37_38_XO_SLEEP_CLKPMIC_GPIO_FUNC_FUNC1 +#define PM8917_GPIO37_38_MP3_CLK PMIC_GPIO_FUNC_FUNC2 Stephens argument was that we don't want to have huge tables for the functions and I can see his point, it will be some work to build all the tables. Adding all this defines is unfortunately doing just that. I had a version of my driver that exposed real functions by name from the driver, following the pattern we have for other pinctrl drivers and making the dts very easy to read. But if you don't think we should go that route then I suggest that we just call it func1/func2 and skip these defines. Regards, Bjorn -- To unsubscribe from this list: send the line unsubscribe linux-arm-msm in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 6/6] ARM: dts: qcom: Add APQ8074 Dragonboard PMIC GPIO bindings
On Mon 11 Aug 08:40 PDT 2014, Ivan T. Ivanov wrote:
diff --git a/arch/arm/boot/dts/qcom-apq8074-dragonboard-pmics-pins.dtsi
b/arch/arm/boot/dts/qcom-apq8074-dragonboard-pmics-pins.dtsi
[...]
+
+pm8941_gpios {
+
+ pinctrl-names = default;
+ pinctrl-0 = pm8941_gpios_default;
+
+ pm8941_gpios_default: default {
+ group-1 {
+ pins = gpio1, gpio2, gpio5, gpio29;
+ function = PMIC_GPIO_FUNC_NORMAL;
This looks really strange, I can't get myself to stop thinking that you forgot
the around this (I know it's a string). I don't like these defines.
+ input-enable;
+ power-source = PM8941_GPIO_S3;
+ qcom,pull-up-strength = PMIC_GPIO_PULL_UP_30;
+ };
+ group-6 { /* TUSB3_HUB-RESET */
Why not name the nodes something useful?
If you name it tusb3-hub-reset you can skip the comment.
+ pins = gpio6;
+ function = PMIC_GPIO_FUNC_NORMAL;
+ output-high;
+ drive-push-pull;
+ power-source = PM8941_GPIO_VPH;
+ qcom,pull-up-strength = PMIC_GPIO_PULL_UP_30;
+ qcom,drive-strength = PMIC_GPIO_STRENGTH_MED;
+ };
Regards,
Bjorn
--
To unsubscribe from this list: send the line unsubscribe linux-arm-msm in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/8] qcom: spm: Add Subsystem Power Manager driver (SAW2)
On 08/19/14 20:24, Lina Iyer wrote: On Tue, Aug 19, 2014 at 07:01:53PM -0700, Stephen Boyd wrote: On 08/19/14 15:15, Lina Iyer wrote: SPM is a hardware block that controls the peripheral logic surrounding the application cores (cpu/l$). When the core executes WFI instruction, the SPM takes over the putting the core in low power state as configured. The wake up for the SPM is an interrupt at the GIC, which then completes the rest of low power mode sequence and brings the core out of low power mode. The SPM has a set of control registers that configure the SPMs individually based on the type of the core and the runtime conditions. SPM is a finite state machine block to which a sequence is provided and it interprets the bytes and executes them in sequence. Each low power mode that the core can enter into is provided to the SPM as a sequence. Configure the SPM to set the core (cpu or L2) into its low power mode, the index of the first command in the sequence is set in the SPM_CTL register. When the core executes ARM wfi instruction, it triggers the SPM state machine to start executing from that index. The SPM state machine waits until the interrupt occurs and starts executing the rest of the sequence until it hits the end of the sequence. The end of the sequence jumps the core out of its low power mode. Signed-off-by: Praveen Chidambaram pchid...@codeaurora.org Signed-off-by: Lina Iyer lina.i...@linaro.org Why isn't this code combined with the cpuidle driver in qcom-cpuidle.c, spm-devices.c, and qcom-pm.c? All these files are interacting with the same hardware, I'm confused why we have to have 4 different files and all these different patches to support this. Basically patches 3, 4, 6 and 7 should be one file under drivers/cpuidle/ named qcom-cpuidle or saw-cpuidle. They all interact with the one hardware, you are right about that. But each of these have a responsibility and provide certain functionality that builds up into the idle framework. Let me explain - The hardware driver spm.c doesnt care what the code calling the driver, intends to do. All it knows is to write to right register. And it can write to only one SPM block. There are multiple SPM blocks which need to be managed and thats provided by spm-devices. The cpuidle framework or the hotplug framework needs to do the same thing. The common functionality between idle and hotplug is abstraced out in msm-pm.c. platsmp.c and cpuidle-qcom.c both would need the same functionality. I can see the end result in the downstream vendor tree and it isn't pretty. The code winds through a handful of different files. The spm.c file is basically just a bunch of functions to read and write to an SPM. By itself it's pretty much worthless and doesn't stand on its own. spm-devices is where we would actually probe a driver for the device that is read/written in spm.c. At the least, these two files should be combined into one driver. Right now just to support cpuidle it seems that it could all be in one file. When we get to hotplug, why don't we use cpuidle_play_dead() on ARM so that this cpuidle driver can configure the SPM for the deepest idle state and power off the CPU? The only problem I see is that we would need another hook for the cpu_kill() op so that we can wait for the state machine to finish bringing down the CPU. That would remove the need for msm-pm.c? This would handle the cpuidle_play_dead() part. Alternatively we call cpuidle_play_dead() from the cpu_die() hook in the platform layer, but I'd prefer we call it directly in arch code if we can. diff --git a/arch/arm/kernel/smp.c b/arch/arm/kernel/smp.c index 7c4fada440f0..fef953ecde22 100644 --- a/arch/arm/kernel/smp.c +++ b/arch/arm/kernel/smp.c @@ -26,6 +26,7 @@ #include linux/completion.h #include linux/cpufreq.h #include linux/irq_work.h +#include linux/cpuidle.h #include linux/atomic.h #include asm/smp.h @@ -290,8 +291,9 @@ void __ref cpu_die(void) * The return path should not be used for platforms which can * power off the CPU. */ - if (smp_ops.cpu_die) - smp_ops.cpu_die(cpu); + if (cpuidle_play_dead()) + if (smp_ops.cpu_die) + smp_ops.cpu_die(cpu); pr_warn(CPU%u: smp_ops.cpu_die() returned, trying to resuscitate\n, cpu); The SPM is not simple register write. It needs quite a bit of configuration and to make it functional and then you may do register writes. SPM also provides voltage control functionality, which again has a lot of support code that need to ensure the hardware is in the correct state before you do that one register write to set the voltage. Again, this and other functionality add up to a whole lot of code to be clumped up in qcom-cpuidle.c and duplicated for hotplug again. It is beneficial to have them this way. Bear with me, as I build up this framework to support the idle and hotplug idle framework.
Re: [PATCH v4 7/8] qcom: cpuidle: Add cpuidle driver for QCOM cpus
On 08/20/2014 12:15 AM, Lina Iyer wrote:
Add cpuidle driver interface to allow cpus to go into C-States.
Use the cpuidle DT interfacecommon across ARM architectures to provide
the C-State information to the cpuidle framework.
Signed-off-by: Lina Iyer lina.i...@linaro.org
---
drivers/cpuidle/Kconfig.arm| 7 +++
drivers/cpuidle/Makefile | 1 +
drivers/cpuidle/cpuidle-qcom.c | 119 +
3 files changed, 127 insertions(+)
create mode 100644 drivers/cpuidle/cpuidle-qcom.c
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index e339c7f..26e31bd 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -63,3 +63,10 @@ config ARM_MVEBU_V7_CPUIDLE
depends on ARCH_MVEBU
help
Select this to enable cpuidle on Armada 370, 38x and XP processors.
+
+config ARM_QCOM_CPUIDLE
+ bool CPU Idle drivers for Qualcomm processors
+ depends on QCOM_PM
+ select DT_IDLE_STATES
+ help
+ Select this to enable cpuidle for QCOM processors
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 4d177b9..6c222d5 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_ARM_ZYNQ_CPUIDLE)+=
cpuidle-zynq.o
obj-$(CONFIG_ARM_U8500_CPUIDLE) += cpuidle-ux500.o
obj-$(CONFIG_ARM_AT91_CPUIDLE) += cpuidle-at91.o
obj-$(CONFIG_ARM_EXYNOS_CPUIDLE)+= cpuidle-exynos.o
+obj-$(CONFIG_ARM_QCOM_CPUIDLE) += cpuidle-qcom.o
###
# MIPS drivers
diff --git a/drivers/cpuidle/cpuidle-qcom.c b/drivers/cpuidle/cpuidle-qcom.c
new file mode 100644
index 000..4aae672
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-qcom.c
@@ -0,0 +1,119 @@
+/* Copyright (c) 2014, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2014, Linaro Limited.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/of.h
+#include linux/cpuidle.h
+#include linux/cpumask.h
+#include linux/slab.h
See below, cpumask is not needed, you can remove slab.h and cpumask.h.
+#include linux/of_device.h
+
+#include soc/qcom/pm.h
+#include dt_idle_states.h
+
+static enum msm_pm_sleep_mode modes[CPUIDLE_STATE_MAX];
+
+static int qcom_lpm_enter(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int index)
+{
+ return msm_cpu_pm_enter_sleep(modes[index], true);
You should return the index here.
+}
+
+static struct cpuidle_driver qcom_cpuidle_driver = {
+ .name = qcom_cpuidle,
+ .owner = THIS_MODULE,
+};
+
+static void parse_state_translations(struct cpuidle_driver *drv)
+{
+ struct device_node *state_node, *cpu_node;
+ const char *mode_name;
+ int i, j;
+
+ struct name_map {
+ enum msm_pm_sleep_mode mode;
+ char *name;
+ };
+ static struct name_map c_states[] = {
+ { MSM_PM_SLEEP_MODE_POWER_COLLAPSE_STANDALONE,
+ standalone-pc },
What is standalone-pc and collapse ? Core power down ?
+ { MSM_PM_SLEEP_MODE_WAIT_FOR_INTERRUPT, wfi },
+ };
+
+ cpu_node = of_cpu_device_node_get(cpumask_first(drv-cpumask));
+ if (!cpu_node)
+ return;
+
+ /**
+* Get the state description from idle-state node entry-method
+* First state is always WFI, per spec.
+*/
+ modes[0] = MSM_PM_SLEEP_MODE_WAIT_FOR_INTERRUPT;
+ for (i = 1; i drv-state_count; i++) {
+ mode_name = NULL;
+ state_node = of_parse_phandle(cpu_node, cpu-idle-states, i);
+ of_property_read_string(state_node, entry-method,
+ mode_name);
+ for (j = 0; mode_name (j ARRAY_SIZE(c_states)); j++) {
+ if (!strcmp(mode_name, c_states[j].name)) {
+ modes[i] = c_states[j].mode;
+ break;
+ }
+ }
+ }
+}
For the function above, I believe we can do better with the idle_dt_init
function to prevent to have to reparse the infos.
I had the opportunity to discuss with Lorenzo privately and we found a
solution he will submit to solve that.
+static int qcom_cpuidle_init(void)
+{
+ struct
