Change in coreboot[master]: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay

[Arthur Heymans (Code Review)]

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35759 )


Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................

[TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay

There is no need for high precision udelays here.

Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Signed-off-by: Arthur Heymans <[e-mail]>
---
M src/northbridge/intel/pineview/raminit.c
1 file changed, 7 insertions(+), 47 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/35759/1

diff --git a/src/northbridge/intel/pineview/raminit.c b/src/northbridge/intel/pineview/raminit.c
index 7f59d91..db13b2a 100644
--- a/src/northbridge/intel/pineview/raminit.c
+++ b/src/northbridge/intel/pineview/raminit.c
@@ -547,20 +547,6 @@
 	}
 }
 
-#define HPET_BASE 0xfed00000
-#define HPET32(x) *((volatile u32 *)(HPET_BASE + x))
-static void enable_hpet(void)
-{
-	u32 reg32;
-	reg32 = RCBA32(HPTC);
-	reg32 &= ~0x3;
-	reg32 |= (1 << 7);
-	RCBA32(HPTC) = reg32;
-	/* On NM10 this only works if read back */
-	RCBA32(HPTC);
-	HPET32(0x10) = HPET32(0x10) | 1;
-}
-
 static void sdram_clk_crossing(struct sysinfo *s)
 {
 	u8 clk_idx, fsb_idx;
@@ -1525,29 +1511,6 @@
 	pci_write_config32(PCI_DEV(0, 0, 0), 0xac, tsegmb[s->dimm_config[0]]);
 }
 
-#if 1
-static void hpet_udelay(u32 del)
-{
-	u32 start, finish, now;
-
-	del *= 15; /* now in usec */
-
-	start = HPET32(0xf0);
-	finish = start + del;
-	while (1) {
-		now = HPET32(0xf0);
-		if (finish > start) {
-			if (now >= finish)
-				break;
-		} else {
-			if ((now < start) && (now >= finish)) {
-				break;
-			}
-		}
-	}
-}
-#endif
-
 static u8 sdram_checkrcompoverride(void)
 {
 	u32 xcomp;
@@ -1588,7 +1551,7 @@
 	MCHBAR8(0x130) = MCHBAR8(0x130) & ~(1 << 7);
 	for (i = 0; i < 3; i++) {
 		MCHBAR8(0x130) = MCHBAR8(0x130) | 1;
-		hpet_udelay(1000);
+		udelay(1000);
 		while ((MCHBAR8(0x130) & 0x1) != 0);
 		ok |= sdram_checkrcompoverride();
 	}
@@ -1600,7 +1563,7 @@
 		MCHBAR32(0x140) = reg32a;
 	}
 	MCHBAR8(0x130) = MCHBAR8(0x130) | 1;
-	hpet_udelay(1000);
+	udelay(1000);
 	while ((MCHBAR8(0x130) & 0x1) != 0);
 }
 
@@ -1614,7 +1577,7 @@
 	MCHBAR8(0x271) = (MCHBAR8(0x271) & ~0x3e) | jmode;
 	read32((void *)reg32);
 	barrier();
-	hpet_udelay(1); // 1us
+	udelay(1); // 1us
 }
 
 static void sdram_zqcl(struct sysinfo *s)
@@ -1660,7 +1623,7 @@
 		rttnom |= (1 << 6);
 	}
 
-	hpet_udelay(200); // 200us
+	udelay(200); // 200us
 	reg16 = 0;
 	FOR_EACH_POPULATED_RANK(s->dimms, ch, r) {
 		for (i = 0; i < 12; i++) {
@@ -1815,13 +1778,13 @@
 	u8 dqsmatches = 1;
 	while (count--) {
 		MCHBAR8(0x5d8) = MCHBAR8(0x5d8) & ~0x2;
-		hpet_udelay(1);
+		udelay(1);
 		MCHBAR8(0x5d8) = MCHBAR8(0x5d8) | 0x2;
-		hpet_udelay(1);
+		udelay(1);
 		barrier();
 		read32((void *)strobeaddr);
 		barrier();
-		hpet_udelay(1);
+		udelay(1);
 
 		if (((MCHBAR8(dqshighaddr) & 0x40) >> 6) != highlow) {
 			dqsmatches = 0;
@@ -2515,9 +2478,6 @@
 	/* Determine smallest common tRAS, tRP, tRCD, etc */
 	sdram_detect_smallest_params(&si);
 
-	/* Enable HPET */
-	enable_hpet();
-
 	MCHBAR16(0xc1c) = MCHBAR16(0xc1c) | (1 << 15);
 
 	sdram_clk_crossing(&si);

-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 1
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-MessageType: newchange

[Arthur Heymans (Code Review)]

Hello Patrick Rudolph, build bot (Jenkins), Damien Zammit, 

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

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

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

Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................

[TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay

There is no need for high precision udelays here.

Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Signed-off-by: Arthur Heymans <[e-mail]>
---
M src/northbridge/intel/pineview/raminit.c
1 file changed, 7 insertions(+), 47 deletions(-)


  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/35759/2
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-MessageType: newpatchset

[Felix Held (Code Review)]

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35759 )

Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................


Patch Set 2: Code-Review+1

looks good to me, but it should be tested that it doesn't break things


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-Comment-Date: Fri, 18 Oct 2019 18:10:54 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35759 )

Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................


Patch Set 2: Code-Review+1
> Patch Set 2: Code-Review+1
>
> looks good to me, but it should be tested that it doesn't break things

Yes.


-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Comment-Date: Tue, 10 Mar 2020 21:47:23 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

[Angel Pons (Code Review)]

Attention is currently required from: Arthur Heymans.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35759 )

Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................


Patch Set 2:

(1 comment)

Patchset:

PS2: 
Almost a year later, I still haven't tested this. I suspect that regular udelay might be too imprecise for use in some raminit parts.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Arthur Heymans <[e-mail]>
Gerrit-Comment-Date: Tue, 12 Jan 2021 22:34:17 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment

[Arthur Heymans (Code Review)]

Attention is currently required from: Angel Pons.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35759 )

Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................


Patch Set 2:

(1 comment)

Patchset:

PS2:
> Almost a year later, I still haven't tested this. I suspect that regular udelay might be too imprecise for use in some raminit parts.

Me neither (and should before it's acceptable). Al other raminits seems fine with the tsc udelay so it's likely that this works fine.



-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-Attention: Angel Pons <[e-mail]>
Gerrit-Comment-Date: Tue, 12 Jan 2021 22:48:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Angel Pons <[e-mail]>
Gerrit-MessageType: comment

[M] Change in coreboot[master]: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay

[Martin L Roth (Code Review)]

Martin L Roth has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35759?usp=email )

Change subject: [TESTME]nb/intel/pineview: Use regular udelay over hpet_udelay
......................................................................


Abandoned

This patch has not been touched in over 12 months. Anyone who wants to take over work on this patch, please feel free to restore it and do any work needed to get it merged. If you create a new patch based on this work, please credit the original author.
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/35759?usp=email
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I619e2f215f94858e53ac1369b2f6d995303aa3cf
Gerrit-Change-Number: 35759
Gerrit-PatchSet: 2
Gerrit-Owner: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Angel Pons <[e-mail]>
Gerrit-Reviewer: Arthur Heymans <[e-mail]>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Felix Held <[e-mail]>
Gerrit-Reviewer: Patrick Rudolph <[e-mail]>
Gerrit-Reviewer: build bot (Jenkins) <[e-mail]>
Gerrit-CC: Martin L Roth <[e-mail]>
Gerrit-CC: Paul Menzel <[e-mail]>
Gerrit-MessageType: abandon