Messages in this thread | | | Date | Fri, 1 Jul 2022 13:58:51 -0700 | Subject | Re: [PATCH v1 0/4] PCI: brcmstb: Re-submit reverted patchset | From | Florian Fainelli <> |
| |
On 7/1/22 09:27, Jim Quinlan wrote: > A submission [1] was made to enable a PCIe root port to turn on regulators > for downstream devices. It was accepted. Months later, a regression was > discovered on an RPi CM4 [2]. The patchset was reverted [3] as the fix > came too late in the release cycle. The regression in question is > triggered only when the PCIe RC DT node has no root port subnode, which is > a perfectly reasonsable configuration. > > The original commits are now being resubmitted with some modifications to > fix the regression. The modifcations on the original commits are > described below (the SHA is that of the original commit): > > [830aa6f29f07 PCI: brcmstb: Split brcm_pcie_setup() into two funcs] > NOTE: In the originally submitted patchset, this commit introduced a > regression that was corrected by a subsequent commit in the same > patchset. Let's not do this again. > > @@ -1411,6 +1411,10 @@ static int brcm_pcie_probe(struct platform_device *pdev) > if (ret) > goto fail; > > + ret = brcm_pcie_linkup(pcie); > + if (ret) > + goto fail; > > > [67211aadcb4b PCI: brcmstb: Add mechanism to turn on subdev regulators] > NOTE: Not related to the regression, the regulators must be freed whenever > the PCIe tree is dismantled: > > @@ -507,6 +507,7 @@ static void pci_subdev_regulators_remove_bus(struct pci_bus *bus) > > if (regulator_bulk_disable(sr->num_supplies, sr->supplies)) > dev_err(dev, "failed to disable regulators for downstream device\n"); > + regulator_bulk_free(sr->num_supplies, sr->supplies); > dev->driver_data = NULL; > > > [93e41f3fca3d PCI: brcmstb: Add control of subdevice voltage regulators] > NOTE: If the PCIe RC DT node was missing a Root Port subnode, the PCIe > link-up was skipped. This is the regression. Fix it by attempting > link-up even if the Root Port DT subnode is missing. > > @@ -503,11 +503,10 @@ static int pci_subdev_regulators_add_bus(struct pci_bus *bus) > > static int brcm_pcie_add_bus(struct pci_bus *bus) > { > - struct device *dev = &bus->dev; > struct brcm_pcie *pcie = (struct brcm_pcie *) bus->sysdata; > int ret; > > - if (!dev->of_node || !bus->parent || !pci_is_root_bus(bus->parent)) > + if (!bus->parent || !pci_is_root_bus(bus->parent)) > return 0; > > ret = pci_subdev_regulators_add_bus(bus); > > [1] https://lore.kernel.org/r/20220106160332.2143-1-jim2101024@gmail.com > [2] https://bugzilla.kernel.org/show_bug.cgi?id=215925 > [3] https://lore.kernel.org/linux-pci/20220511201856.808690-1-helgaas@kernel.org/
On a Raspberry Pi 4B:
Tested-by: Florian Fainelli <f.fainelli@gmail.com> -- Florian
| |