Skip to content

9075 clk pwm#538

Open
nascs wants to merge 11 commits intoradxa:Ubuntu-qcom-6.8.0-1066.67from
nascs:9075_clk-pwm
Open

9075 clk pwm#538
nascs wants to merge 11 commits intoradxa:Ubuntu-qcom-6.8.0-1066.67from
nascs:9075_clk-pwm

Conversation

@nascs
Copy link
Copy Markdown

@nascs nascs commented Apr 20, 2026

No description provided.

strongtz and others added 11 commits April 13, 2026 02:12
@strongtz @nascs
clk: qcom: clk-rcg2: fix clk_rcg2_calc_mnd() producing wrong M/N/pre_div
36da893 
Fix three related bugs in clk_rcg2_calc_mnd() that cause the GP clock
MND divider to produce incorrect frequencies and duty cycles:

1) n_max and n_candidate are declared as u16 but can hold values
   exceeding 65535 during computation. When mnd_width is 16 and
   mnd_max is 65535, even m=1 gives n_max=65536 which overflows u16
   to 0, making "n_candidate < n_max" always false. Similarly
   n_candidate overflows on intermediate products (e.g., 15360 * 5 =
   76800 wraps to 11264), causing the wrong value to be accepted as n.

   Fix by changing both n_max and n_candidate from u16 to u32.

2) n_max is computed as (m + mnd_max), which only accounts for the N
   register constraint (n - m must fit in mnd_width bits). However,
   the D register shares the same mnd_width bits but must store values
   up to 2*n for duty cycle control. When n > mnd_max/2, the D
   register cannot represent high duty cycles, silently clamping the
   maximum achievable duty cycle (e.g. to 85% instead of 99%).

   Fix by computing n_max as (mnd_max + 1) / 2, ensuring 2*n always
   fits within the D register's bit width.

3) When no pre-division is needed (pre_div stays at its initial value
   of 1), "pre_div > 1 ? pre_div : 0" sets f->pre_div to 0. The
   subsequent convert_to_reg_val() computes (0 * 2 - 1) which
   underflows the u8 pre_div field to 255, programming a 128x
   pre-divider into hardware.

   Fix by assigning pre_div unconditionally. Since pre_div is
   initialized to 1 and only multiplied, it is always >= 1. A value
   of 1 correctly converts to register value 1 via
   convert_to_reg_val(), which means no pre-division in calc_rate().

Example with parent=19.2 MHz XO, requesting 25 kHz:

  Before: m=1, n=48,  pre_div=15 -> 26666 Hz  (6.7% error)
  After:  m=1, n=768, pre_div=1  -> 25000 Hz  (exact)

Fixes: 898b72f ("clk: qcom: gcc-sdm845: Add general purpose clock ops")
Signed-off-by: Xilin Wu <sophon@radxa.com>
@strongtz @nascs
clk: qcom: clk-rcg2: use 64-bit arithmetic in set_duty_cycle()
777afcc 
The duty cycle calculation in clk_rcg2_set_duty_cycle() computes
"n * duty->num * 2" using u32 arithmetic. When n is large and
duty->num is also large, the intermediate result overflows u32.

For example, requesting 50% duty on a 1 kHz output derived from a
19.2 MHz parent gives n=19200, duty->num=500000, duty->den=1000000:

  19200 * 500000 * 2 = 19,200,000,000 > U32_MAX (4,294,967,295)

The truncated result produces a completely wrong duty cycle (5.26%
instead of the requested 50%).

Use DIV_ROUND_CLOSEST_ULL() with an explicit (u64) cast to prevent
the overflow.

Fixes: 7f891fa ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
Signed-off-by: Xilin Wu <sophon@radxa.com>
@strongtz @nascs
clk: qcom: clk-branch: calculate timeout based on clock frequency
fc0a9b6 
Clock branches with extremely low rates (tens of Hz to low kHz) take
much longer to toggle than the fixed 200 us timeout allows. A 1 kHz
clock needs at least 3 ms (3 cycles) to toggle.

Instead of increasing the timeout to a huge fixed value for all clocks,
dynamically compute the required timeout based on the current clock
rate, accounting for 3 cycles at the current clock rate.

Based on a downstream patch by Mike Tipton:
https://git.codelinaro.org/clo/la/kernel/qcom/-/commit/aa899c2d1fa31e247f04810f125ac9c60927c901

Fixes: 6e0ad1b ("clk: qcom: Add support for branches/gate clocks")
Signed-off-by: Mike Tipton <quic_mdtipton@quicinc.com>
Signed-off-by: Xilin Wu <sophon@radxa.com>
@strongtz @nascs
clk: qcom: clk-rcg2: calculate timeout based on clock frequency
85cf388 
RCGs with extremely low rates (tens of Hz to low kHz) take much longer
to update than the fixed 500 us timeout allows. A 1 kHz clock needs at
least 3 ms (3 cycles) for the configuration handshake.

Instead of increasing the timeout to a huge fixed value for all clocks,
dynamically compute the required timeout based on both the old and new
clock rates, accounting for 3 cycles at each rate.

