Re: [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing【请注意,邮件由kever.yang at gmail.com代发】
David Wu
david.wu at rock-chips.com
Tue Jan 5 06:26:16 EST 2021
Hi Simon,
This series of patches are okay for me, simplify to read pwm
register to check pwm whether it is enabled. So,
Revieded-by: David Wu <david.wu at rock-chips.com>
在 2020/12/25 下午3:10, Kever Yang 写道:
> + David and Steven,
>
> Hi Steven,
> please help to review this patch set.
>
> Thanks
> - Kever
> Simon South <simon at simonsouth.net <mailto:simon at simonsouth.net>> 于2020
> 年12月24日周四 上午12:01写道:
>
> 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 at simonsouth.net <mailto:simon at 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 at lists.infradead.org
> <mailto:Linux-rockchip at lists.infradead.org>
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
More information about the Linux-rockchip
mailing list