[PATCH v2 6/8] pci: Add DW PCIe controller support for iMX8MP SoC

[Sumit Garg]

On Mon, 26 Feb 2024 at 14:24, Marek Vasut <marex at denx.de> wrote:
>
> On 2/26/24 9:04 AM, Sumit Garg wrote:
> > pcie_imx doesn't seem to share any useful code for iMX8 SoC and it is
> > tied to quite old port of pcie_designware driver from Linux which
> > suffices only iMX6 specific needs.
>
> It is probably hand-rolled, but it does suck, that's true.
>
> > But currently we have the common DWC specific bits which alligns pretty
> > well with DW PCIe controller on iMX8MP SoC. So lets reuse those common
> > bits instead as a new driver for iMX8 SoCs. It should be fairly easy to
> > add support for other iMX8 variants to this driver.
> >
> > iMX8MP SoC also comes up with standalone PCIe PHY support, so hence we
> > can reuse the generic PHY infrastructure to power on PCIe PHY.
>
> [...]
>
> > +static int pcie_dw_imx_of_to_plat(struct udevice *dev)
> > +{
> > +     struct pcie_dw_imx *priv = dev_get_priv(dev);
> > +     ofnode gpr;
> > +     int ret;
> > +
> > +     /* Get the controller base address */
> > +     priv->dw.dbi_base = (void *)dev_read_addr_name(dev, "dbi");
> > +     if ((fdt_addr_t)priv->dw.dbi_base == FDT_ADDR_T_NONE) {
> > +             dev_err(dev, "failed to get dbi_base address\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     /* Get the config space base address and size */
> > +     priv->dw.cfg_base = (void *)dev_read_addr_size_name(dev, "config",
> > +                                                         &priv->dw.cfg_size);
> > +     if ((fdt_addr_t)priv->dw.cfg_base == FDT_ADDR_T_NONE) {
> > +             dev_err(dev, "failed to get cfg_base address\n");
> > +             return -EINVAL;
> > +     }
> > +
> > +     ret = clk_get_bulk(dev, &priv->clks);
> > +     if (ret) {
> > +             dev_err(dev, "failed to get PCIe clks\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = reset_get_by_name(dev, "apps", &priv->apps_reset);
> > +     if (ret) {
> > +             dev_err(dev,
> > +                     "Failed to get PCIe apps reset control\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = gpio_request_by_name(dev, "reset-gpio", 0, &priv->reset_gpio,
> > +                                (GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE));
> > +     if (ret) {
> > +             dev_err(dev, "unable to get reset-gpio\n");
> > +             return ret;
> > +     }
> > +
> > +     ret = generic_phy_get_by_name(dev, "pcie-phy", &priv->phy);
> > +     if (ret) {
> > +             dev_err(dev, "failed to get pcie phy\n");
> > +             return ret;
> > +     }
> > +
> > +     gpr = ofnode_by_compatible(ofnode_null(), "fsl,imx8mp-iomuxc-gpr");
> > +     if (ofnode_equal(gpr, ofnode_null())) {
> > +             dev_err(dev, "unable to find GPR node\n");
> > +             return -ENODEV;
> > +     }
>
> You likely need a fail path here too, to undo allocation and co.

Ack.

-Sumit