From: "Lothar Waßmann" <LW@KARO-electronics.de>
To: linux-arm-kernel@lists.infradead.org
Cc: linux-pwm@vger.kernel.org, Lee Jones <lee.jones@linaro.org>,
NXP Linux Team <linux-imx@nxp.com>,
Fabio Estevam <festevam@gmail.com>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Shawn Guo <shawnguo@kernel.org>,
Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH V2] pwm: imx-tpm: fix kernel crash upon resume due to register access with clocks disabled
Date: Wed, 24 May 2023 07:46:07 +0200 [thread overview]
Message-ID: <20230524074607.41ffc31b@karo-electronics.de> (raw)
In-Reply-To: <20230524052714.3077-1-LW@KARO-electronics.de>
If the pwm-imx-tpm driver is being used e.g. for backlight, the
pwm_imx_tpm_apply() function is being called before the device is
resumed and the clocks are enabled, resulting in a data abort:
echo +5 > /sys/class/rtc/rtc0/wakealarm;echo mem > /sys/power/state
PM: suspend entry (deep)
Filesystems sync: 0.006 seconds
Freezing user space processes ... (elapsed 0.015 seconds) done.
OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
Disabling non-boot CPUs ...
psci: CPU1 killed (polled 0 ms)
Enabling non-boot CPUs ...
Detected VIPT I-cache on CPU1
cacheinfo: Unable to detect cache hierarchy for CPU 1
GICv3: CPU1: found redistributor 100 region 0:0x0000000048060000
CPU1: Booted secondary processor 0x0000000100 [0x412fd050]
CPU1 is up
Internal error: synchronous external abort: 0000000096000010 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 245 Comm: bash Not tainted 6.1.1-karo+g29549c7073bf #1
Hardware name: Ka-Ro electronics GmbH TX93-5210 (NXP i.MX93) module (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : pwm_imx_tpm_apply+0x12c/0x3f0
lr : pwm_imx_tpm_apply+0x104/0x3f0
sp : ffff80000a11b710
x29: ffff80000a11b710 x28: 0000000000000001 x27: 0000000000000000
x26: 0000000000000000 x25: 000000000007a12a x24: 0000000000008236
x23: 0000000000000000 x22: ffff00000309b5d0 x21: 0000000000000000
x20: ffff0000022b0a00 x19: ffff00000309b580 x18: 3030387830383231
x17: 0048008800d90326 x16: 0324032303260320 x15: ffff80000a11b780
x14: ffff80000a11b830 x13: ffff80000a11b834 x12: ffff00003fd90740
x11: ffff00000391da00 x10: 00000000000007d0 x9 : ffff80000a11b6b0
x8 : ffff00000391e230 x7 : 0000000000000000 x6 : ffff00000391da00
x5 : 000000001dcd6500 x4 : 0000000000000000 x3 : ffff00000309b5d0
x2 : ffff00000391da00 x1 : 0000000000000000 x0 : ffff800008fdd010
Call trace:
pwm_imx_tpm_apply+0x12c/0x3f0
pwm_apply_state+0x5c/0xbc
pwm_backlight_update_status+0xc4/0x1ac
drm_panel_enable+0x70/0xe0
[...]
Fix this by remembering the suspend state and returning -EAGAIN in HW
related functions (pwm_imx_tpm_apply() and pwm_imx_tpm_get_state()) as
long as pwm_imx_tpm_resume() has not been called.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
V2: one hunk was missing in the first mail
---
drivers/pwm/pwm-imx-tpm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index e5e7b7c339a8..537c7182f988 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -64,6 +64,7 @@ struct imx_tpm_pwm_chip {
u32 user_count;
u32 enable_count;
u32 real_period;
+ int suspended;
};
struct imx_tpm_pwm_param {
@@ -140,6 +141,9 @@ static void pwm_imx_tpm_get_state(struct pwm_chip *chip,
u32 rate, val, prescale;
u64 tmp;
+ if (tpm->suspended)
+ return -EAGAIN;
+
/* get period */
state->period = tpm->real_period;
@@ -294,6 +298,9 @@ static int pwm_imx_tpm_apply(struct pwm_chip *chip,
struct pwm_state real_state;
int ret;
+ if (tpm->suspended)
+ return -EAGAIN;
+
ret = pwm_imx_tpm_round_state(chip, ¶m, &real_state, state);
if (ret)
return ret;
@@ -397,6 +404,7 @@ static int __maybe_unused pwm_imx_tpm_suspend(struct device *dev)
if (tpm->enable_count > 0)
return -EBUSY;
+ tpm->suspended = 1;
clk_disable_unprepare(tpm->clk);
return 0;
@@ -411,6 +419,7 @@ static int __maybe_unused pwm_imx_tpm_resume(struct device *dev)
if (ret)
dev_err(dev, "failed to prepare or enable clock: %d\n", ret);
+ tpm->suspended = 0;
return ret;
}
--
2.30.2
WARNING: multiple messages have this Message-ID (diff)
From: "Lothar Waßmann" <LW@KARO-electronics.de>
To: linux-arm-kernel@lists.infradead.org
Cc: linux-pwm@vger.kernel.org, Lee Jones <lee.jones@linaro.org>,
NXP Linux Team <linux-imx@nxp.com>,
Fabio Estevam <festevam@gmail.com>,
Pengutronix Kernel Team <kernel@pengutronix.de>,
Shawn Guo <shawnguo@kernel.org>,
Thierry Reding <thierry.reding@gmail.com>
Subject: Re: [PATCH V2] pwm: imx-tpm: fix kernel crash upon resume due to register access with clocks disabled
Date: Wed, 24 May 2023 07:46:07 +0200 [thread overview]
Message-ID: <20230524074607.41ffc31b@karo-electronics.de> (raw)
In-Reply-To: <20230524052714.3077-1-LW@KARO-electronics.de>
If the pwm-imx-tpm driver is being used e.g. for backlight, the
pwm_imx_tpm_apply() function is being called before the device is
resumed and the clocks are enabled, resulting in a data abort:
echo +5 > /sys/class/rtc/rtc0/wakealarm;echo mem > /sys/power/state
PM: suspend entry (deep)
Filesystems sync: 0.006 seconds
Freezing user space processes ... (elapsed 0.015 seconds) done.
OOM killer disabled.
Freezing remaining freezable tasks ... (elapsed 0.001 seconds) done.
Disabling non-boot CPUs ...
psci: CPU1 killed (polled 0 ms)
Enabling non-boot CPUs ...
Detected VIPT I-cache on CPU1
cacheinfo: Unable to detect cache hierarchy for CPU 1
GICv3: CPU1: found redistributor 100 region 0:0x0000000048060000
CPU1: Booted secondary processor 0x0000000100 [0x412fd050]
CPU1 is up
Internal error: synchronous external abort: 0000000096000010 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 PID: 245 Comm: bash Not tainted 6.1.1-karo+g29549c7073bf #1
Hardware name: Ka-Ro electronics GmbH TX93-5210 (NXP i.MX93) module (DT)
pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : pwm_imx_tpm_apply+0x12c/0x3f0
lr : pwm_imx_tpm_apply+0x104/0x3f0
sp : ffff80000a11b710
x29: ffff80000a11b710 x28: 0000000000000001 x27: 0000000000000000
x26: 0000000000000000 x25: 000000000007a12a x24: 0000000000008236
x23: 0000000000000000 x22: ffff00000309b5d0 x21: 0000000000000000
x20: ffff0000022b0a00 x19: ffff00000309b580 x18: 3030387830383231
x17: 0048008800d90326 x16: 0324032303260320 x15: ffff80000a11b780
x14: ffff80000a11b830 x13: ffff80000a11b834 x12: ffff00003fd90740
x11: ffff00000391da00 x10: 00000000000007d0 x9 : ffff80000a11b6b0
x8 : ffff00000391e230 x7 : 0000000000000000 x6 : ffff00000391da00
x5 : 000000001dcd6500 x4 : 0000000000000000 x3 : ffff00000309b5d0
x2 : ffff00000391da00 x1 : 0000000000000000 x0 : ffff800008fdd010
Call trace:
pwm_imx_tpm_apply+0x12c/0x3f0
pwm_apply_state+0x5c/0xbc
pwm_backlight_update_status+0xc4/0x1ac
drm_panel_enable+0x70/0xe0
[...]
Fix this by remembering the suspend state and returning -EAGAIN in HW
related functions (pwm_imx_tpm_apply() and pwm_imx_tpm_get_state()) as
long as pwm_imx_tpm_resume() has not been called.
Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
---
V2: one hunk was missing in the first mail
---
drivers/pwm/pwm-imx-tpm.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/pwm/pwm-imx-tpm.c b/drivers/pwm/pwm-imx-tpm.c
index e5e7b7c339a8..537c7182f988 100644
--- a/drivers/pwm/pwm-imx-tpm.c
+++ b/drivers/pwm/pwm-imx-tpm.c
@@ -64,6 +64,7 @@ struct imx_tpm_pwm_chip {
u32 user_count;
u32 enable_count;
u32 real_period;
+ int suspended;
};
struct imx_tpm_pwm_param {
@@ -140,6 +141,9 @@ static void pwm_imx_tpm_get_state(struct pwm_chip *chip,
u32 rate, val, prescale;
u64 tmp;
+ if (tpm->suspended)
+ return -EAGAIN;
+
/* get period */
state->period = tpm->real_period;
@@ -294,6 +298,9 @@ static int pwm_imx_tpm_apply(struct pwm_chip *chip,
struct pwm_state real_state;
int ret;
+ if (tpm->suspended)
+ return -EAGAIN;
+
ret = pwm_imx_tpm_round_state(chip, ¶m, &real_state, state);
if (ret)
return ret;
@@ -397,6 +404,7 @@ static int __maybe_unused pwm_imx_tpm_suspend(struct device *dev)
if (tpm->enable_count > 0)
return -EBUSY;
+ tpm->suspended = 1;
clk_disable_unprepare(tpm->clk);
return 0;
@@ -411,6 +419,7 @@ static int __maybe_unused pwm_imx_tpm_resume(struct device *dev)
if (ret)
dev_err(dev, "failed to prepare or enable clock: %d\n", ret);
+ tpm->suspended = 0;
return ret;
}
--
2.30.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2023-05-24 5:46 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-24 5:27 [PATCH] pwm: imx-tpm: fix kernel crash upon resume due to register access with clocks disabled Lothar Waßmann
2023-05-24 5:27 ` Lothar Waßmann
2023-05-24 5:46 ` Lothar Waßmann [this message]
2023-05-24 5:46 ` [PATCH V2] " Lothar Waßmann
2023-05-24 5:54 ` Lothar Waßmann
2023-05-24 5:54 ` Lothar Waßmann
2023-05-24 12:15 ` Fabio Estevam
2023-05-24 12:15 ` Fabio Estevam
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=20230524074607.41ffc31b@karo-electronics.de \
--to=lw@karo-electronics.de \
--cc=festevam@gmail.com \
--cc=kernel@pengutronix.de \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-imx@nxp.com \
--cc=linux-pwm@vger.kernel.org \
--cc=shawnguo@kernel.org \
--cc=thierry.reding@gmail.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.