Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220321104843.949645-1-maz@kernel.org>
In-Reply-To: <20220321104843.949645-1-maz@kernel.org>
From:   Rob Herring <robh@kernel.org>
Date:   Mon, 21 Mar 2022 10:17:34 -0500
Message-ID: <CAL_JsqJacC6GbNebTfYyUEScROCFN4+Fg2v1_iYFfqAvW4E9Vw@mail.gmail.com>
Subject: Re: [PATCH v2 0/2] PCI: xgene: Restore working PCIe functionnality
To:     Marc Zyngier <maz@kernel.org>
Cc:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        PCI <linux-pci@vger.kernel.org>,
        Toan Le <toan@os.amperecomputing.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        =?UTF-8?Q?St=C3=A9phane_Graber?= <stgraber@ubuntu.com>,
        dann frazier <dann.frazier@canonical.com>,
        Android Kernel Team <kernel-team@android.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Mar 21, 2022 at 5:49 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Since 6dce5aa59e0b ("PCI: xgene: Use inbound resources for setup") was
> merged in the 5.5 time frame, PCIe on the venerable XGene platform has
> been unusable: 6dce5aa59e0b broke both XGene-1 (Mustang and m400) and
> XGene-2 (Merlin), while the addition of c7a75d07827a ("PCI: xgene: Fix
> IB window setup") fixed XGene-2, but left the rest of the zoo
> unusable.
>
> It is understood that this systems come with "creative" DTs that don't
> match the expectations of modern kernels. However, there is little to
> be gained by forcing these changes on users -- the firmware is not
> upgradable, and the current owner of the IP will deny that these
> machines have ever existed.

The gain for fixing this properly is not having drivers do their own
dma-ranges parsing. We've seen what happens when drivers do their own
parsing of standard properties (e.g. interrupt-map). Currently, we
don't have any drivers doing their own parsing:

$ git grep of_pci_dma_range_parser_init
drivers/of/address.c:int of_pci_dma_range_parser_init(struct
of_pci_range_parser *parser,
drivers/of/address.c:EXPORT_SYMBOL_GPL(of_pci_dma_range_parser_init);
drivers/of/address.c:#define of_dma_range_parser_init
of_pci_dma_range_parser_init
drivers/of/unittest.c:  if (of_pci_dma_range_parser_init(&parser, np)) {
drivers/pci/of.c:       err = of_pci_dma_range_parser_init(&parser, dev_node);
include/linux/of_address.h:extern int
of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,
include/linux/of_address.h:static inline int
of_pci_dma_range_parser_init(struct of_pci_range_parser *parser,

And we can probably further refactor this to be private to drivers/pci/of.c.

For XGene-2 the issue is simply that the driver depends on the order
of dma-ranges entries.

For XGene-1, I'd still like to understand what the issue is. Reverting
the first fix and fixing 'dma-ranges' should have fixed it. I need a
dump of how the IB registers are initialized in both cases. I'm not
saying changing 'dma-ranges' in the firmware is going to be required
here. There's a couple of other ways we could fix that without a
firmware change, but first I need to understand why it broke.

Rob

P.S. We're carrying ACPI and DT support for these platforms. It seems
the few users are using DT, so can we drop the ACPI support? Or do I
need to break it first and wait a year? ;)