Skip to contentSkip to conversation

ipmi: remove SPMI and update core driver with dev_printk

PATCH Superseded Myron Stowe 6 participants 22 messages Mar 4, 2010 – Mar 11, 2010

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

cover ipmi: remove SPMI and update core driver with dev_printk

These patches remove the SPMI based IPMI device discovery mechanism and update the driver's core to use dev_printk() and its constructs.

As part of this patch series I wanted to remove the 'PFX' argument from ipmi_of_probe()'s dev_printk constructs as I believe it produces redundant output but I do not have a PPC platform to test against.

Ultimately, I would like to see if it is possible to also remove IPMI's SMBIOS based device discovery mechanism.

---

Myron Stowe (4):
ipmi: Raise precedence of PNP based discovery mechanisms (ACPI, PCI)
ipmi: Remove SPMI table based device discovery method
ipmi: Convert tracking of the ACPI device pointer to a PNP device
ipmi: Update driver to use 'dev_printk()' and its constructs

 drivers/char/ipmi/ipmi_bt_sm.c      |    2 
 drivers/char/ipmi/ipmi_msghandler.c |   11 +
 drivers/char/ipmi/ipmi_si_intf.c    |  277 +++++++----------------------------
 3 files changed, 60 insertions(+), 230 deletions(-)
#1 lore raw

PATCH 1/4 ipmi: Raise precedence of PNP based discovery mechanisms (ACPI, PCI)

This raises the precedence of PNP discovery mechanisms, ACPI and PCI, above SMBIOS and SPMI as indicated by Appendix C1 of the IPMI specification.

Reference: IPMI - Intelligent Platform Management Interface Specification

           v2.0, Document Revision 1.0, 05.05.2005 Markup

Appendix C1 - Locating IPMI System Interfaces via SMBIOS Tables
"Note that the settings that this structure reports may be over- ridden by Plug-and-Play' reassignment by the OS. Therefore, this structure should be used only when the interface cannot be discovered via Plug-and-Play' discovery mechanisms incorporated in interfaces such as PCI and ACPI."

---

 drivers/char/ipmi/ipmi_si_intf.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +8 -7
index 176f175..86a7dd7 100644
@@ -3197,13 +3197,6 @@ static __devinit int init_ipmi_si(void)
 
 	hardcode_find_bmc();
 
-#ifdef CONFIG_DMI
-	dmi_find_bmc();
-#endif
-
-#ifdef CONFIG_ACPI
-	spmi_find_bmc();
-#endif
 #ifdef CONFIG_ACPI
 	pnp_register_driver(&ipmi_pnp_driver);
 #endif
@@ -3220,6 +3213,14 @@ static __devinit int init_ipmi_si(void)
 	of_register_platform_driver(&ipmi_of_platform_driver);
 #endif
 
+#ifdef CONFIG_DMI
+	dmi_find_bmc();
+#endif
+
+#ifdef CONFIG_ACPI
+	spmi_find_bmc();
+#endif
+
 	if (si_trydefaults) {
 		mutex_lock(&smi_infos_lock);
 		if (list_empty(&smi_infos)) {
#2 lore raw

PATCH 2/4 ipmi: Remove SPMI table based device discovery method

In reply to Myron Stowe

Remove optional ACPI static Service Processor Management Interface (SPMI) table based IPMI device discovery mechanism.

The SPMI table is available for use as an early boot process device description mechanism (i.e. prior to the availability of the OS to utilize ACPI, namespace based [IPI001], control methods) however, ‘Plug-and-Play’ enumeration mechanisms such as PCI and ACPI should be used when possible [1].

Windows relies solely on ACPI/IPI0001 or PCI device based enumeration for IPMI. HP added the SPMI table for HP-UX to use before ACPI namespace is available but Linux doesn't need such as its driver's discovery processing takes place after both ACPI and PCI subsystems have been set up.

[1] From Appendix C1 of the afore mentioned IPMI specification - "... this structure (referring to SPMI) should be used only when the interface cannot be discovered via 'Plug-and-Play' discovery mechanisms incorporated in interfaces such as PCI and ACPI."

---

 drivers/char/ipmi/ipmi_bt_sm.c   |    2 
 drivers/char/ipmi/ipmi_si_intf.c |  165 --------------------------------------
 2 files changed, 2 insertions(+), 165 deletions(-)
drivers/char/ipmi/ipmi_bt_sm.c +1 -1
index 7b98c06..d0e868e 100644
@@ -204,7 +204,7 @@ static unsigned int bt_init_data(struct si_sm_data *bt, struct si_sm_io *io)
 	bt->BT_CAP_req2rsp = BT_NORMAL_TIMEOUT * 1000000;
 	bt->BT_CAP_retries = BT_NORMAL_RETRY_LIMIT;
 	/* BT_CAP_outreqs == zero is a flag to read BT Capabilities */
-	return 3; /* We claim 3 bytes of space; ought to check SPMI table */
+	return 3; /* We claim 3 bytes of space */
 }
 
 /* Jam a completion code (probably an error) into a response */
drivers/char/ipmi/ipmi_si_intf.c +1 -164

189 lines not on this page · raw message

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

#3 lore raw

RE: [Openipmi-developer] [PATCH 2/4] ipmi: Remove SPMI table based device discovery method

Quoted 2 lines · Myron Stowe

Remove optional ACPI static Service Processor Management Interface (SPMI) table based IPMI device discovery mechanism.

Myron,

The other patches look reasonable. This one, I'm not so sure.

My concern is based on these thoughts:

During early hardware bring-up, both ACPI and IPMI subsystems of a new system design tend to be a total mess. The static table parts of ACPI are likely to be usable much earlier than the parts that require a fully functional ACPI subsystem.

For the people developing and testing the new system's IPMI subsystem, losing the SPMI discovery method could be a significant pain point.

A new IPMI subsystem may also be "bolted on" to an existing system for early development before the new system is mature enough to even power on. In such a situation it may be easier to supply a fake table than fake live ACPI information. The hardware at that point may not be in PCI space, yet may also not be at static addresses that would be easy to supply as module load parameters.

I would be more comfortable if you kept this code, perhaps suppressed under a "CONFIG_IPMI_SPMI" config option.

Quoted 1 line

Bela<

#4 lore raw

RE: [Openipmi-developer] [PATCH 2/4] ipmi: Remove SPMI table based device discovery method

Quoted 7 lines · Bela Lubkin
Quoted 2 lines · Myron Stowe

Remove optional ACPI static Service Processor Management Interface (SPMI) table based IPMI device discovery mechanism.

Myron,

Quoted 8 lines

The other patches look reasonable. This one, I'm not so sure.

My concern is based on these thoughts:

During early hardware bring-up, both ACPI and IPMI subsystems of a new system design tend to be a total mess. The static table parts of ACPI are likely to be usable much earlier than the parts that require a fully functional ACPI subsystem.

Hi Bela:

ACPI in new systems does tend to present a lot of challenges. My job is new platform enablement focused which more often than not translates to "helping discover and fix BIOS/ACPI issues" so I definitely understand.

You also astutely point out that ACPI's static tables are available for use by an OS - or OSPM in ACPI speak - much earlier than its namespace. This is true but it is also our belief that no one uses it except HP.

Quoted 5 lines

For the people developing and testing the new system's IPMI subsystem, losing the SPMI discovery method could be a significant pain point.

Quoted 7 lines

A new IPMI subsystem may also be "bolted on" to an existing system for early development before the new system is mature enough to even power on. In such a situation it may be easier to supply a fake table than fake live ACPI information. The hardware at that point may not be in PCI space, yet may also not be at static addresses that would be easy to supply as module load parameters.

The point that in such a situation providing static addresses as module load parameters is not a tenable approach seems to be a stretch but that is tangential to the main discussion so lets not muddy the discussion with such unless you feel there is more to say.

I see how one's life could be easier relying on the BIOS setting up static tables such as either the SMBIOS or SPMI. That way the BIOS is taking the dynamic aspects into account relieving the OS, or OS/platform bring up engineer, from such. In such a case, what benefits does the SPMI table provide as opposed to the SMBIOS table?

Quoted 3 lines

I would be more comfortable if you kept this code, perhaps suppressed under a "CONFIG_IPMI_SPMI" config option.

I just don't see enough remaining value in keeping the SPMI table capability. It seems to be redundant, and if truly so, just adds unnecessary code, complexity, and maintenance aspects to the driver. We typically don't keep code in the kernel just for bring up efforts.

Thanks very much for expressing your concerns here. I fully expected, and still expect, the SPMI removal patch to be the contentious component of this series. Fostering a healthy discussion in these regards should eventually help us understand that removing such is in fact tenable or, that there really are valid technical reasons why it is not.

Myron

Quoted 2 lines
Quoted 2 lines

Bela<

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#5 lore raw

RE: [Openipmi-developer] [PATCH 2/4] ipmi: Remove SPMI table based device discovery method

Quoted 7 lines · Myron Stowe

ACPI in new systems does tend to present a lot of challenges.
My job is
new platform enablement focused which more often than not translates to
"helping discover and fix BIOS/ACPI issues" so I definitely understand.

Quoted 5 lines

You also astutely point out that ACPI's static tables are available for
use by an OS - or OSPM in ACPI speak - much earlier than its namespace.
This is true but it is also our belief that no one uses it except HP.

Are you watching what goes on in embedded systems or only the PC / server marketplace?

Quoted 6 lines

I see how one's life could be easier relying on the BIOS setting up static tables such as either the SMBIOS or SPMI. That way the BIOS is taking the dynamic aspects into account relieving the OS, or OS/platform
bring up engineer, from such. In such a case, what benefits does the SPMI table provide as opposed to the SMBIOS table?

It gives you twice as many chances to find a sufficiently non-buggy table to get on with your work ;-}

