Date:   Mon, 30 Jan 2023 16:36:31 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Emil Renner Berthing <emil.renner.berthing@canonical.com>
Cc:     Uwe =?utf-8?Q?Kleine-K=C3=B6nig?= 
        <u.kleine-koenig@pengutronix.de>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Atish Patra <atishp@atishpatra.org>,
        "Wesley W. Terpstra" <wesley@sifive.com>,
        linux-pwm@vger.kernel.org, linux-riscv@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] pwm: sifive: Always let the first pwm_apply_state
 succeed
Message-ID: <Y9fj/72Tl5FN+HSf@orome>
References: <20221109113724.519021-1-emil.renner.berthing@canonical.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="ir6Gg7F9FhCRM+tI"
Content-Disposition: inline
In-Reply-To: <20221109113724.519021-1-emil.renner.berthing@canonical.com>
User-Agent: Mutt/2.2.9 (2022-11-12)
Precedence: bulk


--ir6Gg7F9FhCRM+tI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Nov 09, 2022 at 12:37:24PM +0100, Emil Renner Berthing wrote:
> 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.
>=20
> 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:
>=20
>   |          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       |
>=20
> 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 >=3D 2 both calls will fail
> with -EBUSY, which will then cause both LED drivers to fail to probe.
>=20
> Fix it by letting the first call to pwm_sifive_apply update the clock
> even when ddata->user_count !=3D 1.
>=20
> Fixes: 9e37a53eb051 ("pwm: sifive: Add a driver for SiFive SoC PWM")
> Signed-off-by: Emil Renner Berthing <emil.renner.berthing@canonical.com>
> ---
>  drivers/pwm/pwm-sifive.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

I've applied this as-is for now. What I'm wondering is if perhaps we
want to implement something into the PWM core to deal with this, now
fairly common, situation.

Thierry

--ir6Gg7F9FhCRM+tI
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmPX4/wACgkQ3SOs138+
s6GN+Q/9GErvttbzCfQ8UlLEkSt6ZyC2QIVVZhEDF7LoH4jprLifKAEsGMR9BK5y
XS2nldqcUtb9q/p6hVdnGcMjyDmoH9OLoVeM3PX56BaeKYaEBdUeg97uFAwsIJU4
TdcFpAzIVKNCsPyK0wwPw2P4bWyiEs1+rf6Go4jNu7aZE77yI11TrhjUxXBc++9g
EcDChQ2pu3PJgCaBx16GZ4RQ5CgkR7JSL50ZohmKbdO/8asYi3aGEBxo5YN6BTaV
NKI2H1n1XH1KKC6m/3/eIC2fM3bv9q+JqRe7vJ9kalxTdYnwZKglRv2WyFsA0kF1
e1bTJrq3P3fpZVfY+dH1SfddToqGftPOc5vBd+NnK/brfmX+uCpg+vDoo/YeP+jU
TDh9FZkWGMJ7XhZh1A65QQazsKfo67gyG5xKZZU/XtT6U9vUXBAffpzWryeS+ulr
iDo6WjhtqBw+7hk57paDn5f49wVJIRijvNU0dAVIGGxaZceiAo1WJsaC7zUxrvq+
9fCZb/o7bfsnFd+/QG8tVcuZ41dKTrqlXHU70qzhXJ4tBxE8NVrk0hG8AmghyGt3
oQiyNw4LJE777ES/OVpkndtr4DMaERL7+e6ZAqJ8w/YQNHEedacHM8KopOrGUTHp
f33dJEjjcqFU3r0PWfo9mRRaxLF7AeWJxOdlMUK28f1k5fQo+V4=
=oOaj
-----END PGP SIGNATURE-----

--ir6Gg7F9FhCRM+tI--