On Thu, May 12, 2016 at 02:20:25PM -0500, Corey Minyard wrote:
> On 05/12/2016 08:35 AM, Michael S. Tsirkin wrote:
> >On Thu, May 12, 2016 at 08:32:51AM -0500, Corey Minyard wrote:
> >>On 05/12/2016 02:36 AM, Michael S. Tsirkin wrote:
> >>>On Wed, May 11, 2016 at 02:46:06PM -0500, miny...@acm.org wrote:
> >>>>From: Corey Minyard <cminy...@mvista.com>
> >>>>
> >>>>Signed-off-by: Corey Minyard <cminy...@mvista.com>
> >>>>---
> >>>> hw/ipmi/smbus_ipmi.c | 1 +
> >>>> 1 file changed, 1 insertion(+)
> >>>>
> >>>>diff --git a/hw/ipmi/smbus_ipmi.c b/hw/ipmi/smbus_ipmi.c
> >>>>index 4e7203b..3a34aaf 100644
> >>>>--- a/hw/ipmi/smbus_ipmi.c
> >>>>+++ b/hw/ipmi/smbus_ipmi.c
> >>>>@@ -167,6 +167,7 @@ static void smbus_ipmi_realize(DeviceState *dev,
> >>>>Error **errp)
> >>>> sid->fwinfo.base_address = sid->parent.i2c.address;
> >>>> sid->fwinfo.memspace = IPMI_MEMSPACE_SMBUS;
> >>>> sid->fwinfo.register_spacing = 1;
> >>>>+ sid->fwinfo.acpi_parent = "\\_SB.PCI0.SMB0";
> >>>I don't think it's a good idea to spread things like PCI0
> >>>outside acpi-build.c. Why do you want to pass in the path
> >>>at all?
> >>You have to define the namespace for the ASL definition,
> >>and you have to give the name in the serial bus definition.
> >>However, thinking it through some more, this name needs
> >>to come from the I2C device, not just hard-coded here.
> >>
> >>-corey
> >For now you can put it as PCI0 within aml-build.c.
> >
> >Also do we need \\_SB.PCI0? Why not just create the
> >device within PCI0 scope?
> I'm not sure I follow here. \SB.PCI0.SMB0 is the proper
> scope to identify which SMBus the device is on. This
> is how the ISA devices are added, for instance.
We add most of them within PCI0 scope instead.
E.g.
Device (PCI0) {
Device (SMB0) {
}
}
this way device does not need to know where it is.
> What I've done now to fix this is added an ACPI namespace
> to the I2C bus structure and stored the value for the I2C
> bus there in the i386 code.
>
> Maybe it should be in BusState? That way the ISA IPMI code
> could pull it from the bus, too.
>
> Putting PCI0 into aml-build.c (or hw/acpi/ipmi.c, really) seems
> like a violation of scope.
>
> -corey
Generally we scan each bus and list devices found there.
> >>>> ipmi_add_fwinfo(&sid->fwinfo, errp);
> >>>> }
> >>>>--
> >>>>2.7.4