Quoted 8 lines
Quoted 2 lines

I would be more comfortable if you kept this code, perhaps suppressed under a "CONFIG_IPMI_SPMI" config option.

I just don't see enough remaining value in keeping the SPMI table capability. It seems to be redundant, and if truly so, just adds unnecessary code, complexity, and maintenance aspects to the driver. We
typically don't keep code in the kernel just for bring up efforts.

Well, I'm not doing any such bring-up work so I will step out of the discussion here. But I'll leave you with a bit of a curse: if you remove this code, I predict that the person it will come back to bite is you...

Quoted 1 line

Bela<

#6 lore raw

Re: [PATCH 2/4] ipmi: Remove SPMI table based device discovery method

Quoted 2 lines · Bela Lubkin

It gives you twice as many chances to find a sufficiently non-buggy table to get on with your work ;-}

I think this is the wrong way to look at it. I don't want a "let's try a bunch of things, maybe one will work" plan. In my opinion, it's better to have a single strategy that everybody uses because that one way becomes well-tested and well-understood.

Windows only looks at ACPI namespace and PCI, and the obvious consequence is that those mechanisms are tested and tend to work.

Linux uses SPMI and SMBIOS and (until recently) ignored the ACPI namespace. SPMI and SMBIOS are redundant copies of what's in the namespace, and that redundancy makes inconsistencies more likely.

The result is that (a) we waste the BIOS engineers' time because they have to implement multiple descriptions, (b) we waste our time resolving inconsistencies between the descriptions, and (c) we delay finding and fixing bugs in the canonical ACPI namespace descriptions.

Bjorn

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

#7 lore raw

RE: [Openipmi-developer] [PATCH 2/4] ipmi: Remove SPMI table based device discovery method

In reply to Bela Lubkin
Quoted 9 lines · Bela Lubkin
Quoted 7 lines · Myron Stowe

ACPI in new systems does tend to present a lot of challenges.
My job is
new platform enablement focused which more often than not translates to
"helping discover and fix BIOS/ACPI issues" so I definitely understand.

Quoted 8 lines
Quoted 5 lines

You also astutely point out that ACPI's static tables are available for
use by an OS - or OSPM in ACPI speak - much earlier than its namespace.
This is true but it is also our belief that no one uses it except HP.

Are you watching what goes on in embedded systems or only the PC / server marketplace?

No, I do not watch the embedded system space. Would such folks tend to track [email protected] (at least with respect to IPMI)? If not, are you aware of a specific mail reflector that I should be including in this discussion?

Quoted 20 lines
Quoted 7 lines

I see how one's life could be easier relying on the BIOS setting up static tables such as either the SMBIOS or SPMI. That way the BIOS is taking the dynamic aspects into account relieving the OS, or OS/platform
bring up engineer, from such. In such a case, what benefits does the SPMI table provide as opposed to the SMBIOS table?

It gives you twice as many chances to find a sufficiently non-buggy table to get on with your work ;-}

Quoted 8 lines
Quoted 2 lines

I would be more comfortable if you kept this code, perhaps suppressed under a "CONFIG_IPMI_SPMI" config option.

I just don't see enough remaining value in keeping the SPMI table capability. It seems to be redundant, and if truly so, just adds unnecessary code, complexity, and maintenance aspects to the driver. We
typically don't keep code in the kernel just for bring up efforts.

Quoted 5 lines

Well, I'm not doing any such bring-up work so I will step out of the discussion here. But I'll leave you with a bit of a curse: if you remove this code, I predict that the person it will come back to bite is you...

I hope I'm not coming off as closed minded and causing you to disengage as most every concern you have raised has been valid and needful of consideration. As mean as putting a jinx on me was (I'm mostly kidding there), you are correct there too: it will be me that it comes back to bite. I still feel that this is worth pursuing as no progress is ever made without some risk but I do want discussions to occur so that I can learn, preferably beforehand, if I am wrong.

Myron

Quoted 2 lines
Quoted 2 lines

Bela<

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#8 lore raw

PATCH 3/4 ipmi: Convert tracking of the ACPI device pointer to a PNP device

In reply to Myron Stowe

