Re: [PATCH v3 0/7] pwm: rockchip: Eliminate potential race condition when probing【请注意，邮件由kever.yang at gmail.com代发】

[David Wu]

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
>