[coreboot-gerrit] Change in coreboot[master]: soc/intel/common/block: Add Intel common systemagent support

[Subrata Banik (Code Review)]

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

Change subject: soc/intel/common/block: Add Intel common systemagent support
......................................................................


Patch Set 8:

(2 comments)

https://review.coreboot.org/#/c/19668/8/src/soc/intel/common/block/include/intelblocks/systemagent.h
File src/soc/intel/common/block/include/intelblocks/systemagent.h:

PS8, Line 25: #define MCHBAR			0x48
            : #define GGC			0x50
            : #define PCIEXBAR		0x60
            : #define  PCIEXBAR_LENGTH_64MB	2
            : #define  PCIEXBAR_LENGTH_128MB	1
            : #define  PCIEXBAR_LENGTH_256MB	0
            : #define  PCIEXBAR_PCIEXBAREN		(1 << 0)
            : 
            : #define PAM0	0x80
            : #define PAM1	0x81
            : #define PAM2	0x82
            : #define PAM3	0x83
            : #define PAM4	0x84
            : #define PAM5	0x85
            : #define PAM6	0x86
            : 
            : #define TOUUD	0xa8    /* Top of Upper Usable DRAM */
            : #define BDSM	0xb0    /* Base Data Stolen Memory */
            : #define BGSM	0xb4    /* Base GTT Stolen Memory */
            : #define TSEG	0xb8    /* TSEG base */
            : #define TOLUD	0xbc    /* Top of Low Used Memory */
            : 
            : /* MCHBAR */
            : #define MCHBAR8(x)	(*(volatile u8 *)(MCH_BASE_ADDRESS + x))
            : #define MCHBAR16(x)	(*(volatile u16 *)(MCH_BASE_ADDRESS + x))
> Is all of this necessary to be exposed to the user of this module?
i guess we can move this into sa-def.h which is local to SA code.


https://review.coreboot.org/#/c/19668/8/src/soc/intel/common/block/systemagent/systemagent.c
File src/soc/intel/common/block/systemagent/systemagent.c:

Line 238: 	return size_k;
> I'm trying to understand why all this code was changed from apl. that code 
If your question specific about IMR implementation, why i havn;t copied entirely from APL IMR API. I could have done that, if you need, i will do this.

But if your question is why entire SA not copied from APL, then answer is APL SA code don't provide me any options to use soc like override concept which SKL code provides. if you see fixed resource allocation table. In APL its done inside same function but SKL use a structure although static but we could make it extern to solve soc override concept as CNL might have diffent entries for the same. And rest DRAM and IMR, i was trying to match as close as APL because no inputs required from SOC.


-- 
To view, visit https://review.coreboot.org/19668
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I969ff187e3d4199864cb2e9c9a13f4d04158e27c
Gerrit-PatchSet: 8
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: V Sowmya <v.sowmya at intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Balaji Manigandan <balaji.manigandan at intel.com>
Gerrit-Reviewer: Barnali Sarkar <barnali.sarkar at intel.com>
Gerrit-Reviewer: Furquan Shaikh <furquan at google.com>
Gerrit-Reviewer: Hannah Williams <hannah.williams at intel.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Subrata Banik <subrata.banik at intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-Reviewer: dhaval v sharma <dhaval.v.sharma at intel.com>
Gerrit-HasComments: Yes