Jacob Garber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34499 )
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
soc/qualcomm/ipq806x: Remove unnecessary allocation
The bus variable doesn't live outside the scope of this function, and is only used as a convenient way for passing the pointers to all the sub-functions, so it doesn't need to be allocated. Put it on the stack instead.
Change-Id: I4370d77445952731d20f7d9a91803612f4d21aef Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1294801 --- M src/soc/qualcomm/ipq806x/lcc.c 1 file changed, 5 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34499/1
diff --git a/src/soc/qualcomm/ipq806x/lcc.c b/src/soc/qualcomm/ipq806x/lcc.c index 68b878c..5a9274f 100644 --- a/src/soc/qualcomm/ipq806x/lcc.c +++ b/src/soc/qualcomm/ipq806x/lcc.c @@ -287,13 +287,7 @@
int audio_clock_config(unsigned frequency) { - Ipq806xLccClocks *bus = malloc(sizeof(*bus)); - - if (!bus) { - printk(BIOS_ERR, "%s: failed to allocate bus structure\n", - __func__); - return 1; - } + Ipq806xLccClocks bus;
bus->gcc_apcs_regs = (void *)(MSM_GCC_BASE + GCC_PLL_APCS_REG); bus->lcc_pll0_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_PLL0_MODE_REG); @@ -301,15 +295,13 @@ bus->lcc_mi2s_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_MI2S_NS_REG); bus->lcc_pll_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_PLL_PCLK_REG);
- - if (lcc_init_enable_pll0(bus)) + if (lcc_init_enable_pll0(&bus)) return 1; - if (lcc_init_enable_ahbix(bus)) + if (lcc_init_enable_ahbix(&bus)) return 1; - if (lcc_init_mi2s(bus, frequency)) + if (lcc_init_mi2s(&bus, frequency)) return 1; - - if (lcc_enable_mi2s(bus)) + if (lcc_enable_mi2s(&bus)) return 1;
return 0;
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34499 )
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
Patch Set 1:
(1 comment)
LGTM once you make it compile.
https://review.coreboot.org/c/coreboot/+/34499/1/src/soc/qualcomm/ipq806x/lc... File src/soc/qualcomm/ipq806x/lcc.c:
https://review.coreboot.org/c/coreboot/+/34499/1/src/soc/qualcomm/ipq806x/lc... PS1, Line 292: -> These all need to be dots now.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34499
to look at the new patch set (#2).
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
soc/qualcomm/ipq806x: Remove unnecessary allocation
The bus variable doesn't live outside the scope of this function, and is only used as a convenient way for passing the pointers to all the sub-functions, so it doesn't need to be allocated. Put it on the stack instead.
Change-Id: I4370d77445952731d20f7d9a91803612f4d21aef Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1294801 --- M src/soc/qualcomm/ipq806x/lcc.c 1 file changed, 11 insertions(+), 19 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/34499/2
Jacob Garber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34499 )
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34499/1/src/soc/qualcomm/ipq806x/lc... File src/soc/qualcomm/ipq806x/lcc.c:
https://review.coreboot.org/c/coreboot/+/34499/1/src/soc/qualcomm/ipq806x/lc... PS1, Line 292: ->
These all need to be dots now.
Oops, thanks
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34499 )
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34499 )
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
Patch Set 2: Code-Review+1
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34499 )
Change subject: soc/qualcomm/ipq806x: Remove unnecessary allocation ......................................................................
soc/qualcomm/ipq806x: Remove unnecessary allocation
The bus variable doesn't live outside the scope of this function, and is only used as a convenient way for passing the pointers to all the sub-functions, so it doesn't need to be allocated. Put it on the stack instead.
Change-Id: I4370d77445952731d20f7d9a91803612f4d21aef Signed-off-by: Jacob Garber jgarber1@ualberta.ca Found-by: Coverity CID 1294801 Reviewed-on: https://review.coreboot.org/c/coreboot/+/34499 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/qualcomm/ipq806x/lcc.c 1 file changed, 11 insertions(+), 19 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/qualcomm/ipq806x/lcc.c b/src/soc/qualcomm/ipq806x/lcc.c index 68b878c..8f862b4 100644 --- a/src/soc/qualcomm/ipq806x/lcc.c +++ b/src/soc/qualcomm/ipq806x/lcc.c @@ -287,29 +287,21 @@
int audio_clock_config(unsigned frequency) { - Ipq806xLccClocks *bus = malloc(sizeof(*bus)); + Ipq806xLccClocks bus = { + .gcc_apcs_regs = (void *)(MSM_GCC_BASE + GCC_PLL_APCS_REG), + .lcc_pll0_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_PLL0_MODE_REG), + .lcc_ahbix_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_AHBIX_NS_REG), + .lcc_mi2s_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_MI2S_NS_REG), + .lcc_pll_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_PLL_PCLK_REG), + };
- if (!bus) { - printk(BIOS_ERR, "%s: failed to allocate bus structure\n", - __func__); + if (lcc_init_enable_pll0(&bus)) return 1; - } - - bus->gcc_apcs_regs = (void *)(MSM_GCC_BASE + GCC_PLL_APCS_REG); - bus->lcc_pll0_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_PLL0_MODE_REG); - bus->lcc_ahbix_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_AHBIX_NS_REG); - bus->lcc_mi2s_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_MI2S_NS_REG); - bus->lcc_pll_regs = (void *)(MSM_LPASS_LCC_BASE + LCC_PLL_PCLK_REG); - - - if (lcc_init_enable_pll0(bus)) + if (lcc_init_enable_ahbix(&bus)) return 1; - if (lcc_init_enable_ahbix(bus)) + if (lcc_init_mi2s(&bus, frequency)) return 1; - if (lcc_init_mi2s(bus, frequency)) - return 1; - - if (lcc_enable_mi2s(bus)) + if (lcc_enable_mi2s(&bus)) return 1;
return 0;
-
Jacob Garber (Code Review) -
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)