Based on a downstream patch by Mike Tipton:
https://git.codelinaro.org/clo/la/kernel/qcom/-/commit/aa899c2d1fa31e247f04810f125ac9c60927c901

Fixes: bcd61c0 ("clk: qcom: Add support for root clock generators (RCGs)")
Signed-off-by: Mike Tipton <quic_mdtipton@quicinc.com>
Signed-off-by: Xilin Wu <sophon@radxa.com>
@strongtz @nascs
clk: qcom: clk-rcg2: fix set_duty_cycle() integer overflow in boundar…
9dab4f0 
…y checks

The duty cycle boundary checks in clk_rcg2_set_duty_cycle() use
integer division to compare the 2d value against hardware limits:

    if ((d / 2) > (n - m))
        d = (n - m) * 2;
    else if ((d / 2) < (m / 2))
        d = m;

When d is odd, d/2 truncates, allowing values one beyond the hardware
maximum to pass. For example with n=7680, m=1, requesting 99.995%
duty:

    d = 15359  (raw 2d value)
    d / 2 = 7679  (truncated)
    n - m = 7679
    7679 > 7679 → false, check passes

But d=15359 exceeds the hardware limit of 2*(n-m)=15358. Writing this
invalid value causes the RCG to fail its configuration update, the
CMD_UPDATE bit never clears, and the clock output stops entirely.

The initial D value in __clk_rcg2_configure_mnd() correctly uses
direct comparison without division:

    d_val = clamp_t(u32, d_val, f->m, 2 * (f->n - f->m));

Align set_duty_cycle() with the same bounds by comparing directly:

    if (d > (n - m) * 2)
    else if (d < m)

Fixes: 7f891fa ("clk: qcom: clk-rcg2: Add support for duty-cycle for RCG")
Signed-off-by: Xilin Wu <sophon@radxa.com>
@strongtz @nascs
clk: qcom: gcc-sc7280: switch GP clocks to clk_rcg2_gp_ops
4263e1e 
The GP1/GP2/GP3 clock sources are general-purpose timer/PWM clocks
that require runtime-computed MND divider values and duty cycle
control. They are currently using clk_rcg2_ops with a frequency table
containing only a few fixed entries (50/100/200 MHz), which:

- Cannot produce arbitrary frequencies needed for PWM periods
- Bypasses the MND divider (m=0, n=0), making duty cycle control
  impossible (MND is in bypass mode, set_duty_cycle returns -EINVAL)

Switch to clk_rcg2_gp_ops which uses clk_rcg2_calc_mnd() to
dynamically compute optimal M/N/pre_div values from any requested
frequency, and empty the frequency table since it is not used by the
GP ops path.

Signed-off-by: Xilin Wu <sophon@radxa.com>
@strongtz @nascs
dt-bindings: pwm: clk-pwm: add optional GPIO and pinctrl properties
5b920ae 
The clk-pwm driver cannot produce constant output levels (0% or 100%
duty cycle, or disabled state) through the clock hardware alone - the
actual pin level when the clock is off is undefined and
hardware-dependent.

Document optional gpios, pinctrl-names, pinctrl-0, and pinctrl-1
properties that allow the driver to switch the pin between clock
function mux (for normal PWM output) and GPIO mode (to drive a
deterministic constant level).

Signed-off-by: Xilin Wu <sophon@radxa.com>
@nascs
clk: qcom: clk-rcg2: document calc_rate function
bcc8f3b 
Update calc_rate docs to reflect, that pre_div
is not pure divisor, but a register value, and requires conversion.

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
Link: https://lore.kernel.org/r/20241118-starqltechn_integration_upstream-v8-1-ac8e36a3aa65@gmail.com
Signed-off-by: Bjorn Andersson <andersson@kernel.org>
@nascs
clk: qcom: rcg2: add dedicated GP clock ops
1a3f854 
Teach the GP RCG path to evaluate all candidate parents, program the
selected parent and M/N state in one configuration update, and expose a
GP-specific clk_ops instance with set_rate_and_parent support.

Signed-off-by: William Norman <nascs@radxa.com>
@nascs
clk: qcom: gcc-sa8775p: wire GP clocks to clk_rcg2_gp_ops
668fa27 
Switch the SA8775P GP clock sources over to the dedicated GP RCG ops and
full parent map so they can take advantage of parent-aware rate
selection instead of relying on the previous fixed-rate table.

Signed-off-by: William Norman <nascs@radxa.com>
@nascs
pwm: clk: reprogram clocks safely for 50% duty
1f371aa 
Disable the backing clock before changing its rate and duty cycle, then
re-enable it after the new configuration is in place. When the request is
already for an exact 50%% duty cycle, skip the redundant clk_set_duty_cycle()
call and leave the clock's default 50%% output untouched.

Signed-off-by: William Norman <nascs@radxa.com>
@nascs nascs requested a review from RadxaYuntian April 20, 2026 07:34
RadxaYuntian
RadxaYuntian reviewed Apr 20, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@RadxaYuntian RadxaYuntian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RadxaYuntian RadxaYuntian RadxaYuntian left review comments

Assignees

@RadxaStephen RadxaStephen

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nascs @RadxaYuntian @RadxaStephen @strongtz