Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
nb/intel/gm45: Introduce memmap.h
Move all memory map definitions into a separate header.
Tested with BUILD_TIMELESS=1, Roda RK9 remains identical.
Change-Id: Idddb63069b7a0b7b4d6c7850473a71318748bb9b Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/gm45/gm45.h A src/northbridge/intel/gm45/memmap.h 2 files changed, 17 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/45451/1
diff --git a/src/northbridge/intel/gm45/gm45.h b/src/northbridge/intel/gm45/gm45.h index 061bf2d..cad0596 100644 --- a/src/northbridge/intel/gm45/gm45.h +++ b/src/northbridge/intel/gm45/gm45.h @@ -170,16 +170,7 @@ #define CMOS_READ_TRAINING 0x80 /* 16 bytes */ #define CMOS_WRITE_TRAINING 0x90 /* 16 bytes (could be reduced to 10 bytes) */
- -#define DEFAULT_MCHBAR 0xfed14000 -#define DEFAULT_DMIBAR 0xfed18000 -#define DEFAULT_EPBAR 0xfed19000 - - -#define IOMMU_BASE1 0xfed90000 -#define IOMMU_BASE2 0xfed91000 -#define IOMMU_BASE3 0xfed92000 -#define IOMMU_BASE4 0xfed93000 +#include "memmap.h"
/* * D0:F0 diff --git a/src/northbridge/intel/gm45/memmap.h b/src/northbridge/intel/gm45/memmap.h new file mode 100644 index 0000000..b778f2d --- /dev/null +++ b/src/northbridge/intel/gm45/memmap.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __NORTHBRIDGE_INTEL_GM45_MEMMAP_H__ +#define __NORTHBRIDGE_INTEL_GM45_MEMMAP_H__ + +#define DEFAULT_MCHBAR 0xfed14000 +#define DEFAULT_DMIBAR 0xfed18000 +#define DEFAULT_EPBAR 0xfed19000 + + +#define IOMMU_BASE1 0xfed90000 +#define IOMMU_BASE2 0xfed91000 +#define IOMMU_BASE3 0xfed92000 +#define IOMMU_BASE4 0xfed93000 + +#endif /* __NORTHBRIDGE_INTEL_GM45_MEMMAP_H__ */
Hello build bot (Jenkins), Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45451
to look at the new patch set (#2).
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
nb/intel/gm45: Introduce memmap.h
Move all memory map definitions into a separate header.
Tested with BUILD_TIMELESS=1, Roda RK9 remains identical.
Change-Id: Idddb63069b7a0b7b4d6c7850473a71318748bb9b Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/gm45/gm45.h A src/northbridge/intel/gm45/memmap.h 2 files changed, 17 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/45451/2
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 3:
(1 comment)
Would you mind not doing no-op in this code while it's being rewritten anyway ( CB:45610 )?
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/gm45.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 173: #define DEFAULT_MCHBAR 0xfed14000 : #define DEFAULT_DMIBAR 0xfed18000 : #define DEFAULT_EPBAR 0xfed19000 please see https://review.coreboot.org/c/coreboot/+/45610/17 Thx
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/gm45.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 173: #define DEFAULT_MCHBAR 0xfed14000 : #define DEFAULT_DMIBAR 0xfed18000 : #define DEFAULT_EPBAR 0xfed19000
please see https://review.coreboot.org/c/coreboot/+/45610/17 […]
You have every right to opt out of my reviews,
but removing your self from CB: 31196 CB: 46282 CB: 46283 CB: 45574 CB: 46279 CB: 45781 CB: 46122 after I politely asked you not to make any changes here because of CB:45610, it's childish ...
so let move current comment go to resolved status (even though I didn't give any score) and avoid possible drama.
Angel Pons has removed HAOUAS Elyes from this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Removed reviewer HAOUAS Elyes.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 3: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 10: Why the additional empty line?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 10:
Why the additional empty line?
It was there when the first patchset was made, and later patchsets didn't change this file
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 10:
It was there when the first patchset was made, and later patchsets didn't change this file
That's more a how than why, isn't it? ;)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 10:
That's more a how than why, isn't it? ;)
It answers why, and also explains that this line wasn't always `additional`. If you want me to remove it, just say so next time.
Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45451
to look at the new patch set (#4).
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
nb/intel/gm45: Introduce memmap.h
Move all memory map definitions into a separate header.
Tested with BUILD_TIMELESS=1, Roda RK9 remains identical.
Change-Id: Idddb63069b7a0b7b4d6c7850473a71318748bb9b Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/northbridge/intel/gm45/gm45.h A src/northbridge/intel/gm45/memmap.h 2 files changed, 16 insertions(+), 8 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/45451/4
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 10:
It answers why, and also explains that this line wasn't always `additional`. […]
I guess I just want to talk to people about such things in case they are intentional. Don't remember if I wanted the line removed. If so, I'd have left the comment unresolved, but I can't recall if I did... Generally, multiple empty lines in a row are frowned upon in coreboot, but I don't like to tell people to remove them without having talked about it *shrug*
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... File src/northbridge/intel/gm45/memmap.h:
https://review.coreboot.org/c/coreboot/+/45451/3/src/northbridge/intel/gm45/... PS3, Line 10:
I guess I just want to talk to people about such things in case they […]
I felt it was quicker to just do it than to think through a reply. It's no big deal, though.
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45451 )
Change subject: nb/intel/gm45: Introduce memmap.h ......................................................................
nb/intel/gm45: Introduce memmap.h
Move all memory map definitions into a separate header.
Tested with BUILD_TIMELESS=1, Roda RK9 remains identical.
Change-Id: Idddb63069b7a0b7b4d6c7850473a71318748bb9b Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45451 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Nico Huber nico.h@gmx.de --- M src/northbridge/intel/gm45/gm45.h A src/northbridge/intel/gm45/memmap.h 2 files changed, 16 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved
diff --git a/src/northbridge/intel/gm45/gm45.h b/src/northbridge/intel/gm45/gm45.h index 0fec20c..8be1873 100644 --- a/src/northbridge/intel/gm45/gm45.h +++ b/src/northbridge/intel/gm45/gm45.h @@ -170,14 +170,7 @@ #define CMOS_READ_TRAINING 0x80 /* 16 bytes */ #define CMOS_WRITE_TRAINING 0x90 /* 16 bytes (could be reduced to 10 bytes) */
-#define DEFAULT_MCHBAR 0xfed14000 -#define DEFAULT_DMIBAR 0xfed18000 -#define DEFAULT_EPBAR 0xfed19000 - -#define IOMMU_BASE1 0xfed90000 -#define IOMMU_BASE2 0xfed91000 -#define IOMMU_BASE3 0xfed92000 -#define IOMMU_BASE4 0xfed93000 +#include "memmap.h"
/* * D0:F0 diff --git a/src/northbridge/intel/gm45/memmap.h b/src/northbridge/intel/gm45/memmap.h new file mode 100644 index 0000000..c0706d0 --- /dev/null +++ b/src/northbridge/intel/gm45/memmap.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __NORTHBRIDGE_INTEL_GM45_MEMMAP_H__ +#define __NORTHBRIDGE_INTEL_GM45_MEMMAP_H__ + +#define DEFAULT_MCHBAR 0xfed14000 +#define DEFAULT_DMIBAR 0xfed18000 +#define DEFAULT_EPBAR 0xfed19000 + +#define IOMMU_BASE1 0xfed90000 +#define IOMMU_BASE2 0xfed91000 +#define IOMMU_BASE3 0xfed92000 +#define IOMMU_BASE4 0xfed93000 + +#endif /* __NORTHBRIDGE_INTEL_GM45_MEMMAP_H__ */
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Nico Huber (Code Review)