From: Matt Porter <mporter@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] Illegal use of FP ops in clock_ti814x.c
Date: Tue, 28 Jan 2014 12:48:48 -0500 [thread overview]
Message-ID: <20140128174848.GQ31176@beef> (raw)
In-Reply-To: <526FA8AB.9050500@ti.com>
On Tue, Oct 29, 2013 at 08:23:07AM -0400, Tom Rini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 10/29/2013 06:48 AM, Wolfgang Denk wrote:
> > Dear M?ns Rullg?rd,
> >
> > In message <yw1x8uxc28y9.fsf@unicorn.mansr.com> you wrote:
> >>
> >>>> Something like this should be equivalent. That said, it
> >>>> looks suspiciously like it's meant to simply do a division
> >>>> and round up. If that is the case, +225 should be +249. It
> >>>> probably makes no difference for the values actually
> >>>> encountered.
> >>>
> >>> Umm... this is the part which I do not understand.
> >>>
> >>> The original code adds 90%; you add 90%, too. However, to
> >>> round up, one usually adds only 50% ?
> >>
> >> Adding 50% would round to nearest. For integer division to round
> >> up, you must add one less than the divisor.
> >
> > Agreed. But do we want to round up? The original code used +90%,
> > which is something else, too...
>
> And I imagine it's unlikely the original author of the code is around
> anymore, or recalls exactly why. I'm pretty sure Matt just lifted the
> code from the vendor tree and since it wasn't throwing warnings didn't
> notice the floating point part.
>
> >>> Where are these 90% coming from? Are they in any way
> >>> meaningful, or even critical?
> >>
> >> My guess is that it was someone's approximation of 249 / 250. I
> >> don't know the hardware, so it's conceivable that it really
> >> should be this way, although it seems unlikely.
> >
> > Are you able to test such a modificationon actual hardware?
>
> I suspect Matt can, after Linaro Connect. I don't have one of these
> platforms handy but I think he still does.
Thankfully Tom reminded me of this because I lost some of the list
traffic due to some local mail issues.
Although not explicitly mentioned in the TRM or any application
notes I can find, the 90% appears to come from jitter compensation for
the delta sigma fractional divider. I see some comments that imply this
in various old vendor kernel tree clock implementations where they are
rounding various pll constants. I can't be 100% sure without some
insight from TI folks. Given that it's working for our known users, I'd
like to preserve that until we get somebody that can shed some light on
that.
As Tom guessed, I low-level cherry-picked a lot of pieces from long-lost
authors in this area. I obviously missed this floating point math in the
cleanup.
I tested this patch on my TI8148 EVM and it works as expected.
Acked-by: Matt Porter <mporter@linaro.org>
-Matt
next prev parent reply other threads:[~2014-01-28 17:48 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-10-27 21:11 [U-Boot] Illegal use of FP ops in clock_ti814x.c Wolfgang Denk
2013-10-28 21:55 ` Tom Rini
2013-10-28 23:19 ` Måns Rullgård
2013-10-28 23:56 ` Wolfgang Denk
2013-10-29 0:54 ` Måns Rullgård
2013-10-29 10:48 ` Wolfgang Denk
2013-10-29 12:23 ` Tom Rini
2014-01-28 17:48 ` Matt Porter [this message]
2013-10-29 12:44 ` Måns Rullgård
2014-02-21 19:14 ` Tom Rini
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=20140128174848.GQ31176@beef \
--to=mporter@linaro.org \
--cc=u-boot@lists.denx.de \
/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