Message ID | 20250423090446.294846-5-nylon.chen@sifive.com |
---|---|
State | Superseded |
Headers | show |
Series | Change PWM-controlled LED pin active mode and algorithm | expand |
Hi Nylon, kernel test robot noticed the following build errors: [auto build test ERROR on robh/for-next] [also build test ERROR on linus/master v6.15-rc5 next-20250507] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Nylon-Chen/riscv-dts-sifive-unleashed-unmatched-Remove-PWM-controlled-LED-s-active-low-properties/20250423-165906 base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next patch link: https://lore.kernel.org/r/20250423090446.294846-5-nylon.chen%40sifive.com patch subject: [PATCH v13 4/5] pwm: sifive: Fix rounding issues in apply and get_state functions config: hexagon-allmodconfig (https://download.01.org/0day-ci/archive/20250508/202505080303.dBfU5YMS-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250508/202505080303.dBfU5YMS-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202505080303.dBfU5YMS-lkp@intel.com/ All error/warnings (new ones prefixed by >>): >> drivers/pwm/pwm-sifive.c:161:2: warning: comparison of distinct pointer types ('typeof ((frac)) *' (aka 'unsigned int *') and 'uint64_t *' (aka 'unsigned long long *')) [-Wcompare-distinct-pointer-types] 161 | do_div(frac, state->period); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ include/asm-generic/div64.h:183:28: note: expanded from macro 'do_div' 183 | (void)(((typeof((n)) *)0) == ((uint64_t *)0)); \ | ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~ >> drivers/pwm/pwm-sifive.c:161:2: error: incompatible pointer types passing 'u32 *' (aka 'unsigned int *') to parameter of type 'uint64_t *' (aka 'unsigned long long *') [-Werror,-Wincompatible-pointer-types] 161 | do_div(frac, state->period); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ include/asm-generic/div64.h:199:22: note: expanded from macro 'do_div' 199 | __rem = __div64_32(&(n), __base); \ | ^~~~ include/asm-generic/div64.h:174:38: note: passing argument to parameter 'dividend' here 174 | extern uint32_t __div64_32(uint64_t *dividend, uint32_t divisor); | ^ >> drivers/pwm/pwm-sifive.c:161:2: warning: shift count >= width of type [-Wshift-count-overflow] 161 | do_div(frac, state->period); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~ include/asm-generic/div64.h:195:25: note: expanded from macro 'do_div' 195 | } else if (likely(((n) >> 32) == 0)) { \ | ^ ~~ include/linux/compiler.h:76:40: note: expanded from macro 'likely' 76 | # define likely(x) __builtin_expect(!!(x), 1) | ^ 2 warnings and 1 error generated. vim +161 drivers/pwm/pwm-sifive.c 131 132 static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, 133 const struct pwm_state *state) 134 { 135 struct pwm_sifive_ddata *ddata = pwm_sifive_chip_to_ddata(chip); 136 struct pwm_state cur_state; 137 unsigned int duty_cycle; 138 unsigned long long num; 139 bool enabled; 140 int ret = 0; 141 u32 frac, inactive; 142 143 if (state->polarity != PWM_POLARITY_NORMAL) 144 return -EINVAL; 145 146 cur_state = pwm->state; 147 enabled = cur_state.enabled; 148 149 duty_cycle = state->duty_cycle; 150 if (!state->enabled) 151 duty_cycle = 0; 152 153 /* 154 * The problem of output producing mixed setting as mentioned at top, 155 * occurs here. To minimize the window for this problem, we are 156 * calculating the register values first and then writing them 157 * consecutively 158 */ 159 num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); 160 frac = num; > 161 do_div(frac, state->period); 162 /* The hardware cannot generate a 0% duty cycle */ 163 frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); 164 inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac; 165 166 mutex_lock(&ddata->lock); 167 if (state->period != ddata->approx_period) { 168 /* 169 * Don't let a 2nd user change the period underneath the 1st user. 170 * However if ddate->approx_period == 0 this is the first time we set 171 * any period, so let whoever gets here first set the period so other 172 * users who agree on the period won't fail. 173 */ 174 if (ddata->user_count != 1 && ddata->approx_period) { 175 mutex_unlock(&ddata->lock); 176 return -EBUSY; 177 } 178 ddata->approx_period = state->period; 179 pwm_sifive_update_clock(ddata, clk_get_rate(ddata->clk)); 180 } 181 mutex_unlock(&ddata->lock); 182 183 /* 184 * If the PWM is enabled the clk is already on. So only enable it 185 * conditionally to have it on exactly once afterwards independent of 186 * the PWM state. 187 */ 188 if (!enabled) { 189 ret = clk_enable(ddata->clk); 190 if (ret) { 191 dev_err(pwmchip_parent(chip), "Enable clk failed\n"); 192 return ret; 193 } 194 } 195 196 writel(inactive, ddata->regs + PWM_SIFIVE_PWMCMP(pwm->hwpwm)); 197 198 if (!state->enabled) 199 clk_disable(ddata->clk); 200 201 return 0; 202 } 203
diff --git a/drivers/pwm/pwm-sifive.c b/drivers/pwm/pwm-sifive.c index 6259f8500f71..4cf3e715fd84 100644 --- a/drivers/pwm/pwm-sifive.c +++ b/drivers/pwm/pwm-sifive.c @@ -122,8 +122,8 @@ static int pwm_sifive_get_state(struct pwm_chip *chip, struct pwm_device *pwm, state->enabled = false; state->period = ddata->real_period; - state->duty_cycle = - (u64)duty * ddata->real_period >> PWM_SIFIVE_CMPWIDTH; + state->duty_cycle = DIV_ROUND_UP_ULL((u64)duty * ddata->real_period, + (1U << PWM_SIFIVE_CMPWIDTH)); state->polarity = PWM_POLARITY_NORMAL; return 0; @@ -157,7 +157,8 @@ static int pwm_sifive_apply(struct pwm_chip *chip, struct pwm_device *pwm, * consecutively */ num = (u64)duty_cycle * (1U << PWM_SIFIVE_CMPWIDTH); - frac = DIV64_U64_ROUND_CLOSEST(num, state->period); + frac = num; + do_div(frac, state->period); /* The hardware cannot generate a 0% duty cycle */ frac = min(frac, (1U << PWM_SIFIVE_CMPWIDTH) - 1); inactive = (1U << PWM_SIFIVE_CMPWIDTH) - 1 - frac;