Re: [PATCH v2 4/5] PCI: tegra: Add Tegra264 support

Mikko Perttunen
2026-03-25T03:49:26Z

On Fri, 20 Mar 2026 23:54:36 +0100, Thierry Reding <thierry.reding@kernel.org> wrote:
> diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> new file mode 100644
> index 000000000000..3ce1ad971bdb
> --- /dev/null
> +++ b/drivers/pci/controller/pcie-tegra264.c
> @@ -0,0 +1,527 @@
> [ ... skip 123 lines ... ]
> + err = tegra_bpmp_transfer(pcie->bpmp, &msg);
> + if (err)
> + dev_info(pcie->dev, "failed to turn off PCIe #%u: %pe\n",
> + pcie->ctl_id, ERR_PTR(err));
> +
> + if (msg.rx.ret)
> + dev_info(pcie->dev, "failed to turn off PCIe #%u: %d\n",
> + pcie->ctl_id, msg.rx.ret);
> +}

Ideally we would distinguish by message in these two cases. I suppose
the %pe vs. %d does that, but it's not quite obvious.

> +
> +static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
> +{
> + u32 value, speed, width, bw;
> + int err;
> +
> + value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
> + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
> +
> + bw = width * (PCIE_SPEED2MBS_ENC(speed) / BITS_PER_BYTE);

PCIE_SPEED2MBS_ENC takes enum pci_bus_speed values rather than
LNKSTA_CLS values. Perhaps it should be converted to a static inline
function to catch such issues.

> + value = MBps_to_icc(bw);

The result of this is unused.

> [ ... skip 271 lines ... ]
> + pcie->ecam = pcie->cfg->win;
> +
> + tegra264_pcie_init(pcie);
> +
> + if (!pcie->link_up)
> + goto free;

This path causes things to be freed, but the driver still probes. The
remove callback will then try to remove things again.

Thanks
Mikko

--