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

[Thierry Reding <thierry.reding@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2617 bytes --]

On Thu, Mar 19, 2026 at 05:14:03PM +0100, Krzysztof Kozlowski wrote:
> On 19/03/2026 17:01, Thierry Reding wrote:
> >  obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
> >  obj-$(CONFIG_PCIE_RCAR_HOST) += pcie-rcar.o pcie-rcar-host.o
> >  obj-$(CONFIG_PCIE_RCAR_EP) += pcie-rcar.o pcie-rcar-ep.o
> > diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> > new file mode 100644
> > index 000000000000..0c8351b88941
> > --- /dev/null
> > +++ b/drivers/pci/controller/pcie-tegra264.c
> > @@ -0,0 +1,506 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +// SPDX-FileCopyrightText: Copyright (c) 2022-2025, NVIDIA CORPORATION. All rights reserved.
> 
> Don't use that tag, please, but standard Copyright.

Done.

> > +static int tegra264_pcie_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct pci_host_bridge *bridge;
> > +	struct tegra264_pcie *pcie;
> > +	struct resource_entry *bus;
> > +	struct resource *res;
> > +	int err;
> > +
> > +	bridge = devm_pci_alloc_host_bridge(dev, sizeof(struct tegra264_pcie));
> > +	if (!bridge) {
> > +		dev_err(dev, "failed to allocate host bridge\n");
> > +		return -ENOMEM;
> > +	}
> > +
> > +	pcie = pci_host_bridge_priv(bridge);
> > +	platform_set_drvdata(pdev, pcie);
> > +	pcie->bridge = bridge;
> > +	pcie->dev = dev;
> > +
> > +	err = pinctrl_pm_select_default_state(dev);
> > +	if (err < 0) {
> > +		dev_err(dev, "failed to configure sideband pins: %d\n", err);
> > +		return err;
> > +	}
> > +
> > +	err = tegra264_pcie_parse_dt(pcie);
> > +	if (err < 0)
> > +		return dev_err_probe(dev, err, "failed to parse device tree");
> > +
> > +	pcie->xal = devm_platform_ioremap_resource_byname(pdev, "xal");
> > +	if (IS_ERR(pcie->xal)) {
> > +		err = PTR_ERR(pcie->xal);
> > +		dev_err(dev, "failed to map xal memory: %d\n", err);
> > +		return err;
> 
> What's with this syntax here? This is just one call return
> dev_err_probe. Looks like you are sending us some old code, instead of
> use recent drivers starting point.

It's not old code, just me being old and used to the old ways. I don't
see much point in dev_err_probe() for some of these cases because there
is no probe deferral and then dev_err_probe() is just extra overhead.
It's also much clunkier to use because it almost always requires
wrapping lines and then what's left of any benefits is pretty much
dissipated.

I get that it's the shiny new thing, but honestly, I don't think we're
doing ourselves any favours by blindly using it everywhere.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]