Convert PNP patch (git 9e368fa011d4e0aa050db348d69514900520e40b) to maintain a pointer to a PNP device, 'pnp_dev', instead of the ACPI device, 'acpi_dev', that is currently being tracked with PNP based IPMI device discovery.

---

 drivers/char/ipmi/ipmi_si_intf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +1 -1
index 806ae83..37c6912 100644
@@ -1934,7 +1934,7 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev *dev,
 		info->irq_setup = std_irq_setup;
 	}
 
-	info->dev = &acpi_dev->dev;
+	info->dev = &dev->dev;
 	pnp_set_drvdata(dev, info);
 
 	return try_smi_init(info);
#9 lore raw

Re: [PATCH 3/4] ipmi: Convert tracking of the ACPI device pointer to a PNP device

Quoted 4 lines · Myron Stowe

Convert PNP patch (git 9e368fa011d4e0aa050db348d69514900520e40b) to maintain a pointer to a PNP device, 'pnp_dev', instead of the ACPI device, 'acpi_dev', that is currently being tracked with PNP based IPMI device discovery.

Hi, Myron
Now the IPMI interface defined in ACPI namespace is detected by using
PNP device driver. But it seems that this still belongs to ACPI detection mechanism. At the same time IPMI device defined in ACPI namespace is also used to enable the communication between ACPI and IPMI device. In such case it will be better to know whether the IPMI interface is discovered by using ACPI detection mechanism , which can be realized by comparing the info->dev with acpi_dev->dev.

     Can we still use the acpi_dev->dev to track the IPMI device

discovery?

thanks.
Yakui

Quoted 25 lines

---

 drivers/char/ipmi/ipmi_si_intf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +1 -2
index 806ae83..37c6912 100644
@@ -1934,7 +1934,7 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev *dev,
 		info->irq_setup = std_irq_setup;
 	}
 
-	info->dev = &acpi_dev->dev;
+	info->dev = &dev->dev;
 	pnp_set_drvdata(dev, info);
 
 	return try_smi_init(info);
 
--

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

#10 lore raw

Re: [PATCH 3/4] ipmi: Convert tracking of the ACPI device pointer to a PNP device

Quoted 10 lines · ykzhao
Quoted 4 lines · Myron Stowe

Convert PNP patch (git 9e368fa011d4e0aa050db348d69514900520e40b) to maintain a pointer to a PNP device, 'pnp_dev', instead of the ACPI device, 'acpi_dev', that is currently being tracked with PNP based IPMI device discovery.

Hi, Myron
Now the IPMI interface defined in ACPI namespace is detected by using
PNP device driver. But it seems that this still belongs to ACPI detection mechanism.

Hi Yakui:

The ACPI namespace enumerated IPMI interface is indeed detected via PNP. This definitely can be a little confusing, especially in light of the fact that ACPI detection mechanisms exist. The reasoning for using PNP is based on the fact that within Linux, PNP subsumes ACPI (and ISA, EIAS, and also PNP BIOS). Using a Venn diagram to model the situation produces something like:
--------------------

  | PNP              |
  |     -----------  |
  |     | ACPI    |  |
  |     -----------  |
  |                  |

--------------------
I left ISA, EISA, and PNP BIOS out of the Venn diagram since ascii based diagrams are so horrific already but each would also be a distinct, non-overlapping, component within PNP.

Quoted 5 lines

At the same time IPMI device defined in ACPI
namespace is also used to enable the communication between ACPI and IPMI device. In such case it will be better to know whether the IPMI interface is discovered by using ACPI detection mechanism , which can be realized by comparing the info->dev with acpi_dev->dev.

I'm thinking your concern here is related to the IPMI op-region patch you have produced that Corey has not yet taken in.

One can still obtain an 'acpi_device' handle from the 'pnp_dev' handle that 'ipmi_pnp_probe()' is given. The 'opregion_setup()' routine provides such. I was going to provide an example but looking at 'ipmi_pnp_probe()' it is already being used at the very top of the routine so you should be able to use 'acpi_dev'; it is already provided and setup. More so, one will not make it into the body of 'ipmi_pnp_probe()' unless the device was enumerated via ACPI namespace due to the check against the just mentioned 'acpi_dev' setup (in other words, if there were an IPMI device that was enumerated via PNP BIOS the check would have detected such and already bailed out). This seems to provide the means for detection type checking that you are alluding to.

Quoted 2 lines
	Can we still use the acpi_dev->dev to track the IPMI device

discovery?

The existing 'acpi_dev' provides both a means for detection type checking and, seems much simpler than carrying forward and maintaining some type of info->dev acpi_dev->dev based comparison.

Myron

Quoted 30 lines

thanks.
Yakui

Quoted 25 lines

---

 drivers/char/ipmi/ipmi_si_intf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +1 -2
index 806ae83..37c6912 100644
@@ -1934,7 +1934,7 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev *dev,
 		info->irq_setup = std_irq_setup;
 	}
 
-	info->dev = &acpi_dev->dev;
+	info->dev = &dev->dev;
 	pnp_set_drvdata(dev, info);
 
 	return try_smi_init(info);
 
--

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

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#11 lore raw

Re: [PATCH 3/4] ipmi: Convert tracking of the ACPI device pointer to a PNP device

Quoted 26 lines · Myron Stowe
Quoted 10 lines · ykzhao
Quoted 4 lines · Myron Stowe

Convert PNP patch (git 9e368fa011d4e0aa050db348d69514900520e40b) to maintain a pointer to a PNP device, 'pnp_dev', instead of the ACPI device, 'acpi_dev', that is currently being tracked with PNP based IPMI device discovery.

Hi, Myron
Now the IPMI interface defined in ACPI namespace is detected by using
PNP device driver. But it seems that this still belongs to ACPI detection mechanism.

Hi Yakui:

The ACPI namespace enumerated IPMI interface is indeed detected via PNP. This definitely can be a little confusing, especially in light of the fact that ACPI detection mechanisms exist. The reasoning for using PNP is based on the fact that within Linux, PNP subsumes ACPI (and ISA, EIAS, and also PNP BIOS). Using a Venn diagram to model the situation produces something like:
--------------------

  | PNP              |
  |     -----------  |
  |     | ACPI    |  |
  |     -----------  |
  |                  |
Quoted 5 lines

--------------------
I left ISA, EISA, and PNP BIOS out of the Venn diagram since ascii based diagrams are so horrific already but each would also be a distinct, non-overlapping, component within PNP.

Hi, Myron
Thanks for the detailed explanation.
It seems that the ACPI device is firstly added to the system. And
then we will add the corresponding PNP device based on detected ACPI device so that most available PNP device drivers still can be used. Although sometimes both the PNP device and ACPI device are added, they will point to the same physical device. Right?

    Of course it will be clearer if the info->dev = pnp_dev->dev. It

indicates that it is tracked by PNP dev.

Quoted 8 lines
Quoted 5 lines

