Change in ...coreboot[master]: soc/intel/icelake: Enable Audio FSP UPD based on devicetree HDA presence
Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33933
Change subject: soc/intel/icelake: Enable Audio FSP UPD based on devicetree HDA presence ......................................................................
soc/intel/icelake: Enable Audio FSP UPD based on devicetree HDA presence
Change-Id: I28c2beca4bc26ddb896e68886571ebdc82276b48 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/romstage/fsp_params.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/33933/1
diff --git a/src/soc/intel/icelake/romstage/fsp_params.c b/src/soc/intel/icelake/romstage/fsp_params.c index 3c49fee..3179d1a 100644 --- a/src/soc/intel/icelake/romstage/fsp_params.c +++ b/src/soc/intel/icelake/romstage/fsp_params.c @@ -25,7 +25,7 @@ const struct soc_intel_icelake_config *config) { unsigned int i; - const struct device *dev = pcidev_on_root(0, 0); + const struct device *dev; uint32_t mask = 0;
/* Set IGD stolen size to 60MB. */ @@ -36,6 +36,7 @@ m_cfg->UserBd = BOARD_TYPE_ULT_ULX; m_cfg->RMT = config->RMT; m_cfg->SkipMbpHob = 1; + dev = pcidev_on_root(0x1f, 3); /* If Audio Codec is enabled, enable FSP UPD */ if (!dev) m_cfg->PchHdaEnable = 0;
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Enable Audio FSP UPD based on devicetree HDA presence ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/33933/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33933/1//COMMIT_MSG@7 PS1, Line 7: soc/intel/icelake: Enable Audio FSP UPD based on devicetree HDA presence I think 'Disable HDA based on devicetree" would be enough here.
It was previously always enabled, right?
https://review.coreboot.org/#/c/33933/1/src/soc/intel/icelake/romstage/fsp_p... File src/soc/intel/icelake/romstage/fsp_params.c:
https://review.coreboot.org/#/c/33933/1/src/soc/intel/icelake/romstage/fsp_p... PS1, Line 39: dev = pcidev_on_root(0x1f, 3); Place this below comment, empty line above comment to make it look pretty.
Hello Kyösti Mälkki, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/33933
to look at the new patch set (#2).
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
soc/intel/icelake: Disable HDA based on devicetree
Change-Id: I28c2beca4bc26ddb896e68886571ebdc82276b48 Signed-off-by: Subrata Banik subrata.banik@intel.com --- M src/soc/intel/icelake/romstage/fsp_params.c 1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/33933/2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/33933/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/33933/1//COMMIT_MSG@7 PS1, Line 7: soc/intel/icelake: Enable Audio FSP UPD based on devicetree HDA presence
I think 'Disable HDA based on devicetree" would be enough here. […]
Done
https://review.coreboot.org/#/c/33933/1/src/soc/intel/icelake/romstage/fsp_p... File src/soc/intel/icelake/romstage/fsp_params.c:
https://review.coreboot.org/#/c/33933/1/src/soc/intel/icelake/romstage/fsp_p... PS1, Line 39: dev = pcidev_on_root(0x1f, 3);
Place this below comment, empty line above comment to make it look pretty.
Done
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
Patch Set 2: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
Patch Set 2: Code-Review+1
Subrata Banik has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
soc/intel/icelake: Disable HDA based on devicetree
Change-Id: I28c2beca4bc26ddb896e68886571ebdc82276b48 Signed-off-by: Subrata Banik subrata.banik@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/33933 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net --- M src/soc/intel/icelake/romstage/fsp_params.c 1 file changed, 3 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/icelake/romstage/fsp_params.c b/src/soc/intel/icelake/romstage/fsp_params.c index 3c49fee..fa6f939 100644 --- a/src/soc/intel/icelake/romstage/fsp_params.c +++ b/src/soc/intel/icelake/romstage/fsp_params.c @@ -25,7 +25,7 @@ const struct soc_intel_icelake_config *config) { unsigned int i; - const struct device *dev = pcidev_on_root(0, 0); + const struct device *dev; uint32_t mask = 0;
/* Set IGD stolen size to 60MB. */ @@ -36,7 +36,9 @@ m_cfg->UserBd = BOARD_TYPE_ULT_ULX; m_cfg->RMT = config->RMT; m_cfg->SkipMbpHob = 1; + /* If Audio Codec is enabled, enable FSP UPD */ + dev = pcidev_on_root(0x1f, 3); if (!dev) m_cfg->PchHdaEnable = 0; else
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
Patch Set 3:
Please try to honour the 24 hour submit rule.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33933/3/src/soc/intel/icelake/romst... File src/soc/intel/icelake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/33933/3/src/soc/intel/icelake/romst... PS3, Line 41: pcidev_on_root(0x1f, 3); nit: If you use pcidev_path_on_root you can specify PCH_DEVFN_HDA as argument instead of having to put in slot and function number as arguments.
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33933 )
Change subject: soc/intel/icelake: Disable HDA based on devicetree ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/33933/3/src/soc/intel/icelake/romst... File src/soc/intel/icelake/romstage/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/33933/3/src/soc/intel/icelake/romst... PS3, Line 41: pcidev_on_root(0x1f, 3);
nit: If you use pcidev_path_on_root you can specify PCH_DEVFN_HDA as argument instead of having to p […]
I still have one hanging CL, there i can fix this. thanks
-
Furquan Shaikh (Code Review) -
Kyösti Mälkki (Code Review)
-
Paul Menzel (Code Review)
-
Subrata Banik (Code Review)