Re: [PATCH v2 2/5] x86/PCI: Support additional MMIO range capabilities

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

 



On 4/28/2014 4:19 PM, Myron Stowe wrote:
On Sat, Apr 19, 2014 at 7:52 AM, Borislav Petkov <bp@xxxxxxxxx> wrote:
On Fri, Apr 18, 2014 at 08:53:23PM -0600, Myron Stowe wrote:
From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>

.......

@@ -118,6 +154,12 @@ static int __init early_fill_mp_bus_info(void)

       pr_debug("Found AMD hostbridge at %x:%x.0\n", bus, slot);

+     /* We enable ECS mode prior to probing MMIO as MMIO related registers
+      * are in the ECS area.
+      */
+     pci_io_ecs_init(bus, slot);

Well, if enabling the ECS failed somehow, you probably want to save
yourself the _amd_read_pci_config() calls further down.

Which means:

* you should propagate an error code from that pci_io_ecs_init() function

* you can carve out the huge loop iterating over AMD_NB_F1_MMIO_RANGES
along with the pci ECS enablement call to pci_io_ecs_init into a
separate function and thus slim down that huge early_fill_mp_bus_info()
monster a bit.


Yes but this was not trivial since most, if not all, usages ignore any
return value.

I'm wanting to focus on just fixing the issue for the most part and
making as few changes as possible.  This is primarly due to:
   o  The lack of engagement by the AMD engineers (Suravee posted these
changes but has since been unresponsive),
   o  As Bjorn just mentioned, we would like to get rid of this entire
file (as we did in a similar manner with intel_bus) as it is just an
"endless treadmill" that needs attention with every new HW release,
   o  I want my name to be minimally related to this  ;)

[Suravee] Sorry for late reply and did not follow up on this patch in a timely manner. Also, thank you Myron for helping to follow up with the patch set.

Regarding the error code from pci_io_ecs_init(), it is currently always return 0 (success). I am not sure what error code should I check/propagate. Let me know if I am missing something here.

......

-static int __init pci_io_ecs_init(void)
+static int __init pci_io_ecs_init(u8 bus, u8 slot)
  {
       int cpu;

@@ -389,9 +472,7 @@ static int __init pci_io_ecs_init(void)
          if (boot_cpu_data.x86 < 0x10)
               return 0;

-     /* Try the PCI method first. */
-     if (early_pci_allowed())
-             pci_enable_pci_io_ecs();

Where did the if-check go?


Good catch.  I assume Suravee didn't drop this on purpose?

[SURAVEE] I dropped the "early_pci_allowed()" here since I have moved the calling of "pci_io_ecs_init()" into the "early_fill_mp_bus_info()" since I would like to enable PCI ecs access at earlier stage. The "early_fill_mp_bus_info() has already check for "early_pci_allowed()" already at the beginning of the function. So, this should not be needed here.

Suravee


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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