diff mbox

hw/piix_pci.c: Fix PIIX3-xen to initialize ids

Message ID 1308758311-32692-1-git-send-email-anthony.perard@citrix.com
State New
Headers show

Commit Message

Anthony PERARD June 22, 2011, 3:58 p.m. UTC 
From: Anthony PERARD <anthony.perard@citrix.com>

Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 hw/piix_pci.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

Comments

Stefano Stabellini June 22, 2011, 5:43 p.m. UTC | #1 
On Wed, 22 Jun 2011, anthony.perard@citrix.com wrote:
> From: Anthony PERARD <anthony.perard@citrix.com>
> 
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
>  hw/piix_pci.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 26ce904..d08b31a 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -478,6 +478,9 @@ static PCIDeviceInfo i440fx_info[] = {
>          .no_hotplug   = 1,
>          .init         = piix3_initfn,
>          .config_write = piix3_write_config_xen,
> +        .vendor_id    = PCI_VENDOR_ID_INTEL,
> +        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
> +        .class_id     = PCI_CLASS_BRIDGE_ISA,
>      },{
>          /* end of list */
>      }
 
shouldn't piix3_initfn take care of setting vendor_id, device_id and
class_id, as in the normal PIIX3 case?
Anthony PERARD June 22, 2011, 5:48 p.m. UTC | #2 
On Wed, Jun 22, 2011 at 18:43, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> On Wed, 22 Jun 2011, anthony.perard@citrix.com wrote:
>> From: Anthony PERARD <anthony.perard@citrix.com>
>>
>> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
>> ---
>>  hw/piix_pci.c |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
>> index 26ce904..d08b31a 100644
>> --- a/hw/piix_pci.c
>> +++ b/hw/piix_pci.c
>> @@ -478,6 +478,9 @@ static PCIDeviceInfo i440fx_info[] = {
>>          .no_hotplug   = 1,
>>          .init         = piix3_initfn,
>>          .config_write = piix3_write_config_xen,
>> +        .vendor_id    = PCI_VENDOR_ID_INTEL,
>> +        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
>> +        .class_id     = PCI_CLASS_BRIDGE_ISA,
>>      },{
>>          /* end of list */
>>      }
>
> shouldn't piix3_initfn take care of setting vendor_id, device_id and
> class_id, as in the normal PIIX3 case?

Not anymore. These ids have been removed from piix3_initfn and added
to the PCIDeviceInfo of PIIX3 in the last update.
Stefano Stabellini June 22, 2011, 6 p.m. UTC | #3 
On Wed, 22 Jun 2011, Anthony PERARD wrote:
> On Wed, Jun 22, 2011 at 18:43, Stefano Stabellini
> <stefano.stabellini@eu.citrix.com> wrote:
> > On Wed, 22 Jun 2011, anthony.perard@citrix.com wrote:
> >> From: Anthony PERARD <anthony.perard@citrix.com>
> >>
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >>  hw/piix_pci.c |    3 +++
> >>  1 files changed, 3 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> >> index 26ce904..d08b31a 100644
> >> --- a/hw/piix_pci.c
> >> +++ b/hw/piix_pci.c
> >> @@ -478,6 +478,9 @@ static PCIDeviceInfo i440fx_info[] = {
> >>          .no_hotplug   = 1,
> >>          .init         = piix3_initfn,
> >>          .config_write = piix3_write_config_xen,
> >> +        .vendor_id    = PCI_VENDOR_ID_INTEL,
> >> +        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
> >> +        .class_id     = PCI_CLASS_BRIDGE_ISA,
> >>      },{
> >>          /* end of list */
> >>      }
> >
> > shouldn't piix3_initfn take care of setting vendor_id, device_id and
> > class_id, as in the normal PIIX3 case?
> 
> Not anymore. These ids have been removed from piix3_initfn and added
> to the PCIDeviceInfo of PIIX3 in the last update.

I see. Good catch.
Alexander Graf June 30, 2011, 11:34 a.m. UTC | #4 
On 06/22/2011 05:58 PM, anthony.perard@citrix.com wrote:
> From: Anthony PERARD<anthony.perard@citrix.com>
>
> Signed-off-by: Anthony PERARD<anthony.perard@citrix.com>
> ---
>   hw/piix_pci.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index 26ce904..d08b31a 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -478,6 +478,9 @@ static PCIDeviceInfo i440fx_info[] = {
>           .no_hotplug   = 1,
>           .init         = piix3_initfn,
>           .config_write = piix3_write_config_xen,
> +        .vendor_id    = PCI_VENDOR_ID_INTEL,
> +        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
> +        .class_id     = PCI_CLASS_BRIDGE_ISA,
>       },{
>           /* end of list */
>       }

$ ./scripts/checkpatch.pl ~/patch-xen/anthony1/\[PATCH\]\ 
hw_piix_pci.c\:\ Fix\ PIIX3-xen\ to\ initialize\ ids.eml
WARNING: line over 80 characters
#70: FILE: hw/piix_pci.c:482:
+        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 
PCI-to-ISA bridge (Step A1)

ERROR: do not use C99 // comments
#70: FILE: hw/piix_pci.c:482:
+        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 
PCI-to-ISA bridge (Step A1)

total: 1 errors, 1 warnings, 9 lines checked
diff mbox

Patch

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 26ce904..d08b31a 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -478,6 +478,9 @@  static PCIDeviceInfo i440fx_info[] = {
         .no_hotplug   = 1,
         .init         = piix3_initfn,
         .config_write = piix3_write_config_xen,
+        .vendor_id    = PCI_VENDOR_ID_INTEL,
+        .device_id    = PCI_DEVICE_ID_INTEL_82371SB_0, // 82371SB PIIX3 PCI-to-ISA bridge (Step A1)
+        .class_id     = PCI_CLASS_BRIDGE_ISA,
     },{
         /* end of list */
     }