At the same time IPMI device defined in ACPI
namespace is also used to enable the communication between ACPI and IPMI device. In such case it will be better to know whether the IPMI interface is discovered by using ACPI detection mechanism , which can be realized by comparing the info->dev with acpi_dev->dev.

I'm thinking your concern here is related to the IPMI op-region patch you have produced that Corey has not yet taken in.

Yes. The ACPI IPMI opregion patch tries to enable the communication between ACPI and IPMI device by comparing the info->dev and acpi_dev->dev. Info->dev will be passed as the argument of ipmi_smi_watcher callback function(new_smi), in which we create the communication channel between ACPI and IPMI.

If we change the info->dev from acpi_dev->dev to pnp_dev->dev, then we will use another mechanism to check whether the IPMI device is defined in ACPI namespace. Of course it will be simpler.

Quoted 12 lines

One can still obtain an 'acpi_device' handle from the 'pnp_dev' handle that 'ipmi_pnp_probe()' is given. The 'opregion_setup()' routine provides such. I was going to provide an example but looking at 'ipmi_pnp_probe()' it is already being used at the very top of the routine so you should be able to use 'acpi_dev'; it is already provided and setup. More so, one will not make it into the body of 'ipmi_pnp_probe()' unless the device was enumerated via ACPI namespace due to the check against the just mentioned 'acpi_dev' setup (in other words, if there were an IPMI device that was enumerated via PNP BIOS the check would have detected such and already bailed out). This seems to provide the means for detection type checking that you are alluding to.

Very good explanation. And you are right. Now the ipmi pnp device driver is only effective to the IPMI device defined in ACPI namespace.

Quoted 41 lines
Quoted 3 lines
	Can we still use the acpi_dev->dev to track the IPMI device

discovery?

The existing 'acpi_dev' provides both a means for detection type checking and, seems much simpler than carrying forward and maintaining some type of info->dev acpi_dev->dev based comparison.

Myron

Quoted 30 lines

thanks.
Yakui

Quoted 25 lines

---

 drivers/char/ipmi/ipmi_si_intf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +1 -2
index 806ae83..37c6912 100644
@@ -1934,7 +1934,7 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev *dev,
 		info->irq_setup = std_irq_setup;
 	}
 
-	info->dev = &acpi_dev->dev;
+	info->dev = &dev->dev;
 	pnp_set_drvdata(dev, info);
 
 	return try_smi_init(info);
 
--

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

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

#12 lore raw

Re: [PATCH 3/4] ipmi: Convert tracking of the ACPI device pointer to a PNP device

Quoted 33 lines · ykzhao
Quoted 26 lines · Myron Stowe
Quoted 10 lines · ykzhao
Quoted 4 lines · Myron Stowe

Convert PNP patch (git 9e368fa011d4e0aa050db348d69514900520e40b) to maintain a pointer to a PNP device, 'pnp_dev', instead of the ACPI device, 'acpi_dev', that is currently being tracked with PNP based IPMI device discovery.

Hi, Myron
Now the IPMI interface defined in ACPI namespace is detected by using
PNP device driver. But it seems that this still belongs to ACPI detection mechanism.

Hi Yakui:

The ACPI namespace enumerated IPMI interface is indeed detected via PNP. This definitely can be a little confusing, especially in light of the fact that ACPI detection mechanisms exist. The reasoning for using PNP is based on the fact that within Linux, PNP subsumes ACPI (and ISA, EIAS, and also PNP BIOS). Using a Venn diagram to model the situation produces something like:
--------------------

  | PNP              |
  |     -----------  |
  |     | ACPI    |  |
  |     -----------  |
  |                  |
Quoted 5 lines

--------------------
I left ISA, EISA, and PNP BIOS out of the Venn diagram since ascii based diagrams are so horrific already but each would also be a distinct, non-overlapping, component within PNP.

Quoted 7 lines

Hi, Myron
Thanks for the detailed explanation.
It seems that the ACPI device is firstly added to the system. And
then we will add the corresponding PNP device based on detected ACPI device so that most available PNP device drivers still can be used. Although sometimes both the PNP device and ACPI device are added, they will point to the same physical device. Right?

Yes, a PNP device (struct pnp_dev) is obviously distinct from an ACPI device (struct acpi_device) but they both end up pointing to the same physical device.

Quoted 18 lines
    Of course it will be clearer if the info->dev = pnp_dev->dev. It

indicates that it is tracked by PNP dev.

Quoted 8 lines
Quoted 5 lines

At the same time IPMI device defined in ACPI
namespace is also used to enable the communication between ACPI and IPMI device. In such case it will be better to know whether the IPMI interface is discovered by using ACPI detection mechanism , which can be realized by comparing the info->dev with acpi_dev->dev.

I'm thinking your concern here is related to the IPMI op-region patch you have produced that Corey has not yet taken in.

Yes. The ACPI IPMI opregion patch tries to enable the communication between ACPI and IPMI device by comparing the info->dev and acpi_dev->dev. Info->dev will be passed as the argument of ipmi_smi_watcher callback function(new_smi), in which we create the communication channel between ACPI and IPMI.

Quoted 3 lines

If we change the info->dev from acpi_dev->dev to pnp_dev->dev, then we will use another mechanism to check whether the IPMI device is defined in ACPI namespace. Of course it will be simpler.

Good, simpler tends to be better.

By the way, if there are conflicts between our two patch series I'll be more than willing to help resolve such. If it ends up being easier for Corey to take in your series first I'll rework my series and resubmit it.

Myron

Quoted 59 lines
Quoted 13 lines

One can still obtain an 'acpi_device' handle from the 'pnp_dev' handle that 'ipmi_pnp_probe()' is given. The 'opregion_setup()' routine provides such. I was going to provide an example but looking at 'ipmi_pnp_probe()' it is already being used at the very top of the routine so you should be able to use 'acpi_dev'; it is already provided and setup. More so, one will not make it into the body of 'ipmi_pnp_probe()' unless the device was enumerated via ACPI namespace due to the check against the just mentioned 'acpi_dev' setup (in other words, if there were an IPMI device that was enumerated via PNP BIOS the check would have detected such and already bailed out). This seems to provide the means for detection type checking that you are alluding to.

Very good explanation. And you are right. Now the ipmi pnp device driver is only effective to the IPMI device defined in ACPI namespace.

Quoted 41 lines
Quoted 3 lines
	Can we still use the acpi_dev->dev to track the IPMI device

discovery?

The existing 'acpi_dev' provides both a means for detection type checking and, seems much simpler than carrying forward and maintaining some type of info->dev acpi_dev->dev based comparison.

Myron

Quoted 30 lines

thanks.
Yakui

Quoted 25 lines

---

 drivers/char/ipmi/ipmi_si_intf.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +1 -2
index 806ae83..37c6912 100644
@@ -1934,7 +1934,7 @@ static int __devinit ipmi_pnp_probe(struct pnp_dev *dev,
 		info->irq_setup = std_irq_setup;
 	}
 
