Hi Joshua,
First of all, apologies for breaking IP27 and the inconvenience this
caused you. And thanks a lot for tracking down the problem and
proposing a solution!
On Tue, Feb 7, 2017 at 12:13 AM, Joshua Kinard <kumba@xxxxxxxxxx> wrote:
> From: Joshua Kinard <kumba@xxxxxxxxxx>
>
> Commit 046136170a56 changed things such that setting PCI_PROBE_ONLY
> will now explicitly claim PCI resources instead of skipping the sizing
> of the bridges and assigning resources. This is okay for IP30's PCI
> code, which doesn't use physical address space to access I/O resources.
>
> However, IP27 is completely different in this regard. Instead of using
> ioremapped addresses for I/O, IP27 has a dedicated address range,
> 0x92xxxxxxxxxxxxxx, that is used for all I/O access. Since this is
> uncached physical address space, the generic MIPS PCI code will not
> probe it correctly and thus, the original behavior of PCI_PROBE_ONLY
> needs to be restored only for the IP27 platform to bypass this logic
> and have working PCI, at least for the IO6/IO6G board that houses the
> base devices, until a better solution is found.
It sounds like there's something different about how ioremap() works
on these platforms and PCI probing is tripping over that. I'd really
like to understand more about this difference to see if we can
converge that instead of adding back the PCI_PROBE_ONLY usage.
Drivers shouldn't know whether they're running on IP27 or IP30, and
they should be using ioremap() in both cases. Does ioremap() work
differently on IP27 and IP30? Does this have something to do with
plat_ioremap() or fixup_bigphys_addr()?
Is there any chance you can collect complete dmesg logs and
/proc/iomem contents from IP27 and IP30? Maybe "lspci -vv" output,
too? I'm not sure where to look to understand the ioremap() behavior.
What exactly is the PCI probe failure without this patch? If you have
a console log (with "ignore_loglevel") it might have a clue, too.
> Fixes: 046136170a56 ("MIPS/PCI: Claim bus resources on PCI_PROBE_ONLY set-ups")
> Signed-off-by: Joshua Kinard <kumba@xxxxxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Cc: Thomas Bogendoerfer <tsbogend@xxxxxxxxxxxxxxxx>
> ---
> arch/mips/pci/pci-bridge.c | 15 +++++++++++++++
> arch/mips/pci/pci-legacy.c | 15 +++++++++++++++
> 2 files changed, 30 insertions(+)
>
> diff --git a/arch/mips/pci/pci-bridge.c b/arch/mips/pci/pci-bridge.c
> index 9df13ce313b5..af7073dba36b 100644
> --- a/arch/mips/pci/pci-bridge.c
> +++ b/arch/mips/pci/pci-bridge.c
> @@ -62,6 +62,21 @@ bridge_probe(nasid_t nasid, int widget_id, int masterwid)
> unsigned long offset = NODE_OFFSET(nasid);
> struct bridge_controller *bc;
>
> +#ifdef CONFIG_SGI_IP27
I don't know how MIPS multi-platform support works. If you enable
CONFIG_SGI_IP27, does that mean the resulting kernel will *only* run
on IP27? Or can you enable several platforms, e.g., SGI_IP22,
SGI_IP27, SGI_IP28, SGI_IP32, etc., and end up with a kernel that will
boot on any of those platforms? From Kconfig, it looks like these
options are not mutually exclusive, so my guess is maybe the latter?
If so, I would think whatever we do should be based on a run-time test
for SGI_IP27 instead of a compile-time test.
> + /*
> + * Commit 046136170a56 changed things such that setting PCI_PROBE_ONLY
> + * will now explicitly claim PCI resources instead of skipping the
> + * sizing of the bridges and assigning resources. This is okay for
> + * the IP30's PCI code, which uses normal, ioremapped addresses to
> + * do I/O. IP27, however, is different and uses a hardware-specific
> + * address range of 0x92xxxxxxxxxxxxxx for all I/O access. As such,
> + * the generic MIPS PCI code will not probe correctly and thus make
> + * PCI on IP27 completely unusable. Thus, we must restore the
> + * original logic only for IP27 until a better solution can be found.
> + */
> + pci_set_flags(PCI_PROBE_ONLY);
> +#endif
> +
> /* XXX: Temporary until the IP27 "mega update". */
> bc = &bridges[num_bridges];
> if (!num_bridges)
> diff --git a/arch/mips/pci/pci-legacy.c b/arch/mips/pci/pci-legacy.c
> index 68268bbb15b8..5590af4f367f 100644
> --- a/arch/mips/pci/pci-legacy.c
> +++ b/arch/mips/pci/pci-legacy.c
> @@ -107,6 +107,20 @@ static void pcibios_scanbus(struct pci_controller *hose)
> need_domain_info = 1;
> }
>
> +#ifdef CONFIG_SGI_IP27
> + /*
> + * Commit 046136170a56 changed things such that setting PCI_PROBE_ONLY
> + * will now explicitly claim PCI resources instead of skipping the
> + * sizing of the bridges and assigning resources. This is okay for
> + * the IP30's PCI code, which uses normal, ioremapped addresses to
> + * do I/O. IP27, however, is different and uses a hardware-specific
> + * address range of 0x92xxxxxxxxxxxxxx for all I/O access. As such,
> + * the generic MIPS PCI code will not probe correctly and thus make
> + * PCI on IP27 completely unusable. Thus, we must restore the
> + * original logic only for IP27 until a better solution can be found.
> + */
> + if (!pci_has_flag(PCI_PROBE_ONLY)) {
> +#else
> /*
> * We insert PCI resources into the iomem_resource and
> * ioport_resource trees in either pci_bus_claim_resources()
> @@ -115,6 +129,7 @@ static void pcibios_scanbus(struct pci_controller *hose)
> if (pci_has_flag(PCI_PROBE_ONLY)) {
> pci_bus_claim_resources(bus);
> } else {
> +#endif
> pci_bus_size_bridges(bus);
> pci_bus_assign_resources(bus);
> }
> --
> 2.11.1
>
