Re: [PATCH 3/4] ipmi: Convert tracking of the ACPI device pointer to a PNP device
From: Myron Stowe <hidden>
Date: 2010-03-05 16:41:08
On Fri, 2010-03-05 at 09:46 +0800, ykzhao wrote:
On Fri, 2010-03-05 at 04:48 +0800, Myron Stowe wrote:quoted
On Thu, 2010-03-04 at 17:20 +0800, ykzhao wrote:quoted
On Thu, 2010-03-04 at 11:44 +0800, Myron Stowe wrote:quoted
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.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.
Of course it will be clearer if the info->dev = pnp_dev->dev. It indicates that it is tracked by PNP dev.quoted
quoted
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.
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
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
quoted
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. Myronquoted
thanks. Yakuiquoted
Signed-off-by: Myron Stowe <redacted> --- drivers/char/ipmi/ipmi_si_intf.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c index 806ae83..37c6912 100644 --- a/drivers/char/ipmi/ipmi_si_intf.c +++ b/drivers/char/ipmi/ipmi_si_intf.c@@ -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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-- Myron Stowe HP Open Source Linux Lab (OSLL)