Messages in this thread | | | Date | Wed, 24 Jun 2020 18:28:18 +0200 | From | Uwe Kleine-König <> | Subject | Re: [RESEND PATCH V13 3/3] Input: new da7280 haptic driver |
| |
Hello,
On Wed, Jun 24, 2020 at 03:39:52PM +0000, Roy Im wrote: > On Wed, Jun 24, 2020 at 10:37 PM, Uwe Kleine-König wrote: > > On Wed, Jun 24, 2020 at 12:04:24PM +0900, Roy Im wrote: > > > period2freq = 1000000 / state.period; > > > if (period2freq < DA7280_MIN_PWM_FREQ_KHZ || > > > period2freq > DA7280_MAX_PWM_FREQ_KHZ) { > > > dev_err(dev, "Unsupported PWM frequency (%u)\n", > > > period2freq); > > > return -EINVAL; > > > } > > > If you look ok, let me update this so. > > > > looks fine. > > > > Note that you don't need the division if you check for: > > > > if (state.period > 100000 || state.period < 4000) { > > ... > > > > (maybe the compiler is already clever enough to get rid of the division for you, but then the check is: > > > > if (state.period > 100000 || state.period < 3985) { > > > > because of rounding errors.) > > OK, you are right, that will be better. So let me change that as below > if you look fine.
I look fine, the code however is wrong. :-)
> /* Check PWM period, PWM freq = 1000000 / state.period. > * The valid PWM freq range: 10k ~ 250kHz. > */ > if (state.period > 100000 || state.period < 3985) {
You want 4000 here ---------------------------------^^^^, don't you?
> dev_err(dev, "Unsupported PWM period (%u)\n", > state.period); > return -EINVAL; > }
Best regards Uwe
-- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | https://www.pengutronix.de/ | [unhandled content-type:application/pgp-signature] | |