| Message ID | 20240830035819.13718-7-zhangzekun11@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Simplify code with _scoped() helper functions | expand |
On Fri, 30 Aug 2024 11:58:19 +0800 Zhang Zekun <zhangzekun11@huawei.com> 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> One coding style thing + some long lines that could do with wrapping. With those tidied up. Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > --- > v2: > - Use dev_err_probe() to simplify code. > - Fix spelling error in commit message. > > drivers/pci/controller/pci-tegra.c | 74 ++++++++++-------------------- > 1 file changed, 23 insertions(+), 51 deletions(-) > > diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c > index d7517c3976e7..94a768a4616d 100644 > --- a/drivers/pci/controller/pci-tegra.c > +++ b/drivers/pci/controller/pci-tegra.c > @@ -2106,47 +2106,36 @@ 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); Long line that I'd wrap. Same for other similar cases. > > lanes |= value << (index << 3); > > @@ -2201,32 +2182,23 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) > if (IS_ERR(rp->reset_gpio)) { > if (PTR_ERR(rp->reset_gpio) == -ENOENT) { This { is only here because of the coding style rule about being consistent. You aren't any more as > 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 this is never valid under the kernel coding style. However, you don't need the {} at all any more as all branches are if the if / else are single lines. > + return dev_err_probe(dev, PTR_ERR(rp->reset_gpio), > + "failed to get reset GPIO\n"); > }
diff --git a/drivers/pci/controller/pci-tegra.c b/drivers/pci/controller/pci-tegra.c index d7517c3976e7..94a768a4616d 100644 --- a/drivers/pci/controller/pci-tegra.c +++ b/drivers/pci/controller/pci-tegra.c @@ -2106,47 +2106,36 @@ 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 +2148,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 +2162,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 @@ -2201,32 +2182,23 @@ static int tegra_pcie_parse_dt(struct tegra_pcie *pcie) if (IS_ERR(rp->reset_gpio)) { 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; } /*
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> --- v2: - Use dev_err_probe() to simplify code. - Fix spelling error in commit message. drivers/pci/controller/pci-tegra.c | 74 ++++++++++-------------------- 1 file changed, 23 insertions(+), 51 deletions(-)