linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
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:54:21 +0200	[thread overview]
Message-ID: <20230524075421.00ee0e57@karo-electronics.de> (raw)
In-Reply-To: <20230524074607.41ffc31b@karo-electronics.de>

Hi,

please ignore this. The V2 patch does not apply. Last minute changes are
always a bad idea...

Lothar Waßmann
On Wed, 24 May 2023 07:46:07 +0200 Lothar Waßmann wrote:
> 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, &param, &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;
>  }
>  

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-05-24  5:55 UTC|newest]

Thread overview: 4+ 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:46 ` [PATCH V2] " Lothar Waßmann
2023-05-24  5:54   ` Lothar Waßmann [this message]
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=20230524075421.00ee0e57@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 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).