Thread (2 messages) 2 messages, 2 authors, 2021-01-13

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

From: David Wu <hidden>
Date: 2021-01-05 11:28:33
Also in: linux-pwm, linux-rockchip

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 [off-list ref]

在 2020/12/25 下午3:10, Kever Yang 写道:
+ David and Steven,

Hi Steven,
     please help to review this patch set.

Thanks
- Kever
Simon South <simon@simonsouth.net <mailto:simon@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@simonsouth.net <mailto: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
    <mailto:Linux-rockchip@lists.infradead.org>
    http://lists.infradead.org/mailman/listinfo/linux-rockchip


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help