Re: [PATCH v3 6/6] PCI: tegra: Use helper function for_each_child_of_node_scoped()

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Sat, Aug 31, 2024 at 12:04:13PM +0800, Zhang Zekun wrote:
> for_each_child_of_node_scoped() provides a scope-based cleanup
> functionality to put the device_node automatically, and we don't need to
> call of_node_put() directly.  Let's simplify the code a bit with the use
> of these functions.
> 
> Signed-off-by: Zhang Zekun <zhangzekun11@huawei.com>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> v2:
> - Use dev_err_probe() to simplify code.
> - Fix spelling error in commit message.
> 
> v3: Wrap the line which is too long.
> 
>  drivers/pci/controller/pci-tegra.c | 80 +++++++++++-------------------
>  1 file changed, 28 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c
> index d7517c3976e7..d2f29b87ffa5 100644
> --- a/drivers/pci/controller/pci-tegra.c
> +++ b/drivers/pci/controller/pci-tegra.c
> @@ -2106,47 +2106,39 @@ static int tegra_pcie_get_regulators(struct tegra_pcie *pcie, u32 lane_mask)
>  static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  {
>  	struct device *dev = pcie->dev;
> -	struct device_node *np = dev->of_node, *port;
> +	struct device_node *np = dev->of_node;
>  	const struct tegra_pcie_soc *soc = pcie->soc;
>  	u32 lanes = 0, mask = 0;
>  	unsigned int lane = 0;
>  	int err;
>  
>  	/* parse root ports */
> -	for_each_child_of_node(np, port) {
> +	for_each_child_of_node_scoped(np, port) {
>  		struct tegra_pcie_port *rp;
>  		unsigned int index;
>  		u32 value;
>  		char *label;
>  
>  		err = of_pci_get_devfn(port);
> -		if (err < 0) {
> -			dev_err(dev, "failed to parse address: %d\n", err);
> -			goto err_node_put;
> -		}
> +		if (err < 0)
> +			return dev_err_probe(dev, err, "failed to parse address\n");
>  
>  		index = PCI_SLOT(err);
>  
> -		if (index < 1 || index > soc->num_ports) {
> -			dev_err(dev, "invalid port number: %d\n", index);
> -			err = -EINVAL;
> -			goto err_node_put;
> -		}
> +		if (index < 1 || index > soc->num_ports)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "invalid port number: %d\n", index);
>  
>  		index--;
>  
>  		err = of_property_read_u32(port, "nvidia,num-lanes", &value);
> -		if (err < 0) {
> -			dev_err(dev, "failed to parse # of lanes: %d\n",
> -				err);
> -			goto err_node_put;
> -		}
> +		if (err < 0)
> +			return dev_err_probe(dev, err,
> +					     "failed to parse # of lanes\n");
>  
> -		if (value > 16) {
> -			dev_err(dev, "invalid # of lanes: %u\n", value);
> -			err = -EINVAL;
> -			goto err_node_put;
> -		}
> +		if (value > 16)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "invalid # of lanes: %u\n", value);
>  
>  		lanes |= value << (index << 3);
>  
> @@ -2159,16 +2151,12 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  		lane += value;
>  
>  		rp = devm_kzalloc(dev, sizeof(*rp), GFP_KERNEL);
> -		if (!rp) {
> -			err = -ENOMEM;
> -			goto err_node_put;
> -		}
> +		if (!rp)
> +			return -ENOMEM;
>  
>  		err = of_address_to_resource(port, 0, &rp->regs);
> -		if (err < 0) {
> -			dev_err(dev, "failed to parse address: %d\n", err);
> -			goto err_node_put;
> -		}
> +		if (err < 0)
> +			return dev_err_probe(dev, err, "failed to parse address\n");
>  
>  		INIT_LIST_HEAD(&rp->list);
>  		rp->index = index;
> @@ -2177,16 +2165,12 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  		rp->np = port;
>  
>  		rp->base = devm_pci_remap_cfg_resource(dev, &rp->regs);
> -		if (IS_ERR(rp->base)) {
> -			err = PTR_ERR(rp->base);
> -			goto err_node_put;
> -		}
> +		if (IS_ERR(rp->base))
> +			return PTR_ERR(rp->base);
>  
>  		label = devm_kasprintf(dev, GFP_KERNEL, "pex-reset-%u", index);
> -		if (!label) {
> -			err = -ENOMEM;
> -			goto err_node_put;
> -		}
> +		if (!label)
> +			return -ENOMEM;
>  
>  		/*
>  		 * Returns -ENOENT if reset-gpios property is not populated
> @@ -2199,34 +2183,26 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie)
>  						       GPIOD_OUT_LOW,
>  						       label);
>  		if (IS_ERR(rp->reset_gpio)) {
> -			if (PTR_ERR(rp->reset_gpio) == -ENOENT) {
> +			if (PTR_ERR(rp->reset_gpio) == -ENOENT)
>  				rp->reset_gpio = NULL;
> -			} else {
> -				dev_err(dev, "failed to get reset GPIO: %ld\n",
> -					PTR_ERR(rp->reset_gpio));
> -				err = PTR_ERR(rp->reset_gpio);
> -				goto err_node_put;
> -			}
> +			else
> +				return dev_err_probe(dev, PTR_ERR(rp->reset_gpio),
> +						     "failed to get reset GPIO\n");
>  		}
>  
>  		list_add_tail(&rp->list, &pcie->ports);
>  	}
>  
>  	err = tegra_pcie_get_xbar_config(pcie, lanes, &pcie->xbar_config);
> -	if (err < 0) {
> -		dev_err(dev, "invalid lane configuration\n");
> -		return err;
> -	}
> +	if (err < 0)
> +		return dev_err_probe(dev, err,
> +				     "invalid lane configuration\n");
>  
>  	err = tegra_pcie_get_regulators(pcie, mask);
>  	if (err < 0)
>  		return err;
>  
>  	return 0;
> -
> -err_node_put:
> -	of_node_put(port);
> -	return err;
>  }
>  
>  /*
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்