Re: stable-rc: 5.4: drivers/pwm/pwm-meson.c:313:25: error: assignment of member 'polarity' in read-only object

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hello,

On Mon, Apr 24, 2023 at 03:16:34AM +0530, Naresh Kamboju wrote:
> Following build errors noticed on arm64 and arm while building stable-rc 5.4.
> 
> Reported-by: Linux Kernel Functional Testing <lkft@xxxxxxxxxx>
> 
> Build error:
> -----------
>   drivers/pwm/pwm-meson.c: In function 'meson_pwm_apply':
>   drivers/pwm/pwm-meson.c:313:25: error: assignment of member
> 'polarity' in read-only object
>     313 |         state->polarity = PWM_POLARITY_NORMAL;
>         |                         ^
>   make[3]: *** [scripts/Makefile.build:262: drivers/pwm/pwm-meson.o] Error 1
>   make[3]: Target '__build' not remade because of errors.
>   make[2]: *** [scripts/Makefile.build:497: drivers/pwm] Error 2
> 
> 
> Suspected commit:
> ------------
>   pwm: meson: Explicitly set .polarity in .get_state()
>   commit 8caa81eb950cb2e9d2d6959b37d853162d197f57 upstream.

Thanks for the report. I sent a fix to that patch in reply to the patch
being announced to be picked for 5.4.y, but failed to Cc you. I just
replied to the mail, so it didn't even go to stable@xxxxxxxxxxxxxxx.

For reference, here comes the fixed patch again, see below.

Best regards
Uwe

------->8--------
From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
Date: Wed, 22 Mar 2023 22:45:44 +0100
Subject: [PATCH] pwm: meson: Explicitly set .polarity in .get_state()

[ Upstream commit 8caa81eb950cb2e9d2d6959b37d853162d197f57 ]

The driver only supports normal polarity. Complete the implementation of
.get_state() by setting .polarity accordingly.

This fixes a regression that was possible since commit c73a3107624d
("pwm: Handle .get_state() failures") which stopped to zero-initialize
the state passed to the .get_state() callback. This was reported at
https://forum.odroid.com/viewtopic.php?f=177&t=46360 . While this was an
unintended side effect, the real issue is the driver's callback not
setting the polarity.

There is a complicating fact, that the .apply() callback fakes support
for inversed polarity. This is not (and cannot) be matched by
.get_state(). As fixing this isn't easy, only point it out in a comment
to prevent authors of other drivers from copying that approach.

Fixes: c375bcbaabdb ("pwm: meson: Read the full hardware state in meson_pwm_get_state()")
Reported-by: Munehisa Kamata <kamatam@xxxxxxxxxx>
Acked-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
Link: https://lore.kernel.org/r/20230310191405.2606296-1-u.kleine-koenig@xxxxxxxxxxxxxx
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
Signed-off-by: Thierry Reding <thierry.reding@xxxxxxxxx>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
---
 drivers/pwm/pwm-meson.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pwm/pwm-meson.c b/drivers/pwm/pwm-meson.c
index 6245bbdb6e6c..3c81858a8261 100644
--- a/drivers/pwm/pwm-meson.c
+++ b/drivers/pwm/pwm-meson.c
@@ -168,6 +168,12 @@ static int meson_pwm_calc(struct meson_pwm *meson, struct pwm_device *pwm,
 	duty = state->duty_cycle;
 	period = state->period;
 
+	/*
+	 * Note this is wrong. The result is an output wave that isn't really
+	 * inverted and so is wrongly identified by .get_state as normal.
+	 * Fixing this needs some care however as some machines might rely on
+	 * this.
+	 */
 	if (state->polarity == PWM_POLARITY_INVERSED)
 		duty = period - duty;
 
@@ -366,6 +372,7 @@ static void meson_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
 		state->period = 0;
 		state->duty_cycle = 0;
 	}
+	state->polarity = PWM_POLARITY_NORMAL;
 }
 
 static const struct pwm_ops meson_pwm_ops = {
-- 
2.39.2

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

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux