Change in ...coreboot[master]: soc/intel/common: Set GSPI clock value to prevent division by zero
John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32974
Change subject: soc/intel/common: Set GSPI clock value to prevent division by zero ......................................................................
soc/intel/common: Set GSPI clock value to prevent division by zero
Clang Static Analyzer version 8.0.0 detects the division by zero if gspi_clk_mhz is initialized to 0. gspi_clk_mhz is referred to speed_mhz in devicetree. Set gspi_clk_mhz to 1 if it is detected as 0 in order to prevent the division by zero in DIV_ROUND_UP operation. Then the value of (ref_clk_mhz - 1) will be fed into GSPI's Serial Clock Rate value.
TEST=Built and boot up to kernel.
Change-Id: I6a09474bff114c57d7a9c4c232bb636ff287e4d5 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/common/block/gspi/gspi.c 1 file changed, 4 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/32974/1
diff --git a/src/soc/intel/common/block/gspi/gspi.c b/src/soc/intel/common/block/gspi/gspi.c index 81eb7ee..fb3bf7a 100644 --- a/src/soc/intel/common/block/gspi/gspi.c +++ b/src/soc/intel/common/block/gspi/gspi.c @@ -437,9 +437,11 @@ { const uint32_t ref_clk_mhz = CONFIG_SOC_INTEL_COMMON_BLOCK_GSPI_CLOCK_MHZ; - const uint32_t gspi_clk_mhz = gspi_get_bus_clk_mhz(gspi_bus); + uint32_t gspi_clk_mhz = gspi_get_bus_clk_mhz(gspi_bus);
- assert(gspi_clk_mhz != 0); + if (!gspi_clk_mhz) + gspi_clk_mhz = 1; + assert(ref_clk_mhz != 0); return (DIV_ROUND_UP(ref_clk_mhz, gspi_clk_mhz) - 1) & SSCR0_SCR_MASK; }
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32974 )
Change subject: soc/intel/common: Set GSPI clock value to prevent division by zero ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/#/c/32974/1/src/soc/intel/common/block/gspi/gspi... File src/soc/intel/common/block/gspi/gspi.c:
https://review.coreboot.org/#/c/32974/1/src/soc/intel/common/block/gspi/gspi... PS1, Line 442: shouldn't the assert catch this?
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32974 )
Change subject: soc/intel/common: Set GSPI clock value to prevent division by zero ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32974/1/src/soc/intel/common/block/gspi/gspi... File src/soc/intel/common/block/gspi/gspi.c:
https://review.coreboot.org/#/c/32974/1/src/soc/intel/common/block/gspi/gspi... PS1, Line 442:
shouldn't the assert catch this?
Asserts aren't fatal by default in coreboot.
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32974 )
Change subject: soc/intel/common: Set GSPI clock value to prevent division by zero ......................................................................
soc/intel/common: Set GSPI clock value to prevent division by zero
Clang Static Analyzer version 8.0.0 detects the division by zero if gspi_clk_mhz is initialized to 0. gspi_clk_mhz is referred to speed_mhz in devicetree. Set gspi_clk_mhz to 1 if it is detected as 0 in order to prevent the division by zero in DIV_ROUND_UP operation. Then the value of (ref_clk_mhz - 1) will be fed into GSPI's Serial Clock Rate value.
TEST=Built and boot up to kernel.
Change-Id: I6a09474bff114c57d7a9c4c232bb636ff287e4d5 Signed-off-by: John Zhao john.zhao@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/32974 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Duncan Laurie dlaurie@chromium.org --- M src/soc/intel/common/block/gspi/gspi.c 1 file changed, 4 insertions(+), 2 deletions(-)
Approvals: build bot (Jenkins): Verified Duncan Laurie: Looks good to me, approved
diff --git a/src/soc/intel/common/block/gspi/gspi.c b/src/soc/intel/common/block/gspi/gspi.c index 7fd7d0f..17532bf 100644 --- a/src/soc/intel/common/block/gspi/gspi.c +++ b/src/soc/intel/common/block/gspi/gspi.c @@ -434,9 +434,11 @@ { const uint32_t ref_clk_mhz = CONFIG_SOC_INTEL_COMMON_BLOCK_GSPI_CLOCK_MHZ; - const uint32_t gspi_clk_mhz = gspi_get_bus_clk_mhz(gspi_bus); + uint32_t gspi_clk_mhz = gspi_get_bus_clk_mhz(gspi_bus);
- assert(gspi_clk_mhz != 0); + if (!gspi_clk_mhz) + gspi_clk_mhz = 1; + assert(ref_clk_mhz != 0); return (DIV_ROUND_UP(ref_clk_mhz, gspi_clk_mhz) - 1) & SSCR0_SCR_MASK; }
-
Duncan Laurie (Code Review) -
John Zhao (Code Review)
-
Martin Roth (Code Review)
-
Patrick Georgi (Code Review)