Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi

[Aamir Bohra (Code Review)]

Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32322

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi
......................................................................

soc/intel/cannonlake: Add null refrence check for Cnvi

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 4 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/32322/1

diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c
index 2688557..994b31c 100644
--- a/src/soc/intel/cannonlake/fsp_params.c
+++ b/src/soc/intel/cannonlake/fsp_params.c
@@ -254,11 +254,13 @@

/* Enable CNVi Wifi if enabled in device tree */
    dev = dev_find_slot(0, PCH_DEVFN_CNViWIFI);
+	if(dev) {
 #if IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
-	params->CnviMode = dev->enabled;
+		params->CnviMode = dev->enabled;
 #else
-	params->PchCnviMode = dev->enabled;
+		params->PchCnviMode = dev->enabled;
 #endif
+	}
    /* PCI Express */
    for (i = 0; i < ARRAY_SIZE(config->PcieClkSrcUsage); i++) {
    	if (config->PcieClkSrcUsage[i] == 0)

[build bot (Jenkins) (Code Review)]

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/32322/1/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/1/src/soc/intel/cannonlake/fsp_params.... 
PS1, Line 257: 	if(dev) {
space required before the open parenthesis '('

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32322

to look at the new patch set (#2).

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi
......................................................................

soc/intel/cannonlake: Add null refrence check for Cnvi

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 4 insertions(+), 2 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/32322/2

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32322

to look at the new patch set (#3).

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi
......................................................................

soc/intel/cannonlake: Add null refrence check for Cnvi

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 5 insertions(+), 3 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/32322/3

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/#/c/32322/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32322/3//COMMIT_MSG@8 
PS3, Line 8: 
Was this reported by coverity?

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
PS3, Line 258: IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
IS_ENABLED has been deprecated. Please use CONFIG(SOC_INTEL_COMETLAKE) instead like it was before.

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
PS3, Line 247: dev
care to add NULL check

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Subrata Banik, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32322

to look at the new patch set (#4).

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 8 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/32322/4

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 4:

(2 comments)

https://review.coreboot.org/#/c/32322/3//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32322/3//COMMIT_MSG@8 
PS3, Line 8:
> Was this reported by coverity?

I did not check on that. Just caught my eye.

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
PS3, Line 258: IS_ENABLED(CONFIG_SOC_INTEL_COMETLAKE)
> IS_ENABLED has been deprecated. Please use CONFIG(SOC_INTEL_COMETLAKE) instead like it was before.

Sorry that was a miss, corrected it now.

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/3/src/soc/intel/cannonlake/fsp_params.... 
PS3, Line 247: dev
> care to add NULL check

Done

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Subrata Banik (Code Review)]

Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 4: Code-Review+2

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 4:

(1 comment)

https://review.coreboot.org/#/c/32322/4/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/4/src/soc/intel/cannonlake/fsp_params.... 
PS4, Line 249: 
dev->enabled was intentionally being set to 0 here to ensure that the device gets disabled in the device tree if xdci_can_enable returns false. With this change, that won't happen anymore.

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Subrata Banik, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32322

to look at the new patch set (#5).

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 14 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/32322/5

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/#/c/32322/4/src/soc/intel/cannonlake/fsp_params.... 
File src/soc/intel/cannonlake/fsp_params.c:

https://review.coreboot.org/#/c/32322/4/src/soc/intel/cannonlake/fsp_params.... 
PS4, Line 249:
> dev->enabled was intentionally being set to 0 here to ensure that the device gets disabled in the de […]

Ok. Understood, revised under PS#5

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 5: Code-Review+2

Change in ...coreboot[master]: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null refrence check for Cnvi and Xdci
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/#/c/32322/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32322/5//COMMIT_MSG@7 
PS5, Line 7: refrence
reference

Change in ...coreboot[master]: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Hello Patrick Rudolph, Subrata Banik, Rizwan Qureshi, build bot (Jenkins), Furquan Shaikh,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/32322

to look at the new patch set (#6).

Change subject: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci
......................................................................

soc/intel/cannonlake: Add null reference check for Cnvi and Xdci

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 14 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/32322/6

Change in ...coreboot[master]: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci

[Aamir Bohra (Code Review)]

Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci
......................................................................

Patch Set 6:

(1 comment)

https://review.coreboot.org/#/c/32322/5//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/32322/5//COMMIT_MSG@7 
PS5, Line 7: refrence
> reference

Done

Change in ...coreboot[master]: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci

[Maulik V Vaghela (Code Review)]

Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci
......................................................................

Patch Set 6: Code-Review+2

Change in ...coreboot[master]: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32322 )

Change subject: soc/intel/cannonlake: Add null reference check for Cnvi and Xdci
......................................................................

soc/intel/cannonlake: Add null reference check for Cnvi and Xdci

Change-Id: I2e1011d9ac93ed764b6c2aa425928a972ec2aa43
Signed-off-by: Aamir Bohra 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32322
Reviewed-by: Maulik V Vaghela 
Reviewed-by: Furquan Shaikh 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/cannonlake/fsp_params.c
1 file changed, 14 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Furquan Shaikh: Looks good to me, approved
  Maulik V Vaghela: Looks good to me, approved

diff --git a/src/soc/intel/cannonlake/fsp_params.c b/src/soc/intel/cannonlake/fsp_params.c
index b8dba18..2b83275 100644
--- a/src/soc/intel/cannonlake/fsp_params.c
+++ b/src/soc/intel/cannonlake/fsp_params.c
@@ -245,9 +245,12 @@

/* Enable xDCI controller if enabled in devicetree and allowed */
    dev = dev_find_slot(0, PCH_DEVFN_USBOTG);
-	if (!xdci_can_enable())
-		dev->enabled = 0;
-	params->XdciEnable = dev->enabled;
+	if (dev) {
+		if (!xdci_can_enable())
+			dev->enabled = 0;
+		params->XdciEnable = dev->enabled;
+	} else
+		params->XdciEnable = 0;

/* Set Debug serial port */
    params->SerialIoDebugUartNumber = CONFIG_UART_FOR_CONSOLE;
@@ -255,9 +258,15 @@
    /* Enable CNVi Wifi if enabled in device tree */
    dev = dev_find_slot(0, PCH_DEVFN_CNViWIFI);
 #if CONFIG(SOC_INTEL_COMETLAKE)
-	params->CnviMode = dev->enabled;
+	if (dev)
+		params->CnviMode = dev->enabled;
+	else
+		params->CnviMode = 0;
 #else
-	params->PchCnviMode = dev->enabled;
+	if (dev)
+		params->PchCnviMode = dev->enabled;
+	else
+		params->PchCnviMode = 0;
 #endif
    /* PCI Express */
    for (i = 0; i < ARRAY_SIZE(config->PcieClkSrcUsage); i++) {