12 Commits

Author SHA1 Message Date
Emil Renner Berthing
c2677c49b7 pwm: sifive: Always let the first pwm_apply_state succeed
[ Upstream commit 334c7b13d38321e47d1a51dba0bef9f4c403ec75 ]

Commit 2cfe9bbec56ea579135cdd92409fff371841904f added support for the
RGB and green PWM controlled LEDs on the HiFive Unmatched board
managed by the leds-pwm-multicolor and leds-pwm drivers respectively.
All three colours of the RGB LED and the green LED run from different
lines of the same PWM, but with the same period so this works fine when
the LED drivers are loaded one after the other.

Unfortunately it does expose a race in the PWM driver when both LED
drivers are loaded at roughly the same time. Here is an example:

  |          Thread A           |          Thread B           |
  |  led_pwm_mc_probe           |  led_pwm_probe              |
  |    devm_fwnode_pwm_get      |                             |
  |      pwm_sifive_request     |                             |
  |        ddata->user_count++  |                             |
  |                             |    devm_fwnode_pwm_get      |
  |                             |      pwm_sifive_request     |
  |                             |        ddata->user_count++  |
  |         ...                 |          ...                |
  |    pwm_state_apply          |    pwm_state_apply          |
  |      pwm_sifive_apply       |      pwm_sifive_apply       |

Now both calls to pwm_sifive_apply will see that ddata->approx_period,
initially 0, is different from the requested period and the clock needs
to be updated. But since ddata->user_count >= 2 both calls will fail
with -EBUSY, which will then cause both LED drivers to fail to probe.

Fix it by letting the first call to pwm_sifive_apply update the clock
even when ddata->user_count != 1.

Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-03-11 16:40:08 +01:00
Uwe Kleine-König
8b98e7a45e pwm: sifive: Reduce time the controller lock is held
[ Upstream commit 0f02f491b786143f08eb19840f1cf4f12aec6dee ]

The lock is only to serialize access and update to user_count and
approx_period between different PWMs served by the same pwm_chip.
So the lock needs only to be taken during the check if the (chip global)
period can and/or needs to be changed.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Stable-dep-of: 334c7b13d383 ("pwm: sifive: Always let the first pwm_apply_state succeed")
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-03-11 16:40:08 +01:00
Uwe Kleine-König
6a65f4644b pwm: sifive: Call pwm_sifive_update_clock() while mutex is held
[ Upstream commit 45558b3abb87eeb2cedb8a59cb2699c120b5102a ]

As was documented in commit 0f02f491b786 ("pwm: sifive: Reduce time the
controller lock is held") a caller of pwm_sifive_update_clock() must
hold the mutex. So fix pwm_sifive_clock_notifier() to grab the lock.

While this necessity was only documented later, the race exists since
the driver was introduced.

Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
Reported-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Link: https://lore.kernel.org/r/20221018061656.1428111-1-u.kleine-koenig@pengutronix.de
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2023-01-14 10:16:07 +01:00
Uwe Kleine-König
6aaac1d924 pwm: sifive: Shut down hardware only after pwmchip_remove() completed
[ Upstream commit 2375e964d541bb09158cd2dff67b5d74e8de61cd ]

The PWMs are expected to be functional until pwmchip_remove() is called.
So disable the clks only afterwards.

Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-08-21 15:15:37 +02:00
Uwe Kleine-König
9073dbec88 pwm: sifive: Ensure the clk is enabled exactly once per running PWM
[ Upstream commit ace41d7564e655c39f709a78c035188a460c7cbd ]

.apply() assumes the clk to be for a given PWM iff the PWM is enabled.
So make sure this is the case when .probe() completes. And in .remove()
disable the according number of times.

This fixes a clk enable/disable imbalance, if some PWMs are already running
at probe time.

Fixes: 9e37a53eb051 (pwm: sifive: Add a driver for SiFive SoC PWM)
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-08-21 15:15:37 +02:00
Uwe Kleine-König
47902de24a pwm: sifive: Simplify offset calculation for PWMCMP registers
[ Upstream commit 20550a61880fc55e68a0d290ad195b74729c0e7b ]

Instead of explicitly using PWM_SIFIVE_PWMCMP0 + pwm->hwpwm *
PWM_SIFIVE_SIZE_PWMCMP for each access to one of the PWMCMP registers,
introduce a macro that takes the hwpwm id as parameter.

For the register definition using a plain 4 instead of the cpp constant
PWM_SIFIVE_SIZE_PWMCMP is easier to read, so define the offset macro
without the constant. The latter can then be dropped as there are no
users left.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Tested-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-08-21 15:15:37 +02:00
Uwe Kleine-König
6d7f7ffbcd pwm: sifive: Don't check the return code of pwmchip_remove()
[ Upstream commit ceb2c2842f3664dcc4e6d8cb317e1e83bb81b1e5 ]

pwmchip_remove() returns always 0. Don't use the value to make it
possible to eventually change the function to return void. Also the
driver core ignores the return value of pwm_sifive_remove().

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
2022-08-21 15:15:37 +02:00
Krzysztof Kozlowski
5530fcaf9c pwm: sifive: Simplify with dev_err_probe()
Common pattern of handling deferred probe can be simplified with
dev_err_probe().  Less code and also it prints the error value.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
2020-09-24 09:18:11 +02:00
Guru Das Srinagesh
4cc23430a5 pwm: sifive: Use 64-bit division macro
Since the PWM framework is switching struct pwm_args.period's datatype
to u64, prepare for this transition by using DIV64_U64_ROUND_CLOSEST to
handle a 64-bit divisor.

Signed-off-by: Guru Das Srinagesh <gurus@codeaurora.org>
Acked-by: Palmer Dabbelt <palmerdabbelt@google.com>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
2020-06-17 20:42:09 +02:00
Uwe Kleine-König
71523d1812 pwm: Ensure pwm_apply_state() doesn't modify the state argument
It is surprising for a PWM consumer when the variable holding the
requested state is modified by pwm_apply_state(). Consider for example a
driver doing:

        #define PERIOD 5000000
        #define DUTY_LITTLE 10
        ...
        struct pwm_state state = {
                .period = PERIOD,
                .duty_cycle = DUTY_LITTLE,
                .polarity = PWM_POLARITY_NORMAL,
                .enabled = true,
        };

        pwm_apply_state(mypwm, &state);
        ...
        state.duty_cycle = PERIOD / 2;
        pwm_apply_state(mypwm, &state);

For sure the second call to pwm_apply_state() should still have
state.period = PERIOD and not something the hardware driver chose for a
reason that doesn't necessarily apply to the second call.

So declare the state argument as a pointer to a const type and adapt all
drivers' .apply callbacks.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
2019-09-21 03:25:10 +02:00
Ding Xiang
f6abac0379 pwm: sifive: Remove redundant error message
devm_ioremap_resource() already outputs an error message, so remove the
extra error message on failure.

Signed-off-by: Ding Xiang <dingxiang@cmss.chinamobile.com>
Acked-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
2019-09-21 00:26:16 +02:00
Yash Shah
9e37a53eb0 pwm: sifive: Add a driver for SiFive SoC PWM
Adds a PWM driver for PWM chip present in SiFive's HiFive Unleashed SoC.

Signed-off-by: Wesley W. Terpstra <wesley@sifive.com>
[Atish: Various fixes and code cleanup]
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Signed-off-by: Yash Shah <yash.shah@sifive.com>
Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Signed-off-by: Thierry Reding <thierry.reding@gmail.com>
2019-06-25 14:48:12 +02:00