All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon South <simon@simonsouth.net>
To: tpiepho@gmail.com, thierry.reding@gmail.com,
	u.kleine-koenig@pengutronix.de, robin.murphy@arm.com,
	lee.jones@linaro.org, heiko@sntech.de, bbrezillon@kernel.org,
	linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Cc: simon@simonsouth.net
Subject: [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing
Date: Wed, 23 Dec 2020 11:01:02 -0500	[thread overview]
Message-ID: <cover.1608735481.git.simon@simonsouth.net> (raw)

This patch series aims to eliminate the race condition Trent Piepho
identified[0] in the Rockchip PWM driver's rockchip_pwm_probe()
function, by moving code that checks whether a device is enabled ahead
of the code that registers it via pwmchip_add().

It has grown to include a number of other small fixes and improvements
to the driver. It now also

- Fixes a potential kernel hang introduced by my earlier commit
  457f74abbed0 ("pwm: rockchip: Keep enabled PWMs running while
  probing") by ensuring a device's APB clock is enabled before its
  registers are accessed;

- Removes a superfluous call to clk_unprepare() that could result in
  warnings from the kernel;

- Clarifies the driver's error messages by replacing "bus clk" with
  "PWM clk";

- Removes the now-unneeded goto targets from rockchip_pwm_probe();

- Tries to improve rockchip_pwm_probe() by having it enable the signal
  clock of only PWM devices that are already running; and

- Ensures the driver enables a clock before querying its rate with
  clk_get_rate(), as stated as a requirement in that function's
  documentation.

The first patch ("Enable APB clock...") is unchanged from version 2.

New in version 3 are

- Finer patch granularity, with patches 2 and 5 added to clarify
  changes included with others in v2;

- A rewritten patch 6 ("Enable PWM clock...") with a smaller change
  and the use of if...else in place of a ternary operator;

- Patches 3 and 7 with fixes suggested by Robin Murphy and Uwe
  Kleine-König; and

- Rewritten and (hopefully) more accurate commit messages.

I've tested these changes on my (RK3399-based) Pinebook Pro with its
screen backlight enabled by U-Boot and each one appears to work fine.

I'd (still) be grateful for help with testing on other devices,
particularly those with SoCs like the RK3328 that use separate bus and
signal clocks for their PWM devices. (My ROCK64 uses its PWM-output
pins for other purposes and wasn't of help here.)

[0] https://www.spinics.net/lists/linux-pwm/msg14611.html

--
Simon South
simon@simonsouth.net


Simon South (7):
  pwm: rockchip: Enable APB clock during register access while probing
  pwm: rockchip: rockchip_pwm_probe(): Remove superfluous
    clk_unprepare()
  pwm: rockchip: Replace "bus clk" with "PWM clk"
  pwm: rockchip: Eliminate potential race condition when probing
  pwm: rockchip: rockchip_pwm_probe(): Remove unneeded goto target
  pwm: rockchip: Enable PWM clock of probed device only if running
  pwm: rockchip: Enable clock before calling clk_get_rate()

 drivers/pwm/pwm-rockchip.c | 64 ++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 24 deletions(-)

-- 
2.29.2


WARNING: multiple messages have this Message-ID (diff)
From: Simon South <simon@simonsouth.net>
To: tpiepho@gmail.com, thierry.reding@gmail.com,
	u.kleine-koenig@pengutronix.de, robin.murphy@arm.com,
	lee.jones@linaro.org, heiko@sntech.de, bbrezillon@kernel.org,
	linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Cc: simon@simonsouth.net
Subject: [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing
Date: Wed, 23 Dec 2020 11:01:02 -0500	[thread overview]
Message-ID: <cover.1608735481.git.simon@simonsouth.net> (raw)

This patch series aims to eliminate the race condition Trent Piepho
identified[0] in the Rockchip PWM driver's rockchip_pwm_probe()
function, by moving code that checks whether a device is enabled ahead
of the code that registers it via pwmchip_add().

It has grown to include a number of other small fixes and improvements
to the driver. It now also

- Fixes a potential kernel hang introduced by my earlier commit
  457f74abbed0 ("pwm: rockchip: Keep enabled PWMs running while
  probing") by ensuring a device's APB clock is enabled before its
  registers are accessed;

- Removes a superfluous call to clk_unprepare() that could result in
  warnings from the kernel;

- Clarifies the driver's error messages by replacing "bus clk" with
  "PWM clk";

- Removes the now-unneeded goto targets from rockchip_pwm_probe();

- Tries to improve rockchip_pwm_probe() by having it enable the signal
  clock of only PWM devices that are already running; and

- Ensures the driver enables a clock before querying its rate with
  clk_get_rate(), as stated as a requirement in that function's
  documentation.

The first patch ("Enable APB clock...") is unchanged from version 2.

New in version 3 are

- Finer patch granularity, with patches 2 and 5 added to clarify
  changes included with others in v2;

- A rewritten patch 6 ("Enable PWM clock...") with a smaller change
  and the use of if...else in place of a ternary operator;

- Patches 3 and 7 with fixes suggested by Robin Murphy and Uwe
  Kleine-König; and

- Rewritten and (hopefully) more accurate commit messages.

I've tested these changes on my (RK3399-based) Pinebook Pro with its
screen backlight enabled by U-Boot and each one appears to work fine.

I'd (still) be grateful for help with testing on other devices,
particularly those with SoCs like the RK3328 that use separate bus and
signal clocks for their PWM devices. (My ROCK64 uses its PWM-output
pins for other purposes and wasn't of help here.)

[0] https://www.spinics.net/lists/linux-pwm/msg14611.html

--
Simon South
simon@simonsouth.net


Simon South (7):
  pwm: rockchip: Enable APB clock during register access while probing
  pwm: rockchip: rockchip_pwm_probe(): Remove superfluous
    clk_unprepare()
  pwm: rockchip: Replace "bus clk" with "PWM clk"
  pwm: rockchip: Eliminate potential race condition when probing
  pwm: rockchip: rockchip_pwm_probe(): Remove unneeded goto target
  pwm: rockchip: Enable PWM clock of probed device only if running
  pwm: rockchip: Enable clock before calling clk_get_rate()

 drivers/pwm/pwm-rockchip.c | 64 ++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 24 deletions(-)

-- 
2.29.2


_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Simon South <simon@simonsouth.net>
To: tpiepho@gmail.com, thierry.reding@gmail.com,
	u.kleine-koenig@pengutronix.de, robin.murphy@arm.com,
	lee.jones@linaro.org, heiko@sntech.de, bbrezillon@kernel.org,
	linux-pwm@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org
Cc: simon@simonsouth.net
Subject: [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing
Date: Wed, 23 Dec 2020 11:01:02 -0500	[thread overview]
Message-ID: <cover.1608735481.git.simon@simonsouth.net> (raw)

This patch series aims to eliminate the race condition Trent Piepho
identified[0] in the Rockchip PWM driver's rockchip_pwm_probe()
function, by moving code that checks whether a device is enabled ahead
of the code that registers it via pwmchip_add().

It has grown to include a number of other small fixes and improvements
to the driver. It now also

- Fixes a potential kernel hang introduced by my earlier commit
  457f74abbed0 ("pwm: rockchip: Keep enabled PWMs running while
  probing") by ensuring a device's APB clock is enabled before its
  registers are accessed;

- Removes a superfluous call to clk_unprepare() that could result in
  warnings from the kernel;

- Clarifies the driver's error messages by replacing "bus clk" with
  "PWM clk";

- Removes the now-unneeded goto targets from rockchip_pwm_probe();

- Tries to improve rockchip_pwm_probe() by having it enable the signal
  clock of only PWM devices that are already running; and

- Ensures the driver enables a clock before querying its rate with
  clk_get_rate(), as stated as a requirement in that function's
  documentation.

The first patch ("Enable APB clock...") is unchanged from version 2.

New in version 3 are

- Finer patch granularity, with patches 2 and 5 added to clarify
  changes included with others in v2;

- A rewritten patch 6 ("Enable PWM clock...") with a smaller change
  and the use of if...else in place of a ternary operator;

- Patches 3 and 7 with fixes suggested by Robin Murphy and Uwe
  Kleine-König; and

- Rewritten and (hopefully) more accurate commit messages.

I've tested these changes on my (RK3399-based) Pinebook Pro with its
screen backlight enabled by U-Boot and each one appears to work fine.

I'd (still) be grateful for help with testing on other devices,
particularly those with SoCs like the RK3328 that use separate bus and
signal clocks for their PWM devices. (My ROCK64 uses its PWM-output
pins for other purposes and wasn't of help here.)

[0] https://www.spinics.net/lists/linux-pwm/msg14611.html

--
Simon South
simon@simonsouth.net


Simon South (7):
  pwm: rockchip: Enable APB clock during register access while probing
  pwm: rockchip: rockchip_pwm_probe(): Remove superfluous
    clk_unprepare()
  pwm: rockchip: Replace "bus clk" with "PWM clk"
  pwm: rockchip: Eliminate potential race condition when probing
  pwm: rockchip: rockchip_pwm_probe(): Remove unneeded goto target
  pwm: rockchip: Enable PWM clock of probed device only if running
  pwm: rockchip: Enable clock before calling clk_get_rate()

 drivers/pwm/pwm-rockchip.c | 64 ++++++++++++++++++++++++--------------
 1 file changed, 40 insertions(+), 24 deletions(-)

-- 
2.29.2


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

             reply	other threads:[~2020-12-23 16:02 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-23 16:01 Simon South [this message]
2020-12-23 16:01 ` [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing Simon South
2020-12-23 16:01 ` Simon South
2020-12-23 16:01 ` [PATCH v3 1/7] pwm: rockchip: Enable APB clock during register access while probing Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2020-12-25  7:11   ` Kever Yang
2021-01-13  7:23   ` Uwe Kleine-König
2021-01-13  7:23     ` Uwe Kleine-König
2021-01-13  7:23     ` Uwe Kleine-König
2020-12-23 16:01 ` [PATCH v3 2/7] pwm: rockchip: rockchip_pwm_probe(): Remove superfluous clk_unprepare() Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2021-01-13  7:31   ` Uwe Kleine-König
2021-01-13  7:31     ` Uwe Kleine-König
2021-01-13  7:31     ` Uwe Kleine-König
2020-12-23 16:01 ` [PATCH v3 3/7] pwm: rockchip: Replace "bus clk" with "PWM clk" Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2020-12-25  7:13   ` Kever Yang
2021-01-13  7:33   ` Uwe Kleine-König
2021-01-13  7:33     ` Uwe Kleine-König
2021-01-13  7:33     ` Uwe Kleine-König
2020-12-23 16:01 ` [PATCH v3 4/7] pwm: rockchip: Eliminate potential race condition when probing Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2021-01-13  7:37   ` Uwe Kleine-König
2021-01-13  7:37     ` Uwe Kleine-König
2021-01-13  7:37     ` Uwe Kleine-König
2020-12-23 16:01 ` [PATCH v3 5/7] pwm: rockchip: rockchip_pwm_probe(): Remove unneeded goto target Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2020-12-25  7:14   ` Kever Yang
2021-01-13  7:38   ` Uwe Kleine-König
2021-01-13  7:38     ` Uwe Kleine-König
2021-01-13  7:38     ` Uwe Kleine-König
2020-12-23 16:01 ` [PATCH v3 6/7] pwm: rockchip: Enable PWM clock of probed device only if running Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2020-12-25  7:14   ` Kever Yang
2021-01-13  7:50   ` Uwe Kleine-König
2021-01-13  7:50     ` Uwe Kleine-König
2021-01-13  7:50     ` Uwe Kleine-König
2021-01-14 15:22     ` Simon South
2021-01-14 15:22       ` Simon South
2021-01-14 15:22       ` Simon South
2020-12-23 16:01 ` [PATCH v3 7/7] pwm: rockchip: Enable clock before calling clk_get_rate() Simon South
2020-12-23 16:01   ` Simon South
2020-12-23 16:01   ` Simon South
2021-01-13  7:54   ` Uwe Kleine-König
2021-01-13  7:54     ` Uwe Kleine-König
2021-01-13  7:54     ` Uwe Kleine-König
2020-12-25  7:10 ` [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing Kever Yang
2021-01-05 11:26   ` [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing【请注意,邮件由kever.yang@gmail.com代发】 David Wu
2021-01-05 11:26     ` David Wu
2021-01-05 11:26     ` David Wu
2021-01-13  9:19     ` Uwe Kleine-König
2021-01-13  9:19       ` Uwe Kleine-König
2021-01-13  9:19       ` Uwe Kleine-König

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=cover.1608735481.git.simon@simonsouth.net \
    --to=simon@simonsouth.net \
    --cc=bbrezillon@kernel.org \
    --cc=heiko@sntech.de \
    --cc=lee.jones@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=thierry.reding@gmail.com \
    --cc=tpiepho@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.