Change in coreboot[master]: sc7180: Remove LIMITS_CFG loading in romstage.

[Ravi kumar (Code Review)]

Ravi kumar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

sc7180: Remove LIMITS_CFG loading in romstage.

Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
---
M src/soc/qualcomm/sc7180/qclib.c
1 file changed, 0 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/45704/1

diff --git a/src/soc/qualcomm/sc7180/qclib.c b/src/soc/qualcomm/sc7180/qclib.c
index 6affc63..274d906 100644
--- a/src/soc/qualcomm/sc7180/qclib.c
+++ b/src/soc/qualcomm/sc7180/qclib.c
@@ -9,7 +9,6 @@
 int qclib_soc_blob_load(void)
 {
    size_t size;
-	ssize_t ssize;

/* Attempt to load PMICCFG Blob */
    size = cbfs_boot_load_file(CONFIG_CBFS_PREFIX "/pmiccfg",
@@ -25,13 +24,5 @@
    	return -1;
    qclib_add_if_table_entry(QCLIB_TE_DCB_SETTINGS, _dcb, size, 0);

-	/* Attempt to load Limits Config Blob */
-	ssize = fmap_read_area(QCLIB_FR_LIMITS_CFG_DATA, _limits_cfg,
-		REGION_SIZE(limits_cfg));
-	if (ssize < 0)
-		return -1;
-	qclib_add_if_table_entry(QCLIB_TE_LIMITS_CFG_DATA,
-				 _limits_cfg, ssize, 0);
-
    return 0;
 }

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 1:

What about the constants in <soc/qclib_common.h> and the memlayout section? Just do a `grep -i limits_cfg` over the repository to make sure you got it all.

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45704/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45704/1//COMMIT_MSG@9 
PS1, Line 9: Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
Commit message needs a couple of full sentences to justify the patch and a Signed-off-by tag at the end.

[mturney mturney (Code Review)]

mturney mturney has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 1:
> Patch Set 1:
>
> What about the constants in <soc/qclib_common.h> and the memlayout section? Just do a `grep -i limits_cfg` over the repository to make sure you got it all.

We aren't touching code in common/ as other QC chips make use of this code.
All code under sc7180/ has been cleansed of limits_cfg support.

[Ravi kumar (Code Review)]

Hello build bot (Jenkins), mturney mturney, Julius Werner,

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

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

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

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

sc7180: Remove LIMITS_CFG loading in romstage.

LIMITS_CFG is not used/required by trogdor. Supporting this requires an FMAP
partition as well as code, removing this support saves space and maintenance
headaches.

Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
Signed-off-by: Manideep Kurumella 
---
M src/soc/qualcomm/sc7180/qclib.c
1 file changed, 0 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/45704/2

[Ravi Kumar Bokka (Code Review)]

Ravi Kumar Bokka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45704/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/45704/1//COMMIT_MSG@9 
PS1, Line 9: Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
> Commit message needs a couple of full sentences to justify the patch and a Signed-off-by tag at the  […]

Done

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 2:
> All code under sc7180/ has been cleansed of limits_cfg support.

But it hasn't? It's still in src/soc/qualcomm/sc7180/memlayout.ld.

[Ravi kumar (Code Review)]

Hello build bot (Jenkins), mturney mturney, Julius Werner,

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

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

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

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

sc7180: Remove LIMITS_CFG loading in romstage.

LIMITS_CFG is not used/required by trogdor. Supporting this requires an FMAP
partition as well as code, removing this support saves space and maintenance
headaches.

Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
Signed-off-by: Manideep Kurumella 
---
M src/soc/qualcomm/sc7180/memlayout.ld
M src/soc/qualcomm/sc7180/qclib.c
2 files changed, 1 insertion(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/45704/3

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45704/3/src/soc/qualcomm/sc7180/mem... 
File src/soc/qualcomm/sc7180/memlayout.ld:

https://review.coreboot.org/c/coreboot/+/45704/3/src/soc/qualcomm/sc7180/mem... 
PS3, Line 46: 	REGION(qclib, 0x1486a000, 600K, 4K)
The QcLib load address is hardcoded in the QcLib binary. Adding more space here is not useful, it will not be used (unless we want to uprev QcLib for this which seems a bit excessive for 4K). Just leave it empty or add it to the PMIC part or something.

[Ravi kumar (Code Review)]

Hello build bot (Jenkins), mturney mturney, Julius Werner,

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

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

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

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

sc7180: Remove LIMITS_CFG loading in romstage.

LIMITS_CFG is not used/required by trogdor. Supporting this requires an FMAP
partition as well as code, removing this support saves space and maintenance
headaches.

Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
Signed-off-by: Manideep Kurumella 
---
M src/soc/qualcomm/sc7180/memlayout.ld
M src/soc/qualcomm/sc7180/qclib.c
2 files changed, 1 insertion(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/45704/4

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 5: Code-Review+2

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/45704/3/src/soc/qualcomm/sc7180/mem... 
File src/soc/qualcomm/sc7180/memlayout.ld:

https://review.coreboot.org/c/coreboot/+/45704/3/src/soc/qualcomm/sc7180/mem... 
PS3, Line 46: 	REGION(qclib, 0x1486a000, 600K, 4K)
> The QcLib load address is hardcoded in the QcLib binary. […]

Done

[Julius Werner (Code Review)]

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45704 )

Change subject: sc7180: Remove LIMITS_CFG loading in romstage.
......................................................................

sc7180: Remove LIMITS_CFG loading in romstage.

LIMITS_CFG is not used/required by trogdor. Supporting this requires an FMAP
partition as well as code, removing this support saves space and maintenance
headaches.

Change-Id: I9f57f5b520599ba6d708c91df9851e0e86b4b6c0
Signed-off-by: Manideep Kurumella 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45704
Reviewed-by: Julius Werner 
Tested-by: build bot (Jenkins) 
---
M src/soc/qualcomm/sc7180/memlayout.ld
M src/soc/qualcomm/sc7180/qclib.c
2 files changed, 1 insertion(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved

diff --git a/src/soc/qualcomm/sc7180/memlayout.ld b/src/soc/qualcomm/sc7180/memlayout.ld
index 0142242..9b0adc4 100644
--- a/src/soc/qualcomm/sc7180/memlayout.ld
+++ b/src/soc/qualcomm/sc7180/memlayout.ld
@@ -42,8 +42,7 @@
    REGION(ddr_information, 0x14853000, 1K, 1K)
    FMAP_CACHE(0x14853400, 2K)
    REGION(dcb, 0x1485b000, 16K, 4K)
-	REGION(pmic, 0x1485f000, 44K, 4K)
-	REGION(limits_cfg, 0x1486a000, 4K, 4K)
+	REGION(pmic, 0x1485f000, 48K, 4K)
    REGION(qclib, 0x1486b000, 596K, 4K)
    BSRAM_END(0x14900000)

diff --git a/src/soc/qualcomm/sc7180/qclib.c b/src/soc/qualcomm/sc7180/qclib.c
index 6affc63..274d906 100644
--- a/src/soc/qualcomm/sc7180/qclib.c
+++ b/src/soc/qualcomm/sc7180/qclib.c
@@ -9,7 +9,6 @@
 int qclib_soc_blob_load(void)
 {
    size_t size;
-	ssize_t ssize;

/* Attempt to load PMICCFG Blob */
    size = cbfs_boot_load_file(CONFIG_CBFS_PREFIX "/pmiccfg",
@@ -25,13 +24,5 @@
    	return -1;
    qclib_add_if_table_entry(QCLIB_TE_DCB_SETTINGS, _dcb, size, 0);

-	/* Attempt to load Limits Config Blob */
-	ssize = fmap_read_area(QCLIB_FR_LIMITS_CFG_DATA, _limits_cfg,
-		REGION_SIZE(limits_cfg));
-	if (ssize < 0)
-		return -1;
-	qclib_add_if_table_entry(QCLIB_TE_LIMITS_CFG_DATA,
-				 _limits_cfg, ssize, 0);
-
    return 0;
 }