Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34572 )
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
soc/nvidia/tegra124: Assert divisor is non-zero
The logic for the calculation of plld.m is rather complicated, so do a sanity check that it is non-zero before doing the division.
Change-Id: I60f49b8eed47a3de86713304bde7a4d3f3d935dd Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1260981 --- M src/soc/nvidia/tegra124/clock.c 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34572/1
diff --git a/src/soc/nvidia/tegra124/clock.c b/src/soc/nvidia/tegra124/clock.c index 6877c04..8fb4b05 100644 --- a/src/soc/nvidia/tegra124/clock.c +++ b/src/soc/nvidia/tegra124/clock.c @@ -377,6 +377,7 @@ printk(BIOS_WARNING, "%s: Failed to match output frequency %u, " "best difference is %u.\n", __func__, frequency, best_diff); + assert(plld.m != 0); rounded_rate = (ref / plld.m * plld.n) >> plld.p; }
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34572 )
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34572/1/src/soc/nvidia/tegra124/clo... File src/soc/nvidia/tegra124/clock.c:
https://review.coreboot.org/c/coreboot/+/34572/1/src/soc/nvidia/tegra124/clo... PS1, Line 380: assert(plld.m != 0); Why only m and not n? Seems pretty much the same situation in both cases here.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34572 )
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/34572/1/src/soc/nvidia/tegra124/clo... File src/soc/nvidia/tegra124/clock.c:
https://review.coreboot.org/c/coreboot/+/34572/1/src/soc/nvidia/tegra124/clo... PS1, Line 380: assert(plld.m != 0);
Why only m and not n? Seems pretty much the same situation in both cases here.
Oh wait, I missed how n is a multiplier again (that formula could use some more parenthesis...). Ignore this then.
Hello Julius Werner, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34572
to look at the new patch set (#2).
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
soc/nvidia/tegra124: Assert divisor is non-zero
The logic for the calculation of plld.m is rather complicated, so do a sanity check that it is non-zero before doing the division.
Change-Id: I60f49b8eed47a3de86713304bde7a4d3f3d935dd Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1260981 --- M src/soc/nvidia/tegra124/clock.c 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/34572/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34572 )
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34572 )
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
Patch Set 2: Code-Review+1
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34572 )
Change subject: soc/nvidia/tegra124: Assert divisor is non-zero ......................................................................
soc/nvidia/tegra124: Assert divisor is non-zero
The logic for the calculation of plld.m is rather complicated, so do a sanity check that it is non-zero before doing the division.
Change-Id: I60f49b8eed47a3de86713304bde7a4d3f3d935dd Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1260981 Reviewed-on: https://review.coreboot.org/c/coreboot/+/34572 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/soc/nvidia/tegra124/clock.c 1 file changed, 2 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Julius Werner: Looks good to me, approved
diff --git a/src/soc/nvidia/tegra124/clock.c b/src/soc/nvidia/tegra124/clock.c index 6877c04..bb0343d 100644 --- a/src/soc/nvidia/tegra124/clock.c +++ b/src/soc/nvidia/tegra124/clock.c @@ -13,6 +13,7 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ +#include <assert.h> #include <arch/clock.h> #include <device/mmio.h> #include <console/console.h> @@ -377,6 +378,7 @@ printk(BIOS_WARNING, "%s: Failed to match output frequency %u, " "best difference is %u.\n", __func__, frequency, best_diff); + assert(plld.m != 0); rounded_rate = (ref / plld.m * plld.n) >> plld.p; }
-
Jacob Garber (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)