Skip to contentSkip to conversation

ipmi: Fix ACPI detecting with regspacing

PATCH In review Yinghai Lu 4 participants 9 messages Jul 26, 2010 – Jul 28, 2010 IPMI

lore
Share & tools
lore

Yinghai Lu

author
#0 lore raw

PATCH 1/1 ipmi: Fix ACPI detecting with regspacing

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.

ipmi_si can not be initialized.

[  138.799739] calling  init_ipmi_devintf+0x0/0x109 @ 1
[  138.805050] ipmi device interface
[  138.818131] initcall init_ipmi_devintf+0x0/0x109 returned 0 after 12797 usecs
[  138.822998] calling  init_ipmi_si+0x0/0xa90 @ 1
[  138.840276] IPMI System Interface driver.
[  138.846137] ipmi_si: probing via ACPI
[  138.849225] ipmi_si 00:09: [io  0x0ca2] regsize 1 spacing 1 irq 0
[  138.864438] ipmi_si: Adding ACPI-specified kcs state machine
[  138.870893] ipmi_si: probing via SMBIOS
[  138.880945] ipmi_si: Adding SMBIOS-specified kcs state machineipmi_si: duplicate interface
[  138.896511] ipmi_si: probing via SPMI
[  138.899861] ipmi_si: Adding SPMI-specified kcs state machineipmi_si: duplicate interface
[  138.917095] ipmi_si: Trying ACPI-specified kcs state machine at i/o address 0xca2, slave address 0x0, irq 0
[  138.928658] ipmi_si: Interface detection failed
[  138.953411] initcall init_ipmi_si+0x0/0xa90 returned 0 after 110847 usecs

in smbios has
DMI/SMBIOS
Handle 0x00C5, DMI type 38, 18 bytes
IPMI Device Information
Interface Type: KCS (Keyboard Control Style) Specification Version: 2.0
I2C Slave Address: 0x00
NV Storage Device: Not Present
Base Address: 0x0000000000000CA2 (I/O) Register Spacing: 32-bit Boundaries
in DSDT has

                    Device (BMC)
                    {

                        Name (_HID, EisaId ("IPI0001"))
                        Method (_STA, 0, NotSerialized)
                        {
                            If (LEqual (OSN, Zero))
                            {
                                Return (Zero)
                            }

                            Return (0x0F)
                        }

                        Name (_STR, Unicode ("IPMI_KCS"))
                        Name (_UID, Zero)
                        Name (_CRS, ResourceTemplate ()
                        {
                            IO (Decode16,
                                0x0CA2,             // Range Minimum
                                0x0CA2,             // Range Maximum
                                0x00,               // Alignment
                                0x01,               // Length
                                )
                            IO (Decode16,
                                0x0CA6,             // Range Minimum
                                0x0CA6,             // Range Maximum
                                0x00,               // Alignment
                                0x01,               // Length
                                )
                        })
                        Method (_IFT, 0, NotSerialized)
                        {
                            Return (One)
                        }

                        Method (_SRV, 0, NotSerialized)
                        {
                            Return (0x0200)
                        }
                    }

so the reg spacing should be 4 instead of 1.

Try to calculate regspacing for this kind of system.

---
 drivers/char/ipmi/ipmi_si_intf.c |    8 ++++++++
 1 file changed, 8 insertions(+)
