| Message ID | 1462995966-1184-8-git-send-email-minyard@acm.org |
|---|---|
| State | New |
| Headers | show |
On 05/12/2016 02:36 AM, Michael S. Tsirkin wrote: > On Wed, May 11, 2016 at 02:46:06PM -0500, minyard@acm.org wrote: >> From: Corey Minyard <cminyard@mvista.com> >> >> Signed-off-by: Corey Minyard <cminyard@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 >> ipmi_add_fwinfo(&sid->fwinfo, errp); >> } >> >> -- >> 2.7.4
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, minyard@acm.org wrote: >>>> From: Corey Minyard <cminyard@mvista.com> >>>> >>>> Signed-off-by: Corey Minyard <cminyard@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. 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 >>>> ipmi_add_fwinfo(&sid->fwinfo, errp); >>>> } >>>> -- >>>> 2.7.4
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"; ipmi_add_fwinfo(&sid->fwinfo, errp); }