[coreboot] msrtool CS5536 interrupt and DIVIL LBAR MSRs

Myles Watson mylesgw at gmail.com
Fri Jan 30 18:08:59 CET 2009



> -----Original Message-----
> From: coreboot-bounces at coreboot.org [mailto:coreboot-bounces at coreboot.org]
> On Behalf Of Peter Stuge
> Sent: Friday, January 30, 2009 9:17 AM
> To: coreboot at coreboot.org
> Subject: Re: [coreboot] msrtool CS5536 interrupt and DIVIL LBAR MSRs
> 
> Peter Stuge wrote:
> > Can someone please review these register definitions? Thank you!

+	{ 0x51400008, MSRTYPE_RDWR, MSR2(0, 0), "DIVIL_LBAR_IRQ", "Local BAR
- IRQ Mapper", {
+		{ 15, 8, "BASE_ADDR", "Base Address in I/O Space",
PRESENT_HEX, {
+			{ BITVAL_EOT }
+		}},
+		{ 7, 8, RESERVED },
+		{ BITS_EOT }

This would match the docs better if it were 15,11 BASE_ADDR & 4,5 Reserved.
You could also add a comment saying that because it's a base address (See
page 104) more bits are reserved.  It just took me extra time to check.

+	{ 0x5140000f, MSRTYPE_RDWR, MSR2(0, 0), "DIVIL_LBAR_PMS", "Local BAR
- Power Management Support", {
...
+		{ 15, 8, "BASE_ADDR", "Base Address in I/O Space",
PRESENT_HEX, {
+			{ BITVAL_EOT }
+		}},
+		{ 7, 8, RESERVED },
+		{ BITS_EOT }

Same thing, only this time it would be 15,9 BASE_ADDR & 6,7 Reserved, even
though both places refer to pg 104.  I didn't take enough time to understand
the difference between the two.

+	{ 0x51400026, MSRTYPE_RDONLY, MSR2(0, 0), "PIC_XIRR_STS_LOW", "IRQ
Mapper Extended Interrupt Request Status Low", {
+		{ 63, 32, RESERVED },
+		{ 31, 1, "IG7_STS_Z", "Unrestricted Source Z Input 7",
PRESENT_BIN, {
+			{ MSR1(0), "No interrupt." },
+			{ MSR1(1), "Interrupt status." },
+			{ BITVAL_EOT }

I think "Interrupt status" should be replaced with "Interrupt set" or
something that suggests that there is an interrupt.

Besides that I didn't see anything amiss.

Acked-by: Myles Watson <mylesgw at gmail.com>

Thanks,
Myles






More information about the coreboot mailing list