Re: [PATCH v2 1/1] PCI: brcmstb: Fix regression regarding missing PCIe linkup

From: Thorsten Leemhuis <regressions@leemhuis.info>
To: Jim Quinlan <jim2101024@gmail.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	Nicolas Saenz Julienne <nsaenz@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	James Dutton <james.dutton@gmail.com>,
	Cyril Brulebois <kibi@debian.org>,
	bcm-kernel-feedback-list <bcm-kernel-feedback-list@broadcom.com>,
	Jim Quinlan <james.quinlan@broadcom.com>
Cc: "Florian Fainelli" <f.fainelli@gmail.com>,
	"Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE"
	<linux-rpi-kernel@lists.infradead.org>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE"
	<linux-arm-kernel@lists.infradead.org>,
	"open list" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/1] PCI: brcmstb: Fix regression regarding missing PCIe linkup
Date: Sun, 29 May 2022 19:36:24 +0200	[thread overview]
Message-ID: <ee5c8d64-c6a3-ad43-f6fc-098a2ae3dce1@leemhuis.info> (raw)
In-Reply-To: <CANCKTBvyUOHJgh1=kFdyRFxWX5D4POnn0wmHYj-eNUQEP6esvA@mail.gmail.com>

On 29.05.22 18:52, Jim Quinlan wrote:
> On Sat, May 28, 2022 at 6:44 PM Jim Quinlan <jim2101024@gmail.com> wrote:
>>
>> commit 93e41f3fca3d ("PCI: brcmstb: Add control of subdevice voltage regulators")
> 
>> Signed-off-by: Jim Quinlan <jim2101024@gmail.com>
>> introduced a regression on the PCIe RPi4 Compute Module.  If the PCIe
>> root port DT node described in [2] was missing, no linkup would be attempted,
>> and subsequent accesses would cause a panic because this particular PCIe HW
>> causes a CPU abort on illegal accesses (instead of returning 0xffffffff).
>>
>> We fix this by allowing the DT root port node to be missing, as it behaved
>> before the original patchset messed things up.
>>
>> In addition, two small changes are made:
>>
>>   1. Having pci_subdev_regulators_remove_bus() call
>>      regulator_bulk_free() in addtion to regulator_bulk_disable().
>>   2. Having brcm_pcie_add_bus() return 0 if there is an
>>      error in calling pci_subdev_regulators_add_bus().
>>      Instead, we dev_err() and turn on our refusal mode instead.
>>
>> It would be best if this commit were tested by someone with a Rpi CM4
>> platform, as that is how the regression was found.  I have only emulated
>> the problem and fix on different platform.
>>
>> Note that a bisection identified
>>
>> commit 830aa6f29f07 ("PCI: brcmstb: Split brcm_pcie_setup() into two funcs")
>>
>> as the first failing commit.  This commit is a regression, but is unrelated
>> and was fixed by a subsequent commit in the original patchset.
>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=215925
>> [2] Documentation/devicetree/bindings/pci/brcm,stb-pcie.yaml
>>
>> Fixes: 93e41f3fca3d ("PCI: brcmstb: Add control of subdevice voltage regulators")
>> Fixes: 830aa6f29f07 ("PCI: brcmstb: Split brcm_pcie_setup() into two funcs")
>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215925
> Thorston -- I forgot to replace the bugzilla link; I'll get it on V3.  -- Jim

Don't worry to much about these details that might matter for regzbot
normally (the automatic handling of links to bugzilla ticket is sadly
dysfunctional currently anyway). Just getting the issue fixed in the
not-to-distant future is what I mainly care about. :-D

Ciao
Thorsten