2024-02-19 03:39:11

by Raag Jadav

[permalink] [raw]
Subject: [PATCH v4 1/4] pwm: dwc: Fix PM regression

While preparing dwc driver for devm_pwmchip_alloc() usage, commit
df41cd8bbcad ("pwm: dwc: Prepare removing pwm_chip from driver data")
modified ->suspend() handle to use the pwm_chip as driver_data for
accessing struct dwc_pwm, but didn't modify ->resume() handle with
relevant changes. This results into illegal memory access during
device wakeup and causes a PM regression.

Fix this by correctly accessing struct dwc_pwm in ->resume() handle.

Fixes: df41cd8bbcad ("pwm: dwc: Prepare removing pwm_chip from driver data")
Signed-off-by: Raag Jadav <[email protected]>
---
drivers/pwm/pwm-dwc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
index 8ca1c20a6aaf..c0e586688e57 100644
--- a/drivers/pwm/pwm-dwc.c
+++ b/drivers/pwm/pwm-dwc.c
@@ -95,7 +95,8 @@ static int dwc_pwm_suspend(struct device *dev)

static int dwc_pwm_resume(struct device *dev)
{
- struct dwc_pwm *dwc = dev_get_drvdata(dev);
+ struct pwm_chip *chip = dev_get_drvdata(dev);
+ struct dwc_pwm *dwc = to_dwc_pwm(chip);
int i;

for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
--
2.35.3



2024-02-19 07:22:54

by Uwe Kleine-König

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] pwm: dwc: Fix PM regression

Hello Raag,

On Mon, Feb 19, 2024 at 09:08:32AM +0530, Raag Jadav wrote:
> While preparing dwc driver for devm_pwmchip_alloc() usage, commit
> df41cd8bbcad ("pwm: dwc: Prepare removing pwm_chip from driver data")
> modified ->suspend() handle to use the pwm_chip as driver_data for
> accessing struct dwc_pwm, but didn't modify ->resume() handle with
> relevant changes. This results into illegal memory access during
> device wakeup and causes a PM regression.
>
> Fix this by correctly accessing struct dwc_pwm in ->resume() handle.
>
> Fixes: df41cd8bbcad ("pwm: dwc: Prepare removing pwm_chip from driver data")
> Signed-off-by: Raag Jadav <[email protected]>
> ---
> drivers/pwm/pwm-dwc.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> index 8ca1c20a6aaf..c0e586688e57 100644
> --- a/drivers/pwm/pwm-dwc.c
> +++ b/drivers/pwm/pwm-dwc.c
> @@ -95,7 +95,8 @@ static int dwc_pwm_suspend(struct device *dev)
>
> static int dwc_pwm_resume(struct device *dev)
> {
> - struct dwc_pwm *dwc = dev_get_drvdata(dev);
> + struct pwm_chip *chip = dev_get_drvdata(dev);
> + struct dwc_pwm *dwc = to_dwc_pwm(chip);
> int i;
>
> for (i = 0; i < DWC_TIMERS_TOTAL; i++) {

If you're ok I'd squash this into df41cd8bbcad adding

Thanks to Raag Jadav for providing a hunk of this patch that Uwe
missed during creation of this patch.

to the commit log.

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-K?nig |
Industrial Linux Solutions | https://www.pengutronix.de/ |


Attachments:
(No filename) (1.61 kB)
signature.asc (499.00 B)
Download all attachments

2024-02-19 09:06:40

by Raag Jadav

[permalink] [raw]
Subject: Re: [PATCH v4 1/4] pwm: dwc: Fix PM regression

On Mon, Feb 19, 2024 at 08:22:15AM +0100, Uwe Kleine-K?nig wrote:
> Hello Raag,
>
> On Mon, Feb 19, 2024 at 09:08:32AM +0530, Raag Jadav wrote:
> > While preparing dwc driver for devm_pwmchip_alloc() usage, commit
> > df41cd8bbcad ("pwm: dwc: Prepare removing pwm_chip from driver data")
> > modified ->suspend() handle to use the pwm_chip as driver_data for
> > accessing struct dwc_pwm, but didn't modify ->resume() handle with
> > relevant changes. This results into illegal memory access during
> > device wakeup and causes a PM regression.
> >
> > Fix this by correctly accessing struct dwc_pwm in ->resume() handle.
> >
> > Fixes: df41cd8bbcad ("pwm: dwc: Prepare removing pwm_chip from driver data")
> > Signed-off-by: Raag Jadav <[email protected]>
> > ---
> > drivers/pwm/pwm-dwc.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pwm/pwm-dwc.c b/drivers/pwm/pwm-dwc.c
> > index 8ca1c20a6aaf..c0e586688e57 100644
> > --- a/drivers/pwm/pwm-dwc.c
> > +++ b/drivers/pwm/pwm-dwc.c
> > @@ -95,7 +95,8 @@ static int dwc_pwm_suspend(struct device *dev)
> >
> > static int dwc_pwm_resume(struct device *dev)
> > {
> > - struct dwc_pwm *dwc = dev_get_drvdata(dev);
> > + struct pwm_chip *chip = dev_get_drvdata(dev);
> > + struct dwc_pwm *dwc = to_dwc_pwm(chip);
> > int i;
> >
> > for (i = 0; i < DWC_TIMERS_TOTAL; i++) {
>
> If you're ok I'd squash this into df41cd8bbcad adding
>
> Thanks to Raag Jadav for providing a hunk of this patch that Uwe
> missed during creation of this patch.
>
> to the commit log.

Works for me.

Raag