drivers/char/ipmi/ipmi_si_intf.c +8 -0
Index: linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
===================================================================
@@ -2176,6 +2176,14 @@ static int __devinit ipmi_pnp_probe(stru
 	info->io.addr_data = res->start;
 
 	info->io.regspacing = DEFAULT_REGSPACING;
+	res = pnp_get_resource(dev,
+			       (info->io.addr_type == IPMI_IO_ADDR_SPACE) ?
+					IORESOURCE_IO : IORESOURCE_MEM,
+			       1);
+	if (res) {
+		if (res->start > info->io.addr_data)
+			info->io.regspacing = res->start - info->io.addr_data;
+	}
 	info->io.regsize = DEFAULT_REGSPACING;
 	info->io.regshift = 0;
 
Acked-by on v1 1/1
#1 lore raw
Quoted 83 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.

ipmi_si can not be initialized.

[  138.799739] calling  init_ipmi_devintf+0x0/0x109 @ 1
[  138.805050] ipmi device interface
[  138.818131] initcall init_ipmi_devintf+0x0/0x109 returned 0 after 12797 usecs
[  138.822998] calling  init_ipmi_si+0x0/0xa90 @ 1
[  138.840276] IPMI System Interface driver.
[  138.846137] ipmi_si: probing via ACPI
[  138.849225] ipmi_si 00:09: [io  0x0ca2] regsize 1 spacing 1 irq 0
[  138.864438] ipmi_si: Adding ACPI-specified kcs state machine
[  138.870893] ipmi_si: probing via SMBIOS
[  138.880945] ipmi_si: Adding SMBIOS-specified kcs state machineipmi_si: duplicate interface
[  138.896511] ipmi_si: probing via SPMI
[  138.899861] ipmi_si: Adding SPMI-specified kcs state machineipmi_si: duplicate interface
[  138.917095] ipmi_si: Trying ACPI-specified kcs state machine at i/o address 0xca2, slave address 0x0, irq 0
[  138.928658] ipmi_si: Interface detection failed
[  138.953411] initcall init_ipmi_si+0x0/0xa90 returned 0 after 110847 usecs

in smbios has
DMI/SMBIOS
Handle 0x00C5, DMI type 38, 18 bytes
IPMI Device Information
Interface Type: KCS (Keyboard Control Style) Specification Version: 2.0
I2C Slave Address: 0x00
NV Storage Device: Not Present
Base Address: 0x0000000000000CA2 (I/O) Register Spacing: 32-bit Boundaries
in DSDT has

                    Device (BMC)
                    {

                        Name (_HID, EisaId ("IPI0001"))
                        Method (_STA, 0, NotSerialized)
                        {
                            If (LEqual (OSN, Zero))
                            {
                                Return (Zero)
                            }

                            Return (0x0F)
                        }

                        Name (_STR, Unicode ("IPMI_KCS"))
                        Name (_UID, Zero)
                        Name (_CRS, ResourceTemplate ()
                        {
                            IO (Decode16,
                                0x0CA2,             // Range Minimum
                                0x0CA2,             // Range Maximum
                                0x00,               // Alignment
                                0x01,               // Length
                                )
                            IO (Decode16,
                                0x0CA6,             // Range Minimum
                                0x0CA6,             // Range Maximum
                                0x00,               // Alignment
                                0x01,               // Length
                                )
                        })
                        Method (_IFT, 0, NotSerialized)
                        {
                            Return (One)
                        }

                        Method (_SRV, 0, NotSerialized)
                        {
                            Return (0x0200)
                        }
                    }
Quoted 5 lines

so the reg spacing should be 4 instead of 1.

Try to calculate regspacing for this kind of system.

Signature · 1 line
Matthew Garrett | [email protected]
#2 lore raw
In reply to Yinghai Lu
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Bjorn

Quoted 103 lines
[  138.799739] calling  init_ipmi_devintf+0x0/0x109 @ 1
[  138.805050] ipmi device interface
[  138.818131] initcall init_ipmi_devintf+0x0/0x109 returned 0 after 12797 usecs
[  138.822998] calling  init_ipmi_si+0x0/0xa90 @ 1
[  138.840276] IPMI System Interface driver.
[  138.846137] ipmi_si: probing via ACPI
[  138.849225] ipmi_si 00:09: [io  0x0ca2] regsize 1 spacing 1 irq 0
[  138.864438] ipmi_si: Adding ACPI-specified kcs state machine
[  138.870893] ipmi_si: probing via SMBIOS
[  138.880945] ipmi_si: Adding SMBIOS-specified kcs state machineipmi_si: duplicate interface
[  138.896511] ipmi_si: probing via SPMI
[  138.899861] ipmi_si: Adding SPMI-specified kcs state machineipmi_si: duplicate interface
[  138.917095] ipmi_si: Trying ACPI-specified kcs state machine at i/o address 0xca2, slave address 0x0, irq 0
[  138.928658] ipmi_si: Interface detection failed
[  138.953411] initcall init_ipmi_si+0x0/0xa90 returned 0 after 110847 usecs

in smbios has
DMI/SMBIOS
Handle 0x00C5, DMI type 38, 18 bytes
IPMI Device Information
Interface Type: KCS (Keyboard Control Style) Specification Version: 2.0
I2C Slave Address: 0x00
NV Storage Device: Not Present
Base Address: 0x0000000000000CA2 (I/O) Register Spacing: 32-bit Boundaries
in DSDT has

                    Device (BMC)
                    {

                        Name (_HID, EisaId ("IPI0001"))
                        Method (_STA, 0, NotSerialized)
                        {
                            If (LEqual (OSN, Zero))
                            {
                                Return (Zero)
                            }

                            Return (0x0F)
                        }

                        Name (_STR, Unicode ("IPMI_KCS"))
                        Name (_UID, Zero)
                        Name (_CRS, ResourceTemplate ()
                        {
                            IO (Decode16,
                                0x0CA2,             // Range Minimum
                                0x0CA2,             // Range Maximum
                                0x00,               // Alignment
                                0x01,               // Length
                                )
                            IO (Decode16,
                                0x0CA6,             // Range Minimum
                                0x0CA6,             // Range Maximum
                                0x00,               // Alignment
                                0x01,               // Length
                                )
                        })
                        Method (_IFT, 0, NotSerialized)
                        {
                            Return (One)
                        }

                        Method (_SRV, 0, NotSerialized)
                        {
                            Return (0x0200)
                        }
                    }

so the reg spacing should be 4 instead of 1.

Try to calculate regspacing for this kind of system.

---
 drivers/char/ipmi/ipmi_si_intf.c |    8 ++++++++
 1 file changed, 8 insertions(+)

Index: linux-2.6/drivers/char/ipmi/ipmi_si_intf.c
===================================================================

drivers/char/ipmi/ipmi_si_intf.c +8 -1
@@ -2176,6 +2176,14 @@ static int __devinit ipmi_pnp_probe(stru
 	info->io.addr_data = res->start;
 
 	info->io.regspacing = DEFAULT_REGSPACING;
+	res = pnp_get_resource(dev,
+			       (info->io.addr_type == IPMI_IO_ADDR_SPACE) ?
+					IORESOURCE_IO : IORESOURCE_MEM,
+			       1);
+	if (res) {
+		if (res->start > info->io.addr_data)
+			info->io.regspacing = res->start - info->io.addr_data;
+	}
 	info->io.regsize = DEFAULT_REGSPACING;
 	info->io.regshift = 0;
 
--

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

author
#3 lore raw
Quoted 21 lines · Bjorn Helgaas
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

Quoted 4 lines

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Sun Fire X4800.

Yes. other OSes work and pass certification.

Thanks

Yinghai

#4 lore raw
Quoted 22 lines · Yinghai Lu
Quoted 21 lines · Bjorn Helgaas
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

Quoted 8 lines
Quoted 4 lines

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Sun Fire X4800.

Yes. other OSes work and pass certification.

Please put this information in the changelog (if that wasn't obvious).

Bjorn

author
#5 lore raw
Quoted 28 lines · Bjorn Helgaas
Quoted 27 lines · Yinghai Lu
Quoted 25 lines · Bjorn Helgaas
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Quoted 6 lines
Quoted 3 lines

Sun Fire X4800.

Yes. other OSes work and pass certification.

Please put this information in the changelog (if that wasn't obvious).

I don't want to mention other os.

Yinghai

#6 lore raw
Quoted 29 lines · Yinghai Lu
Quoted 28 lines · Bjorn Helgaas
Quoted 27 lines · Yinghai Lu
Quoted 25 lines · Bjorn Helgaas
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines

Sun Fire X4800.

Yes. other OSes work and pass certification.

Please put this information in the changelog (if that wasn't obvious).

I don't want to mention other os.

Sigh. Why is this so difficult?

It's very useful to know which other OSes work and *how* they work. If another OS (particularly a popular one like Windows) works, and we know it's using the ACPI namespace, that tells us we need a change like yours. If another OS works but is using SPMI, that tells us nothing about whether we should make this change.

If no other OS uses the namespace, then we might be adding a Linux change just to deal with a firmware defect on this machine, and that sort of change could easily break other machines.

I don't think that's the case here; I think we *should* make the change you're proposing. But it's just a good practice to be diligent about documenting the reasons for the changes we make.

Bjorn

#7 lore raw
Quoted 32 lines · On Tue, 27 Jul 2010 11:41:10 -0600 Bjorn Helgaas <[email protected]> wrote:
Quoted 31 lines · Yinghai Lu
Quoted 30 lines · Bjorn Helgaas
Quoted 29 lines · Yinghai Lu
Quoted 21 lines · Bjorn Helgaas
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

Quoted 4 lines

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Sun Fire X4800.

Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

Yes. other OSes work and pass certification.

Please put this information in the changelog (if that wasn't obvious).

I don't want to mention other os.

Sigh. Why is this so difficult?

It wasn't for me ;)

Do we have acks for 2.6.36 and a -stable backport on this patch?

author
#8 lore raw
Quoted 39 lines · Andrew Morton
Quoted 36 lines · On Tue, 27 Jul 2010 11:41:10 -0600 Bjorn Helgaas <[email protected]> wrote:
Quoted 35 lines · Yinghai Lu
Quoted 33 lines · Bjorn Helgaas
Quoted 22 lines · Yinghai Lu
Quoted 21 lines · Bjorn Helgaas
Quoted 13 lines · Yinghai Lu

after the commint that change ipmi_si detecting sequence from SMBIOS/ACPI to ACPI/SMBIOS,

| commit 754d453185275951d39792865927ec494fa1ebd8
| Author: Matthew Garrett <[email protected]>
| Date:   Wed May 26 14:43:47 2010 -0700
|
|    ipmi: change device discovery order
|
|    The ipmi spec provides an ordering for si discovery.  Change the driver to
|    match, with the exception of preferring smbios to SPMI as HPs (at least)
|    contain accurate information in the former but not the latter.
Quoted 1 line

ipmi_si can not be initialized.

I think this patch makes sense, as long as this is not a bringup issue that only affects pre-release firmware. If this only affects a prototype, it would be better to change the firmware so it conforms to the conventional _CRS usage of systems in the field.

Quoted 8 lines
Quoted 4 lines

To that end, I'd like to know what system this is, and whether IPMI works under Windows on this system. We know that Windows doesn't look at SMBIOS or SPMI, so if Windows works, it must be doing something similar to your patch.

Sun Fire X4800.

Yes. other OSes work and pass certification.

Please put this information in the changelog (if that wasn't obvious).

Quoted 7 lines
Quoted 3 lines
Quoted 1 line

I don't want to mention other os.

Sigh. Why is this so difficult?

It wasn't for me ;)

Do we have acks for 2.6.36 and a -stable backport on this patch?

Matthew Garrett <[email protected]>
gave ack to it.

looks like Matthew's patch only get into 2.6.35.

So we need it for 2.6.35 and later.

Thanks

Yinghai