Re: piix4-poweroff.c I/O BAR usage

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Bjorn,

 Paul may or may not be reachable anymore, so I'll step in.

> This looks like it might be a bug:
> 
>   static const int piix4_pm_io_region = PCI_BRIDGE_RESOURCES;
> 
>   static int piix4_poweroff_probe(struct pci_dev *dev,
>                                   const struct pci_device_id *id)
>   {
>           ...
>           /* Request access to the PIIX4 PM IO registers */
>           res = pci_request_region(dev, piix4_pm_io_region,
>                                    "PIIX4 PM IO registers");
> 
> pci_request_region() takes a BAR number (0-5), but here we're passing
> PCI_BRIDGE_RESOURCES (13 if CONFIG_PCI_IOV, or 7 otherwise), which is
> the bridge I/O window.
> 
> I don't think this device ([8086:7113]) is a bridge, so that resource
> should be empty.

 Hmm, isn't the resource actually set up by `quirk_piix4_acpi' though?

> Based on this spec:
> https://www.intel.com/Assets/PDF/datasheet/290562.pdf,
> it looks like it should be the PIIX4 power management function at
> function 3, which has no standard PCI BARs but does have a PMBA (Power
> Management Base Address) at 0x40 and an SMBBA (SMBus Base Address) at
> 0x90 in config space.

 Correct, this is what Malta firmware reports for this function:

Bus = 0x00, Dev = 0x0a, Function = 0x03
Vendor Id = 0x8086 (Intel), Dev ID = 0x7113 (PIIX4 Power)
 Min Gnt = 0x00, Max Lat = 0x00, Lat Tim = 0x20
 Int Pin = None, Int Line = 0x09
 BAR count = 0x02
  IO:  Pos = 0x40, Base(CPU/PCI) = 0x18001000/0x00001000, Size = 0x00000100
  IO:  Pos = 0x90, Base(CPU/PCI) = 0x18001100/0x00001100, Size = 0x00000100

I'm somewhat familiar with this southbridge, although this was looong ago.

> I suppose on an ACPI system the regions described by PMBA and SMBBA
> might be described via ACPI, since they're not discoverable by
> standard PCI enumeration?  Pretty sure you don't have ACPI on MIPS
> though.
> 
> Maybe the driver should read PMBA and SMBBA and reserve those regions
> by hand with request_region()?

 Well, I think `quirk_piix4_acpi' covers it.  It dates back to 2.3.49 
AFAICT.  I can try to boot my Malta system over the weekend to see if 
there are any issues with it, but I'm fairly sure there is none here.

  Maciej



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux