From 927ec427493fb894ed3ca7a17174cb7ddcbc7dba Mon Sep 17 00:00:00 2001 From: Damon Ding Date: Tue, 1 Jul 2025 17:29:26 +0800 Subject: [PATCH] pwm: rockchip: Add &rockchip_pwm_chip.oneshot_valid to indicate validity of configurations In the past, the flag &rockchip_pwm_chip.oneshot_en may not represent the accurate enabled status for oneshot mode, because the oneshot mode should be active after setting the 'pwm_en' bit. Therefore, we add the &rockchip_pwm_chip.oneshot_valid to represent the validity of oneshot configurations, and &rockchip_pwm_chip.oneshot_en does what it should do. In addition, the disabling of oneshot mode does not need to delay one period(related commit 42e759004f12 ("pwm: rockchip: add one period delay before disabling the dclk")). It will end after the last period sent. What's more serious, the disabling process may be done in interrupt handler for oneshot mode(The handler is flexible for user as designed), so it is unreasonable to call fsleep() in the interrupt handler, which may cause the following error with 100000ns period: [ 6.517981] BUG: scheduling while atomic: swapper/0/0/0x00010000 [ 6.518045] Modules linked in: [ 6.518060] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 6.1.118 #944 [ 6.518069] Hardware name: Rockchip RK3576 EVB1 V10 Board (DT) [ 6.518078] Call trace: [ 6.518085] dump_backtrace+0xd8/0x130 [ 6.518108] show_stack+0x1c/0x30 [ 6.518118] dump_stack_lvl+0x64/0x7c [ 6.518132] dump_stack+0x14/0x2c [ 6.518141] __schedule_bug+0x58/0x70 [ 6.518155] __schedule+0x6f0/0x7c0 [ 6.518164] schedule+0x54/0xe0 [ 6.518172] schedule_hrtimeout_range_clock+0xa8/0x144 [ 6.518184] schedule_hrtimeout_range+0x18/0x20 [ 6.518193] usleep_range_state+0x7c/0xb0 [ 6.518204] rockchip_pwm_enable_v4+0xc8/0x104 [ 6.518219] rockchip_pwm_apply+0x80/0x190 [ 6.518229] pwm_apply_state+0x68/0x190 [ 6.518239] rockchip_pwm_irq_v4+0x7c/0x1b0 [ 6.518250] __handle_irq_event_percpu+0x58/0x1d0 [ 6.518265] handle_irq_event+0x4c/0x110 [ 6.518276] handle_fasteoi_irq+0xc0/0x24c [ 6.518290] generic_handle_domain_irq+0x30/0x44 [ 6.518302] gic_handle_irq+0x60/0x90 [ 6.518312] call_on_irq_stack+0x24/0x34 [ 6.518323] do_interrupt_handler+0x80/0x94 [ 6.518333] el1_interrupt+0x44/0xa0 [ 6.518345] el1h_64_irq_handler+0x14/0x20 [ 6.518357] el1h_64_irq+0x74/0x78 [ 6.518366] cpuidle_enter_state+0xbc/0x434 [ 6.518382] cpuidle_enter+0x3c/0x50 [ 6.518393] do_idle+0x228/0x2b0 [ 6.518405] cpu_startup_entry+0x38/0x40 [ 6.518416] kernel_init+0x0/0x12c [ 6.518425] arch_post_acpi_subsys_init+0x0/0x18 [ 6.518439] start_kernel+0x6b0/0x6ec [ 6.518450] __primary_switched+0xb4/0xbc This patch will also help to avoid the above abnormal situation. Change-Id: I0df715921d79803f06329a71b966a4ae40876f33 Signed-off-by: Damon Ding --- drivers/pwm/pwm-rockchip.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/pwm/pwm-rockchip.c b/drivers/pwm/pwm-rockchip.c index 5c4c5669eec5..19a158a077c3 100644 --- a/drivers/pwm/pwm-rockchip.c +++ b/drivers/pwm/pwm-rockchip.c @@ -321,6 +321,7 @@ struct rockchip_pwm_chip { unsigned long is_clk_enabled; bool vop_pwm_en; /* indicate voppwm mirror register state */ bool center_aligned; + bool oneshot_valid; bool oneshot_en; bool capture_en; bool wave_en; @@ -561,7 +562,7 @@ static void rockchip_pwm_config_v1(struct pwm_chip *chip, struct pwm_device *pwm ctrl &= ~PWM_CLK_SEL_MASK; ctrl |= PWM_SEL_SCALED_CLOCK; - pc->oneshot_en = true; + pc->oneshot_valid = true; ctrl &= ~PWM_MODE_MASK; ctrl |= PWM_ONESHOT; @@ -584,7 +585,7 @@ static void rockchip_pwm_config_v1(struct pwm_chip *chip, struct pwm_device *pwm dev_err(chip->dev, "Oneshot_count must be between 1 and %d.\n", pc->data->oneshot_cnt_max); - pc->oneshot_en = false; + pc->oneshot_valid = false; ctrl &= ~PWM_MODE_MASK; ctrl |= PWM_CONTINUOUS; @@ -651,7 +652,7 @@ static int rockchip_pwm_enable_v1(struct pwm_chip *chip, struct pwm_device *pwm, val |= PWM_OUTPUT_CENTER; } - if (pc->oneshot_en) { + if (pc->oneshot_valid) { enable_conf &= ~PWM_MODE_MASK; enable_conf |= PWM_ONESHOT; } else if (pc->capture_en) { @@ -679,6 +680,8 @@ static int rockchip_pwm_enable_v1(struct pwm_chip *chip, struct pwm_device *pwm, if (!enable) clk_disable(pc->clk); + pc->oneshot_en = pc->oneshot_valid ? enable : false; + return 0; } @@ -830,17 +833,17 @@ static void rockchip_pwm_config_v4(struct pwm_chip *chip, struct pwm_device *pwm state->duty_cycle, state->period); } - pc->oneshot_en = true; + pc->oneshot_valid = true; } else { if (state->oneshot_count) dev_err(chip->dev, "Oneshot_count must be between 1 and %d.\n", pc->data->oneshot_cnt_max); - pc->oneshot_en = false; + pc->oneshot_valid = false; } #endif - if (pc->oneshot_en) { + if (pc->oneshot_valid) { writel_relaxed(PWM_MODE(ONESHOT_MODE) | PWM_ALIGNED_INVALID(true), pc->base + CTRL_V4); writel_relaxed(offset, pc->base + OFFSET); @@ -874,17 +877,19 @@ static int rockchip_pwm_enable_v4(struct pwm_chip *chip, struct pwm_device *pwm, writel_relaxed(PWM_EN(enable) | PWM_CLK_EN(enable), pc->base + ENABLE); /* - * For pwm v4, the disable operation, which sets polarity to inactive state, - * will not take effect until the end of current period. Therefore, it makes - * sense to delay one period before disabling the dclk. + * For pwm v4, the disable operation of continuous mode, which sets polarity + * to inactive state, will not take effect until the end of current period. + * Therefore, it makes sense to delay one period before disabling the dclk. */ - if (!enable) { + if (!enable && !pc->oneshot_en) { pwm_get_state(pwm, &curstate); delay_us = DIV_ROUND_UP_ULL(curstate.period, NSEC_PER_USEC); fsleep(delay_us); clk_disable(pc->clk); } + pc->oneshot_en = pc->oneshot_valid ? enable : false; + return 0; }