-	info->dev = &acpi_dev->dev;
+	info->dev = &dev->dev;
 	pnp_set_drvdata(dev, info);
 
 	return try_smi_init(info);
 
--

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

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#13 lore raw

Re: [PATCH 3/4] ipmi: Convert tracking of the ACPI device pointer to a PNP device

Quoted 4 lines · Myron Stowe

By the way, if there are conflicts between our two patch series I'll be more than willing to help resolve such.  If it ends up being easier for Corey to take in your series first I'll rework my series and resubmit it.

BTW, Corey, what's happening as far as merging Yakaui's IPMI opregion patches? I checked linux-next, and I don't see them there. Are you planning to push them upstream during this (soon-to-close) merge window?

http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=history;f=drivers/char/ipmi/ipmi_si_intf.c;h=176f1751237f57807f3023d9427972dcb6614392;hb=HEAD

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

#14 lore raw

PATCH 4/4 ipmi: Update driver to use 'dev_printk()' and its constructs

In reply to Myron Stowe

Update core IPMI driver printk()'s with dev_printk(), and its constructs, to provide additional device topology information.

An example of the additional device topology for a PNP device -
ipmi_si 00:02: probing via ACPI
ipmi_si 00:02: [io 0x0ca2-0x0ca3] regsize 1 spacing 1 irq 0 ipmi_si 00:02: Found new BMC (man_id: 0x00000b, prod_id: 0x0000, ... ipmi_si 00:02: IPMI kcs interface initialized

and for a PCI device -
ipmi_si 0000:01:04.6: probing via PCI
ipmi_si 0000:01:04.6: PCI INT A -> GSI 21 (level, low) -> IRQ 21 ipmi_si 0000:01:04.6: [mem 0xf1ef0000-0xf1ef00ff] regsize 1 spaci... ipmi_si 0000:01:04.6: IPMI kcs interface initialized

---

 drivers/char/ipmi/ipmi_msghandler.c |   11 ++--
 drivers/char/ipmi/ipmi_si_intf.c    |  103 ++++++++++++++++-------------------
 2 files changed, 53 insertions(+), 61 deletions(-)
drivers/char/ipmi/ipmi_msghandler.c +5 -6
index ec5e3f8..0e30114 100644
@@ -2495,12 +2495,11 @@ static int ipmi_bmc_register(ipmi_smi_t intf, int ifnum,
 			return rv;
 		}
 
-		printk(KERN_INFO
-		       "ipmi: Found new BMC (man_id: 0x%6.6x, "
-		       " prod_id: 0x%4.4x, dev_id: 0x%2.2x)\n",
-		       bmc->id.manufacturer_id,
-		       bmc->id.product_id,
-		       bmc->id.device_id);
+		dev_info(intf->si_dev, "Found new BMC (man_id: 0x%6.6x, "
+			 "prod_id: 0x%4.4x, dev_id: 0x%2.2x)\n",
+			 bmc->id.manufacturer_id,
+			 bmc->id.product_id,
+			 bmc->id.device_id);
 	}
 
 	/*
drivers/char/ipmi/ipmi_si_intf.c +48 -55

249 lines not on this page · raw message

#15 lore raw
In reply to Myron Stowe
Quoted 7 lines · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism and update the driver's core to use dev_printk() and its constructs.

As part of this patch series I wanted to remove the 'PFX' argument from ipmi_of_probe()'s dev_printk constructs as I believe it produces redundant output but I do not have a PPC platform to test against.

I like the dev_printk() stuff. The order of discovery change is correct, too, I believe. I don't have a PPC platform (with OF, anyway) with IPMI, so I can't test that, either.

Quoted 3 lines

Ultimately, I would like to see if it is possible to also remove IPMI's SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so I have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

-corey

#16 lore raw
Quoted 11 lines · Corey Minyard
Quoted 7 lines · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism and update the driver's core to use dev_printk() and its constructs.

As part of this patch series I wanted to remove the 'PFX' argument from ipmi_of_probe()'s dev_printk constructs as I believe it produces redundant output but I do not have a PPC platform to test against.

I like the dev_printk() stuff. The order of discovery change is correct, too, I believe. I don't have a PPC platform (with OF, anyway) with IPMI, so I can't test that, either.

Bummer, I would have liked to clean up 'ipmi_of_probe()' as part of this series as it is obvious it was not written in the style of the original author - reading 'ipmi_pci_probe()', 'ipmi_pnp_probe()', and 'ipmi_of_probe()' side-by-side the latter sticks out like a sore thumb.

Quoted 8 lines
Quoted 4 lines

Ultimately, I would like to see if it is possible to also remove IPMI's SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so I have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

It was a mistake on my part to make the comment about SMBIOS as the patch series does not include such action and it unnecessarily creates confusion with the current intent of removing SPMI as a discovery method.

It would seem that the most likely situation requiring SPMI would be an embedded system that does not use ACPI namespace and does not support running Windows (Windows relies on ACPI namespace and/or PCI; it does not look for, or use, SMBIOS and/or SPMI). If anyone reading this discussion thread knows of an explicit example in which SPMI is truly required then obviously I would be wrong and could no longer pursue such. Its just that I'm not yet convinced of such based on my experiences.

Myron

Quoted 1 line

-corey

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#17 lore raw
In reply to Corey Minyard
Quoted 4 lines · Corey Minyard
Quoted 3 lines · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism and update the driver's core to use dev_printk() and its constructs.

snip

Quoted 9 lines
Quoted 4 lines

Ultimately, I would like to see if it is possible to also remove IPMI's SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so I have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

Discussions concerning this patch series seem to have ceased and I have not heard back from anyone that can definitively state that the SPMI table is actively being used in the field.

At this point, it seems that the most logical ways to proceed are:
* Take the patch series as it is with the possible risk that we will encounter a system in the field that relies on SPMI.

* Or, I can rework the patch series, basically splitting it into
two. The first series would include
ipmi: Raise precedence of PNP based discovery mechanisms (ACPI, PCI),
ipmi: Convert tracking of the ACPI device pointer to a PNP device,
ipmi: Update driver to use 'dev_printk()' and its constructs,
with the second series including
ipmi: Remove SPMI table based device discovery method,
ipmi: Further 'dev_printk()' conversion that is dependent on SPMI removal.
The idea being that the more contentious second series, now isolated,
could be easily retracted if necessary.

I do understand that it is possible that we will encounter a system that relies on SPMI. The most likely scenario would seem to be with respect to embedded systems. However, I think it is unlikely for the following reasons (as always, please respond to any and all concerns as it is this community that are the subject matter experts) and would still like to consider removing such:

The IPMI related SMBIOS and SPMI tables are static, the idea being that such tables can be referenced by the OS very early in its boot related processing; well before the OS has progressed to the point of ACPI namespace based control method availability. A couple of points concerning such are: they seem relatively redundant with respect to each other, and, the current IPMI driver is not truly capable of taking advantage of such "static" capability[1].

IPMI's SPMI table discovery mechanism was championed by HP. This was mainly for HP's proprietary HP-UX OS as it did want to take advantage of the early reference capability that static tables provide (I do not know if SMBIOS was considered, or if so, why it was not considered viable - perhaps someone on this list has that history). As such, we seriously question whether other vendors have even included SPMI into their BIOS'.

Modern systems rely on ACPI namespace and/or PCI as preferred enumeration techniques. Windows only looks at these, and not SMBIOS or SPMI, which has bearing since x86 Linux typically runs on platforms also capable of running Windows. Yes, embedded systems are the biggest exception here.

Bjorn Helgaas also covered many of these points, and a few others, in his reply to Bela Lubkin concerning [Patch 2/4] which is worth re-reading.

I'm looking forward to any feedback or responses of any kind, even rebuttals.

Thanks,

Myron

[1] I was not able to express this point as well as I would have liked. The point I'm attempting to make is that the IPMI driver, as it currently exists, is not capable of probing and attaching to a device during early boot processing to really take advantage of "static" tables. The fact that it is capable of being built and run/installed as a module is a big hint towards such but is not, in itself, definitive. However, its PCI and ACPI probing mechanisms are definitive in that they rely on their respective subsystems having already been setup which is well after early boot processing.

Quoted 3 lines

-corey

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#18 lore raw

Re: [SPAM] - Re: [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Myron,

How about this for an approach with SPMI:
Rather than remove it entirely, use '#ifdef SPMI_DEPRECATED' around that code, so that it can still be enabled at compile time by those that later find that they need it.
And add a comment with instructions to notify this list if someone finds that they need to enable it.

That should prove conclusively whether it is used/needed or not.

Andy

-----Original Message-----
From: Myron Stowe [mailto:[email protected]]
Sent: Tuesday, March 09, 2010 6:16 PM
To: Corey Minyard
Cc: [email protected]; [email protected]; [email protected]
Subject: [SPAM] - Re: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 2 lines · Corey Minyard
Quoted 1 line · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism

and

Quoted 2 lines
Quoted 2 lines

update the driver's core to use dev_printk() and its constructs.

snip

Quoted 2 lines
Quoted 2 lines

Ultimately, I would like to see if it is possible to also remove

IPMI's

Quoted 3 lines
Quoted 2 lines

SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so

I

Quoted 4 lines

have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

Discussions concerning this patch series seem to have ceased and I have not heard back from anyone that can definitively state that the SPMI table is actively being used in the field.

At this point, it seems that the most logical ways to proceed are:
* Take the patch series as it is with the possible risk that we will encounter a system in the field that relies on SPMI.

* Or, I can rework the patch series, basically splitting it into two. The first series would include
ipmi: Raise precedence of PNP based discovery mechanisms
(ACPI, PCI),
ipmi: Convert tracking of the ACPI device pointer to a
PNP device,
ipmi: Update driver to use 'dev_printk()' and its
constructs,
with the second series including
ipmi: Remove SPMI table based device discovery method,
ipmi: Further 'dev_printk()' conversion that is
dependent on SPMI removal.
The idea being that the more contentious second series, now isolated, could be easily retracted if necessary.

I do understand that it is possible that we will encounter a system that relies on SPMI. The most likely scenario would seem to be with respect to embedded systems. However, I think it is unlikely for the following reasons (as always, please respond to any and all concerns as it is this community that are the subject matter experts) and would still like to consider removing such:

The IPMI related SMBIOS and SPMI tables are static, the idea being that such tables can be referenced by the OS very early in its boot related processing; well before the OS has progressed to the point of ACPI namespace based control method availability. A couple of points concerning such are: they seem relatively redundant with respect to each other, and, the current IPMI driver is not truly capable of taking advantage of such "static" capability[1].

IPMI's SPMI table discovery mechanism was championed by HP. This was mainly for HP's proprietary HP-UX OS as it did want to take advantage of the early reference capability that static tables provide (I do not know if SMBIOS was considered, or if so, why it was not considered viable - perhaps someone on this list has that history). As such, we seriously question whether other vendors have even included SPMI into their BIOS'.

Modern systems rely on ACPI namespace and/or PCI as preferred enumeration techniques. Windows only looks at these, and not SMBIOS or SPMI, which has bearing since x86 Linux typically runs on platforms also capable of running Windows. Yes, embedded systems are the biggest exception here.

Bjorn Helgaas also covered many of these points, and a few others, in his reply to Bela Lubkin concerning [Patch 2/4] which is worth re-reading.

I'm looking forward to any feedback or responses of any kind, even rebuttals.

Thanks,

Myron

[1] I was not able to express this point as well as I would have liked. The point I'm attempting to make is that the IPMI driver, as it currently exists, is not capable of probing and attaching to a device during early boot processing to really take advantage of "static" tables. The fact that it is capable of being built and run/installed as a module is a big hint towards such but is not, in itself, definitive. However, its PCI and ACPI probing mechanisms are definitive in that they rely on their respective subsystems having already been setup which is well after early boot processing.

Quoted 3 lines

-corey

Signature · 26 lines
Myron Stowe                             HP Open Source Linux Lab (OSLL)




------------------------------------------------------------------------
------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Openipmi-developer mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer
sts.sourceforge.net/lists/listinfo/openipmi-developer



------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs
proactively, and fine-tune applications for parallel performance.
See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
#19 lore raw

Re: [SPAM] - Re: [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 11 lines · Andy Cress

Myron,

How about this for an approach with SPMI:
Rather than remove it entirely, use '#ifdef SPMI_DEPRECATED' around that code, so that it can still be enabled at compile time by those that later find that they need it.
And add a comment with instructions to notify this list if someone finds that they need to enable it.

That should prove conclusively whether it is used/needed or not.

I'd prefer way to prove this without causing people problems in the field. The average user is not going to look at the source code and go "Oh, this is what I need to do!" It will be a painful process for them to figure out what happened.

I don't see a way to test this in the field without causing issues. If this code does not hurt anything, then IMHO it needs to stay. This is a legacy driver, really, so removing things of this nature is dangerous. If leaving the code in is a problem, then we need to figure something out.

I'm for the "two patch" approach mentioned by Myron.

-corey

Quoted 106 lines

Andy

-----Original Message-----
From: Myron Stowe [mailto:[email protected]]
Sent: Tuesday, March 09, 2010 6:16 PM
To: Corey Minyard
Cc: [email protected]; [email protected]; [email protected]
Subject: [SPAM] - Re: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 4 lines · Corey Minyard
Quoted 2 lines · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism

and

Quoted 3 lines
Quoted 3 lines

update the driver's core to use dev_printk() and its constructs.

snip

Quoted 2 lines
Quoted 2 lines

Ultimately, I would like to see if it is possible to also remove

IPMI's

Quoted 5 lines
Quoted 3 lines

SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so

I

Quoted 5 lines

have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

Discussions concerning this patch series seem to have ceased and I have not heard back from anyone that can definitively state that the SPMI table is actively being used in the field.

At this point, it seems that the most logical ways to proceed are:
* Take the patch series as it is with the possible risk that we will encounter a system in the field that relies on SPMI.

* Or, I can rework the patch series, basically splitting it into two. The first series would include
ipmi: Raise precedence of PNP based discovery mechanisms
(ACPI, PCI),
ipmi: Convert tracking of the ACPI device pointer to a
PNP device,
ipmi: Update driver to use 'dev_printk()' and its
constructs,
with the second series including
ipmi: Remove SPMI table based device discovery method,
ipmi: Further 'dev_printk()' conversion that is
dependent on SPMI removal.
The idea being that the more contentious second series, now isolated, could be easily retracted if necessary.

I do understand that it is possible that we will encounter a system that relies on SPMI. The most likely scenario would seem to be with respect to embedded systems. However, I think it is unlikely for the following reasons (as always, please respond to any and all concerns as it is this community that are the subject matter experts) and would still like to consider removing such:

The IPMI related SMBIOS and SPMI tables are static, the idea being that such tables can be referenced by the OS very early in its boot related processing; well before the OS has progressed to the point of ACPI namespace based control method availability. A couple of points concerning such are: they seem relatively redundant with respect to each other, and, the current IPMI driver is not truly capable of taking advantage of such "static" capability[1].

IPMI's SPMI table discovery mechanism was championed by HP. This was mainly for HP's proprietary HP-UX OS as it did want to take advantage of the early reference capability that static tables provide (I do not know if SMBIOS was considered, or if so, why it was not considered viable - perhaps someone on this list has that history). As such, we seriously question whether other vendors have even included SPMI into their BIOS'.

Modern systems rely on ACPI namespace and/or PCI as preferred enumeration techniques. Windows only looks at these, and not SMBIOS or SPMI, which has bearing since x86 Linux typically runs on platforms also capable of running Windows. Yes, embedded systems are the biggest exception here.

Bjorn Helgaas also covered many of these points, and a few others, in his reply to Bela Lubkin concerning [Patch 2/4] which is worth re-reading.

I'm looking forward to any feedback or responses of any kind, even rebuttals.

Thanks,

Myron

Quoted 17 lines

[1] I was not able to express this point as well as I would have liked. The point I'm attempting to make is that the IPMI driver, as it currently exists, is not capable of probing and attaching to a device during early boot processing to really take advantage of "static" tables. The fact that it is capable of being built and run/installed as a module is a big hint towards such but is not, in itself, definitive. However, its PCI and ACPI probing mechanisms are definitive in that they rely on their respective subsystems having already been setup which is well after early boot processing.

Quoted 3 lines

-corey

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev

#20 lore raw

Re: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 10 lines · Corey Minyard
Quoted 9 lines · Andy Cress

Myron,

How about this for an approach with SPMI:
Rather than remove it entirely, use '#ifdef SPMI_DEPRECATED' around that code, so that it can still be enabled at compile time by those that later find that they need it.
And add a comment with instructions to notify this list if someone finds that they need to enable it.

Quoted 8 lines
Quoted 2 lines

That should prove conclusively whether it is used/needed or not.

I'd prefer way to prove this without causing people problems in the field. The average user is not going to look at the source code and go "Oh, this is what I need to do!" It will be a painful process for them to figure out what happened.

I don't see a way to test this in the field without causing issues.

Unfortunately I don't either (assuming that it would have).

Quoted 2 lines

If
this code does not hurt anything, then IMHO it needs to stay.

This code is causing HP problems (yes, the irony is not lost on me) which was the basis of us researching whether or not it is truly required and attempting to remove it.

Quoted 5 lines

This is a
legacy driver, really, so removing things of this nature is dangerous. If leaving the code in is a problem, then we need to figure something out.

I'm for the "two patch" approach mentioned by Myron.

It looks like I need to concede here. As I've said before, I'll rework the patch. My question at this point is - Is there any point in creating the second patch series in the "two patch" approach?

Myron

P.S. Thanks Andy for your implied support - or at least open minded attitude.

Quoted 127 lines

-corey

Quoted 123 lines

Andy

-----Original Message-----
From: Myron Stowe [mailto:[email protected]]
Sent: Tuesday, March 09, 2010 6:16 PM
To: Corey Minyard
Cc: [email protected]; [email protected]; [email protected]
Subject: [SPAM] - Re: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 4 lines · Corey Minyard
Quoted 2 lines · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism

and

Quoted 3 lines
Quoted 3 lines

update the driver's core to use dev_printk() and its constructs.

snip

Quoted 2 lines
Quoted 2 lines

Ultimately, I would like to see if it is possible to also remove

IPMI's

Quoted 5 lines
Quoted 3 lines

SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so

I

Quoted 5 lines

have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

Discussions concerning this patch series seem to have ceased and I have not heard back from anyone that can definitively state that the SPMI table is actively being used in the field.

At this point, it seems that the most logical ways to proceed are:
* Take the patch series as it is with the possible risk that we will encounter a system in the field that relies on SPMI.

* Or, I can rework the patch series, basically splitting it into two. The first series would include
ipmi: Raise precedence of PNP based discovery mechanisms
(ACPI, PCI),
ipmi: Convert tracking of the ACPI device pointer to a
PNP device,
ipmi: Update driver to use 'dev_printk()' and its
constructs,
with the second series including
ipmi: Remove SPMI table based device discovery method,
ipmi: Further 'dev_printk()' conversion that is
dependent on SPMI removal.
The idea being that the more contentious second series, now isolated, could be easily retracted if necessary.

I do understand that it is possible that we will encounter a system that relies on SPMI. The most likely scenario would seem to be with respect to embedded systems. However, I think it is unlikely for the following reasons (as always, please respond to any and all concerns as it is this community that are the subject matter experts) and would still like to consider removing such:

The IPMI related SMBIOS and SPMI tables are static, the idea being that such tables can be referenced by the OS very early in its boot related processing; well before the OS has progressed to the point of ACPI namespace based control method availability. A couple of points concerning such are: they seem relatively redundant with respect to each other, and, the current IPMI driver is not truly capable of taking advantage of such "static" capability[1].

IPMI's SPMI table discovery mechanism was championed by HP. This was mainly for HP's proprietary HP-UX OS as it did want to take advantage of the early reference capability that static tables provide (I do not know if SMBIOS was considered, or if so, why it was not considered viable - perhaps someone on this list has that history). As such, we seriously question whether other vendors have even included SPMI into their BIOS'.

Modern systems rely on ACPI namespace and/or PCI as preferred enumeration techniques. Windows only looks at these, and not SMBIOS or SPMI, which has bearing since x86 Linux typically runs on platforms also capable of running Windows. Yes, embedded systems are the biggest exception here.

Bjorn Helgaas also covered many of these points, and a few others, in his reply to Bela Lubkin concerning [Patch 2/4] which is worth re-reading.

I'm looking forward to any feedback or responses of any kind, even rebuttals.

Thanks,

Myron

[1] I was not able to express this point as well as I would have liked. The point I'm attempting to make is that the IPMI driver, as it currently exists, is not capable of probing and attaching to a device during early boot processing to really take advantage of "static" tables. The fact that it is capable of being built and run/installed as a module is a big hint towards such but is not, in itself, definitive. However, its PCI and ACPI probing mechanisms are definitive in that they rely on their respective subsystems having already been setup which is well after early boot processing.

Quoted 3 lines

-corey

Signature · 1 line
Myron Stowe                             HP Open Source Linux Lab (OSLL)
#21 lore raw

RE: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

I support the "#ifdef it out" approach, provided it is done according to usual Linux kernel procedure. ifdef it on CONFIG_IPMI_SPMI, add appropriate lines to the right Kconfig file, turn it off by default. Making it an obscure #ifdef with no proper configuration infrastructure around it would be wrong.

Configuring a feature out (but leaving the escape hatch for people who need it on) is a time-tested method of removing functionality when it is not 100% proven that it can be safely removed in all cases.

Quoted 1 line

Bela<

________________________________________
From: Myron Stowe [[email protected]]
Sent: Wednesday, March 10, 2010 9:55 AM
To: Corey Minyard
Cc: [email protected]; [email protected]; Andy Cress; [email protected]
Subject: Re: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 10 lines · Corey Minyard
Quoted 9 lines · Andy Cress

Myron,

How about this for an approach with SPMI:
Rather than remove it entirely, use '#ifdef SPMI_DEPRECATED' around that code, so that it can still be enabled at compile time by those that later find that they need it.
And add a comment with instructions to notify this list if someone finds that they need to enable it.

Quoted 8 lines
Quoted 2 lines

That should prove conclusively whether it is used/needed or not.

I'd prefer way to prove this without causing people problems in the field. The average user is not going to look at the source code and go "Oh, this is what I need to do!" It will be a painful process for them to figure out what happened.

I don't see a way to test this in the field without causing issues.

Unfortunately I don't either (assuming that it would have).

Quoted 2 lines

If
this code does not hurt anything, then IMHO it needs to stay.

This code is causing HP problems (yes, the irony is not lost on me) which was the basis of us researching whether or not it is truly required and attempting to remove it.

Quoted 5 lines

This is a
legacy driver, really, so removing things of this nature is dangerous. If leaving the code in is a problem, then we need to figure something out.

I'm for the "two patch" approach mentioned by Myron.

It looks like I need to concede here. As I've said before, I'll rework the patch. My question at this point is - Is there any point in creating the second patch series in the "two patch" approach?

Myron

P.S. Thanks Andy for your implied support - or at least open minded attitude.

Quoted 127 lines

-corey

Quoted 123 lines

Andy

-----Original Message-----
From: Myron Stowe [mailto:[email protected]]
Sent: Tuesday, March 09, 2010 6:16 PM
To: Corey Minyard
Cc: [email protected]; [email protected]; [email protected]
Subject: [SPAM] - Re: [Openipmi-developer] [PATCH 0/4] ipmi: remove SPMI and update core driver with dev_printk - Email found in subject

Quoted 4 lines · Corey Minyard
Quoted 2 lines · Myron Stowe

These patches remove the SPMI based IPMI device discovery mechanism

and

Quoted 3 lines
Quoted 3 lines

update the driver's core to use dev_printk() and its constructs.

snip

Quoted 2 lines
Quoted 2 lines

Ultimately, I would like to see if it is possible to also remove

IPMI's

Quoted 5 lines
Quoted 3 lines

SMBIOS based device discovery mechanism.

Maybe in an ideal world, but I don't know where an ideal world is, so

I

Quoted 5 lines

have to live in the one I'm in. There's plenty of systems that only document this in SMBIOS tables, there's plenty of systems with broken ACPI, etc. So SMBIOS and SPMI are going to have to stay.

Discussions concerning this patch series seem to have ceased and I have not heard back from anyone that can definitively state that the SPMI table is actively being used in the field.

At this point, it seems that the most logical ways to proceed are:
* Take the patch series as it is with the possible risk that we will encounter a system in the field that relies on SPMI.

* Or, I can rework the patch series, basically splitting it into two. The first series would include
ipmi: Raise precedence of PNP based discovery mechanisms
(ACPI, PCI),
ipmi: Convert tracking of the ACPI device pointer to a
PNP device,
ipmi: Update driver to use 'dev_printk()' and its
constructs,
with the second series including
ipmi: Remove SPMI table based device discovery method,
ipmi: Further 'dev_printk()' conversion that is
dependent on SPMI removal.
The idea being that the more contentious second series, now isolated, could be easily retracted if necessary.

I do understand that it is possible that we will encounter a system that relies on SPMI. The most likely scenario would seem to be with respect to embedded systems. However, I think it is unlikely for the following reasons (as always, please respond to any and all concerns as it is this community that are the subject matter experts) and would still like to consider removing such:

The IPMI related SMBIOS and SPMI tables are static, the idea being that such tables can be referenced by the OS very early in its boot related processing; well before the OS has progressed to the point of ACPI namespace based control method availability. A couple of points concerning such are: they seem relatively redundant with respect to each other, and, the current IPMI driver is not truly capable of taking advantage of such "static" capability[1].

IPMI's SPMI table discovery mechanism was championed by HP. This was mainly for HP's proprietary HP-UX OS as it did want to take advantage of the early reference capability that static tables provide (I do not know if SMBIOS was considered, or if so, why it was not considered viable - perhaps someone on this list has that history). As such, we seriously question whether other vendors have even included SPMI into their BIOS'.

Modern systems rely on ACPI namespace and/or PCI as preferred enumeration techniques. Windows only looks at these, and not SMBIOS or SPMI, which has bearing since x86 Linux typically runs on platforms also capable of running Windows. Yes, embedded systems are the biggest exception here.

Bjorn Helgaas also covered many of these points, and a few others, in his reply to Bela Lubkin concerning [Patch 2/4] which is worth re-reading.

I'm looking forward to any feedback or responses of any kind, even rebuttals.

Thanks,

Myron

[1] I was not able to express this point as well as I would have liked. The point I'm attempting to make is that the IPMI driver, as it currently exists, is not capable of probing and attaching to a device during early boot processing to really take advantage of "static" tables. The fact that it is capable of being built and run/installed as a module is a big hint towards such but is not, in itself, definitive. However, its PCI and ACPI probing mechanisms are definitive in that they rely on their respective subsystems having already been setup which is well after early boot processing.

Quoted 3 lines

-corey

--
Myron Stowe HP Open Source Linux Lab (OSLL)

------------------------------------------------------------------------------
Download Intel&#174; Parallel Studio Eval
Try the new software tools for yourself. Speed compiling, find bugs proactively, and fine-tune applications for parallel performance. See why Intel Parallel Studio got high marks during beta.
http://p.sf.net/sfu/intel-sw-dev
_______________________________________________
Openipmi-developer mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openipmi-developer