From: Andy Shevchenko <andriy.shevchenko@intel.com>
To: Aldo Conte <aldocontelk@gmail.com>
Cc: jic23@kernel.org, dlechner@baylibre.com, nuno.sa@analog.com,
andy@kernel.org, shuah@kernel.org, joshua.crofts1@gmail.com,
linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-kernel-mentees@lists.linux.dev
Subject: Re: [PATCH v4] iio: tcs3472: implement wait time and sampling frequency
Date: Tue, 9 Jun 2026 17:41:40 +0300 [thread overview]
Message-ID: <aigmJD2r96nCu2dg@ashevche-desk.local> (raw)
In-Reply-To: <dd8129c5-fe15-465e-8203-103be3d0c9c0@gmail.com>
On Tue, Jun 09, 2026 at 03:36:03PM +0200, Aldo Conte wrote:
> On 09/06/26 14:15, Andy Shevchenko wrote:
> > On Tue, Jun 09, 2026 at 12:27:52PM +0200, Aldo Conte wrote:
...
> > > case IIO_EV_INFO_PERIOD:
> > > period = tcs3472_cycle_time_us(data) *
> > > tcs3472_intr_pers[data->apers];
> > > *val = period / USEC_PER_SEC;
> > > *val2 = period % USEC_PER_SEC;
> >
> > Again, use HZ-based multiplier, see above.
>
> Here i do not understand why MICROHZ_PER_HZ is better.
> In the previous case, we were indeed referring to the denominator of the
> calculation, which was actually a frequency, so MICROHZ_PER_HZ is indeed
> correct.
> But in this case, we are calculating the integer and fractional parts of a
> period, so I think this would be fine.
> Tell me where I’m going wrong.
Hmm... I think you are right and I stand corrected.
> > > return IIO_VAL_INT_PLUS_MICRO;
> > >
> > > write becomes:
> > >
> > > case IIO_EV_INFO_PERIOD:{
> > > unsigned int cycle_us;
> > >
> > > period = val * USEC_PER_SEC + val2;
> >
> > And again, use HZ-based multiplier, see above.
>
> as above...
...
> > > for (i = 1; i < ARRAY_SIZE(tcs3472_intr_pers) - 1; i++) {
> > > if (period <= cycle_us * tcs3472_intr_pers[i])
> > > break;
> > > }
> >
> > do {} while () seems better choice here (and do it in reverse order?).
>
> Could you suggest the do {} while {} form, which seems better? Because I
> think it is already comprehensible.
OK, it's not do {} while
i = ARRAY_SIZE(tcs3472_intr_pers);
while (i--) {
if (period > cycle_us * tcs3472_intr_pers[i])
break;
}
if I am not mistaken... OTOH, the for-loop is in the original code, perhaps
better to leave this approach and change separately if needed.
--
With Best Regards,
Andy Shevchenko
prev parent reply other threads:[~2026-06-09 14:41 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-07 11:27 [PATCH v4] iio: tcs3472: implement wait time and sampling frequency Aldo Conte
2026-06-09 6:44 ` Andy Shevchenko
2026-06-09 10:27 ` Aldo Conte
2026-06-09 12:15 ` Andy Shevchenko
2026-06-09 13:36 ` Aldo Conte
2026-06-09 14:41 ` Andy Shevchenko [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aigmJD2r96nCu2dg@ashevche-desk.local \
--to=andriy.shevchenko@intel.com \
--cc=aldocontelk@gmail.com \
--cc=andy@kernel.org \
--cc=dlechner@baylibre.com \
--cc=jic23@kernel.org \
--cc=joshua.crofts1@gmail.com \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel-mentees@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=nuno.sa@analog.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox