7:22 a.m.
Hi Baruch,
I love your patch! Perhaps something to improve:
[auto build test WARNING on thierry-reding-pwm/for-next]
[also build test WARNING on robh/for-next v5.17-rc3 next-20220207]
[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]
url:
https://github.com/0day-ci/linux/commits/Baruch-Siach/pwm-driver-for-qual...
base: https://git.kernel.org/pub/scm/linux/kernel/git/thierry.reding/linux-pwm.git
for-next
config: hexagon-allyesconfig
(https://download.01.org/0day-ci/archive/20220208/202202080410.R0qwqtXx-lk...)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project
0d8850ae2cae85d49bea6ae0799fa41c7202c05c)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/71e449eb6d19b141b4527caae529e16c5...
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Baruch-Siach/pwm-driver-for-qualcomm-ipq6018-pwm-block/20220207-175605
git checkout 71e449eb6d19b141b4527caae529e16c52bcfeea
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir
ARCH=hexagon SHELL=/bin/bash drivers/pwm/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp(a)intel.com>
All warnings (new ones prefixed by >>):
> drivers/pwm/pwm-ipq.c:122:11: warning: result of comparison of
constant 16000000000 with expression of type 'unsigned long' is always false
[-Wtautological-constant-out-of-range-compare]
if (rate > 16ULL *
GIGA)
~~~~ ^ ~~~~~~~~~~~~
1 warning generated.
vim +122 drivers/pwm/pwm-ipq.c
99
100 static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
101 const struct pwm_state *state)
102 {
103 struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
104 unsigned int pre_div, pwm_div, best_pre_div, best_pwm_div;
105 unsigned long rate = clk_get_rate(ipq_chip->clk);
106 u64 period_ns, duty_ns, period_rate;
107 u64 min_diff;
108
109 if (state->polarity != PWM_POLARITY_NORMAL)
110 return -EINVAL;
111
112 if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC, rate))
113 return -ERANGE;
114
115 period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
116 duty_ns = min(state->duty_cycle, period_ns);
117
118 /*
119 * period_ns is 1G or less. As long as rate is less than 16 GHz,
120 * period_rate does not overflow. Make that explicit.
121 */
122 if (rate > 16ULL * GIGA)
123 return -EINVAL;
124 period_rate = period_ns * rate;
125 best_pre_div = IPQ_PWM_MAX_DIV;
126 best_pwm_div = IPQ_PWM_MAX_DIV;
127 /*
128 * We don't need to consider pre_div values smaller than
129 *
130 * period_rate
131 * pre_div_min := ------------------------------------
132 * NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1)
133 *
134 * because pre_div = pre_div_min results in a better
135 * approximation.
136 */
137 pre_div = div64_u64(period_rate,
138 (u64)NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1));
139 min_diff = period_rate;
140
141 for (; pre_div <= IPQ_PWM_MAX_DIV; pre_div++) {
142 u64 remainder;
143
144 pwm_div = div64_u64_rem(period_rate,
145 (u64)NSEC_PER_SEC * (pre_div + 1), &remainder);
146 /* pwm_div is unsigned; the check below catches underflow */
147 pwm_div--;
148
149 /*
150 * Swapping values for pre_div and pwm_div produces the same
151 * period length. So we can skip all settings with pre_div >
152 * pwm_div which results in bigger constraints for selecting
153 * the duty_cycle than with the two values swapped.
154 */
155 if (pre_div > pwm_div)
156 break;
157
158 /*
159 * Make sure we can do 100% duty cycle where
160 * hi_dur == pwm_div + 1
161 */
162 if (pwm_div > IPQ_PWM_MAX_DIV - 1)
163 continue;
164
165 if (remainder < min_diff) {
166 best_pre_div = pre_div;
167 best_pwm_div = pwm_div;
168 min_diff = remainder;
169
170 if (min_diff == 0) /* bingo */
171 break;
172 }
173 }
174
175 /* config divider values for the closest possible frequency */
176 config_div_and_duty(pwm, best_pre_div, best_pwm_div,
177 rate, duty_ns, state->enabled);
178
179 return 0;
180 }
181
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
5:51 p.m.
New subject: [PATCH 1/3] pwm: driver for qualcomm ipq6018 pwm block
Hi test robot,
Thanks for testing and reporting.
On Tue, Feb 08 2022, kernel test robot wrote:
[snip]
>> drivers/pwm/pwm-ipq.c:122:11: warning: result of comparison
of constant 16000000000 with expression of type 'unsigned long' is always false
[-Wtautological-constant-out-of-range-compare]
if (rate > 16ULL * GIGA)
~~~~ ^ ~~~~~~~~~~~~
1 warning generated.
This clang warning is only enabled with W=1 (see commit
afe956c577b). Not sure how to avoid it.
Is there a way to express this condition without making clang warn on
platforms where ULONG_MAX == 2^32? Maybe cast to unsigned long long? Or
should we just ignore this W=1 warning?
baruch
vim +122 drivers/pwm/pwm-ipq.c
99
100 static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
101 const struct pwm_state *state)
102 {
103 struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
104 unsigned int pre_div, pwm_div, best_pre_div, best_pwm_div;
105 unsigned long rate = clk_get_rate(ipq_chip->clk);
106 u64 period_ns, duty_ns, period_rate;
107 u64 min_diff;
108
109 if (state->polarity != PWM_POLARITY_NORMAL)
110 return -EINVAL;
111
112 if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC, rate))
113 return -ERANGE;
114
115 period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
116 duty_ns = min(state->duty_cycle, period_ns);
117
118 /*
119 * period_ns is 1G or less. As long as rate is less than 16 GHz,
120 * period_rate does not overflow. Make that explicit.
121 */
> 122 if (rate > 16ULL * GIGA)
123 return -EINVAL;
124 period_rate = period_ns * rate;
125 best_pre_div = IPQ_PWM_MAX_DIV;
126 best_pwm_div = IPQ_PWM_MAX_DIV;
127 /*
128 * We don't need to consider pre_div values smaller than
129 *
130 * period_rate
131 * pre_div_min := ------------------------------------
132 * NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1)
133 *
134 * because pre_div = pre_div_min results in a better
135 * approximation.
136 */
137 pre_div = div64_u64(period_rate,
138 (u64)NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1));
139 min_diff = period_rate;
140
141 for (; pre_div <= IPQ_PWM_MAX_DIV; pre_div++) {
142 u64 remainder;
143
144 pwm_div = div64_u64_rem(period_rate,
145 (u64)NSEC_PER_SEC * (pre_div + 1), &remainder);
146 /* pwm_div is unsigned; the check below catches underflow */
147 pwm_div--;
148
149 /*
150 * Swapping values for pre_div and pwm_div produces the same
151 * period length. So we can skip all settings with pre_div >
152 * pwm_div which results in bigger constraints for selecting
153 * the duty_cycle than with the two values swapped.
154 */
155 if (pre_div > pwm_div)
156 break;
157
158 /*
159 * Make sure we can do 100% duty cycle where
160 * hi_dur == pwm_div + 1
161 */
162 if (pwm_div > IPQ_PWM_MAX_DIV - 1)
163 continue;
164
165 if (remainder < min_diff) {
166 best_pre_div = pre_div;
167 best_pwm_div = pwm_div;
168 min_diff = remainder;
169
170 if (min_diff == 0) /* bingo */
171 break;
172 }
173 }
174
175 /* config divider values for the closest possible frequency */
176 config_div_and_duty(pwm, best_pre_div, best_pwm_div,
177 rate, duty_ns, state->enabled);
178
179 return 0;
180 }
181
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch(a)tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
5:47 a.m.
New subject: [PATCH 1/3] pwm: driver for qualcomm ipq6018 pwm block
Hi Baruch,
On Tue, Feb 08, 2022 at 08:51:40AM +0200, Baruch Siach wrote:
Hi test robot,
Thanks for testing and reporting.
On Tue, Feb 08 2022, kernel test robot wrote:
[snip]
>>> drivers/pwm/pwm-ipq.c:122:11: warning: result of comparison of constant
16000000000 with expression of type 'unsigned long' is always false
[-Wtautological-constant-out-of-range-compare]
> if (rate > 16ULL * GIGA)
> ~~~~ ^ ~~~~~~~~~~~~
> 1 warning generated.
This clang warning is only enabled with W=1 (see commit
afe956c577b). Not sure how to avoid it.
Is there a way to express this condition without making clang warn on
platforms where ULONG_MAX == 2^32? Maybe cast to unsigned long long? Or
should we just ignore this W=1 warning?
As far as I am aware, casting to unsigned long long would be an
appropriate way to fix this warning, as has been done in the following
patches in mainline:
c9ae8eed4463 ("media: omap3isp: avoid warnings at IS_OUT_OF_BOUNDS()")
4853396f03c3 ("memstick: avoid out-of-range warning")
7ff4034e910f ("staging: vc04_services: shut up out-of-range warning")
a2fa9e57a68c ("ARM: mvebu: avoid clang -Wtautological-constant warning")
The below diff fixes the warning for me with ARCH=hexagon allyesconfig:
diff --git a/drivers/pwm/pwm-ipq.c b/drivers/pwm/pwm-ipq.c
index 994027290bcb..7ea29468e76e 100644
--- a/drivers/pwm/pwm-ipq.c
+++ b/drivers/pwm/pwm-ipq.c
@@ -119,7 +119,7 @@ static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device
*pwm,
* period_ns is 1G or less. As long as rate is less than 16 GHz,
* period_rate does not overflow. Make that explicit.
*/
- if (rate > 16ULL * GIGA)
+ if ((unsigned long long)rate > 16ULL * GIGA)
return -EINVAL;
period_rate = period_ns * rate;
best_pre_div = IPQ_PWM_MAX_DIV;
Alternatively, you could widen rate to unsigned long long / u64 but I
don't know what kind of implications that has in this function but it
has been done in other places:
95c58291ee70 ("drm/msm/submit: fix overflow check on 64-bit architectures")
cfd6fb45cfaf ("crypto: ccree - avoid out-of-range warnings from clang")
335aea75b0d9 ("drm/amdgpu: fix warning for overflow check")
844b85dda2f5 ("ARM: keystone: fix integer overflow warning")
While the warning is currently under W=1, I think it is one that we
would like to turn on at some point so fixing instances as they come up
helps us get closer to that goal.
Cheers,
Nathan
> > vim +122 drivers/pwm/pwm-ipq.c
> >
> > 99
> > 100 static int ipq_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> > 101 const struct pwm_state *state)
> > 102 {
> > 103 struct ipq_pwm_chip *ipq_chip = ipq_pwm_from_chip(chip);
> > 104 unsigned int pre_div, pwm_div, best_pre_div, best_pwm_div;
> > 105 unsigned long rate = clk_get_rate(ipq_chip->clk);
> > 106 u64 period_ns, duty_ns, period_rate;
> > 107 u64 min_diff;
> > 108
> > 109 if (state->polarity != PWM_POLARITY_NORMAL)
> > 110 return -EINVAL;
> > 111
> > 112 if (state->period < DIV64_U64_ROUND_UP(NSEC_PER_SEC, rate))
> > 113 return -ERANGE;
> > 114
> > 115 period_ns = min(state->period, IPQ_PWM_MAX_PERIOD_NS);
> > 116 duty_ns = min(state->duty_cycle, period_ns);
> > 117
> > 118 /*
> > 119 * period_ns is 1G or less. As long as rate is less than 16 GHz,
> > 120 * period_rate does not overflow. Make that explicit.
> > 121 */
> > > 122 if (rate > 16ULL * GIGA)
> > 123 return -EINVAL;
> > 124 period_rate = period_ns * rate;
> > 125 best_pre_div = IPQ_PWM_MAX_DIV;
> > 126 best_pwm_div = IPQ_PWM_MAX_DIV;
> > 127 /*
> > 128 * We don't need to consider pre_div values smaller than
> > 129 *
> > 130 * period_rate
> > 131 * pre_div_min := ------------------------------------
> > 132 * NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1)
> > 133 *
> > 134 * because pre_div = pre_div_min results in a better
> > 135 * approximation.
> > 136 */
> > 137 pre_div = div64_u64(period_rate,
> > 138 (u64)NSEC_PER_SEC * (IPQ_PWM_MAX_DIV + 1));
> > 139 min_diff = period_rate;
> > 140
> > 141 for (; pre_div <= IPQ_PWM_MAX_DIV; pre_div++) {
> > 142 u64 remainder;
> > 143
> > 144 pwm_div = div64_u64_rem(period_rate,
> > 145 (u64)NSEC_PER_SEC * (pre_div + 1), &remainder);
> > 146 /* pwm_div is unsigned; the check below catches underflow */
> > 147 pwm_div--;
> > 148
> > 149 /*
> > 150 * Swapping values for pre_div and pwm_div produces the same
> > 151 * period length. So we can skip all settings with pre_div >
> > 152 * pwm_div which results in bigger constraints for selecting
> > 153 * the duty_cycle than with the two values swapped.
> > 154 */
> > 155 if (pre_div > pwm_div)
> > 156 break;
> > 157
> > 158 /*
> > 159 * Make sure we can do 100% duty cycle where
> > 160 * hi_dur == pwm_div + 1
> > 161 */
> > 162 if (pwm_div > IPQ_PWM_MAX_DIV - 1)
> > 163 continue;
> > 164
> > 165 if (remainder < min_diff) {
> > 166 best_pre_div = pre_div;
> > 167 best_pwm_div = pwm_div;
> > 168 min_diff = remainder;
> > 169
> > 170 if (min_diff == 0) /* bingo */
> > 171 break;
> > 172 }
> > 173 }
> > 174
> > 175 /* config divider values for the closest possible frequency */
> > 176 config_div_and_duty(pwm, best_pre_div, best_pwm_div,
> > 177 rate, duty_ns, state->enabled);
> > 178
> > 179 return 0;
> > 180 }
> > 181
> >
> > ---
> > 0-DAY CI Kernel Test Service, Intel Corporation
> > https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
>
> --
> ~. .~ Tk Open Systems
> =}------------------------------------------------ooO--U--Ooo------------{=
> - baruch(a)tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
>
224
days inactive
225
days old
2 comments
3 participants
participants (3)
-
Baruch Siach -
kernel test robot -
Nathan Chancellor