From: Evan Green <evgreen@chromium.org>
To: andy.gross@linaro.org, david.brown@linaro.org,
Karthikeyan Ramasubramanian <kramasub@codeaurora.org>,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
swboyd@chromium.org, dianders@chromium.org,
Girish Mahadevan <girishm@codeaurora.org>,
linux-i2c@vger.kernel.org
Cc: Evan Green <evgreen@chromium.org>
Subject: [PATCH] i2c: i2c-qcom-geni: Fix suspend clock handling
Date: Mon, 9 Jul 2018 16:49:37 -0700 [thread overview]
Message-ID: <20180709234937.176154-1-evgreen@chromium.org> (raw)
pm_runtime_suspended can return 0 even if the last runtime power
management function called in the device was a suspend call. This
trips up the i2c-qcom-geni's suspend_noirq accounting, which was
trying to use the function to determine if it should disable clocks
or if that had already been done.
The fix is to track whether or not clocks have been enabled explicitly
in the driver with a new member. While in there, also honor the return
value of geni_se_resources_off, which can technically fail.
An example of the warning generated during suspend:
[ 68.314733] WARNING: CPU: 0 PID: 1990 at drivers/clk/clk.c:595 clk_core_disable+0x28/0x200
[ 68.464576] Call trace:
[ 68.554410] [<ffffff80083f0ed0>] clk_core_disable+0x28/0x200
[ 68.560244] [<ffffff80083f349c>] clk_disable+0x38/0x4c
[ 68.565541] [<ffffff8008408910>] geni_se_resources_off+0x40/0x74
[ 68.571731] [<ffffff800862aadc>] geni_i2c_runtime_suspend+0x2c/0x3c
[ 68.578185] [<ffffff800862ae6c>] geni_i2c_suspend_noirq+0x38/0x68
[ 68.584475] [<ffffff800852bd40>] dpm_run_callback+0x104/0x210
[ 68.590404] [<ffffff800852c3d4>] __device_suspend_noirq+0x158/0x1a8
[ 68.596859] [<ffffff800852dff0>] dpm_noirq_suspend_devices+0x180/0x38c
[ 68.603594] [<ffffff80080fdebc>] suspend_devices_and_enter+0x49c/0x964
[ 68.610321] [<ffffff80080fea14>] pm_suspend+0x690/0x6e0
[ 68.615712] [<ffffff80080fc930>] state_store+0xd4/0xf8
[ 68.621014] [<ffffff80088a57f0>] kobj_attr_store+0x18/0x28
[ 68.626672] [<ffffff8008297a54>] sysfs_kf_write+0x5c/0x68
[ 68.632240] [<ffffff8008296a00>] kernfs_fop_write+0x174/0x1b8
[ 68.638177] [<ffffff800821a2c4>] __vfs_write+0x58/0x160
[ 68.643567] [<ffffff800821a5d8>] vfs_write+0xcc/0x184
[ 68.648780] [<ffffff800821a850>] SyS_write+0x64/0xb4
Signed-off-by: Evan Green <evgreen@chromium.org>
---
This change applies atop Karthik's patch [1], which as far as I can tell
technically hasn't landed yet, but also hasn't had feedback in awhile
either. I opted to submit this as a separate patch, but if the maintainers
prefer it's fine with me if this get folded in to another spin of the
base series.
[1] https://patchwork.kernel.org/patch/10460819/
drivers/i2c/busses/i2c-qcom-geni.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 54bcd1a79cc5..90c4cbfa22d8 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -85,6 +85,7 @@ struct geni_i2c_dev {
spinlock_t lock;
u32 clk_freq_out;
const struct geni_i2c_clk_fld *clk_fld;
+ int suspended;
};
struct geni_i2c_err_log {
@@ -567,9 +568,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
geni_se_config_packing(&gi2c->se, BITS_PER_BYTE, PACKING_BYTES_PW,
true, true, true);
- geni_se_resources_off(&gi2c->se);
+ ret = geni_se_resources_off(&gi2c->se);
+ if (ret) {
+ dev_err(&pdev->dev, "Error turning off resources %d\n", ret);
+ return ret;
+ }
+
dev_dbg(&pdev->dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
+ gi2c->suspended = 1;
pm_runtime_set_suspended(gi2c->se.dev);
pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
pm_runtime_use_autosuspend(gi2c->se.dev);
@@ -590,10 +597,19 @@ static int geni_i2c_remove(struct platform_device *pdev)
static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
{
+ int ret;
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
disable_irq(gi2c->irq);
- geni_se_resources_off(&gi2c->se);
+ ret = geni_se_resources_off(&gi2c->se);
+ if (ret) {
+ enable_irq(gi2c->irq);
+ return ret;
+
+ } else {
+ gi2c->suspended = 1;
+ }
+
return 0;
}
@@ -607,12 +623,15 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
return ret;
enable_irq(gi2c->irq);
+ gi2c->suspended = 0;
return 0;
}
static int __maybe_unused geni_i2c_suspend_noirq(struct device *dev)
{
- if (!pm_runtime_suspended(dev)) {
+ struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
+
+ if (!gi2c->suspended) {
geni_i2c_runtime_suspend(dev);
pm_runtime_disable(dev);
pm_runtime_set_suspended(dev);
--
2.18.0.203.gfac676dfb9-goog
next reply other threads:[~2018-07-09 23:50 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-07-09 23:49 Evan Green [this message]
2018-07-12 22:03 ` [PATCH] i2c: i2c-qcom-geni: Fix suspend clock handling Wolfram Sang
2018-07-13 22:21 ` Karthik Ramasubramanian
2018-07-13 22:24 ` Evan Green
2018-07-31 19:20 ` Wolfram Sang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180709234937.176154-1-evgreen@chromium.org \
--to=evgreen@chromium.org \
--cc=andy.gross@linaro.org \
--cc=david.brown@linaro.org \
--cc=dianders@chromium.org \
--cc=girishm@codeaurora.org \
--cc=kramasub@codeaurora.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=swboyd@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox