[PATCH v2 3/3] of: Use scope based of_node_put() cleanups

From: Rob Herring
Date: Tue Apr 09 2024 - 15:00:35 EST

Use the relatively new scope based of_node_put() cleanup to simplify
function exit handling. Doing so reduces the chances of forgetting an
of_node_put() and simplifies error paths by avoiding the need for goto
statements.

Reviewed-by: Saravana Kannan <saravanak@xxxxxxxxxx>
Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
---
v2:
- Also use cleanup for 'dev' in __of_translate_address()
- Further simplify of_dma_is_coherent() and of_mmio_is_nonposted()
---
drivers/of/address.c | 113 +++++++++++++++++---------------------------------
drivers/of/property.c | 22 ++++------
2 files changed, 46 insertions(+), 89 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index ae46a3605904..c350185ceaeb 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -486,34 +486,30 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
* device that had registered logical PIO mapping, and the return code is
* relative to that node.
*/
-static u64 __of_translate_address(struct device_node *dev,
+static u64 __of_translate_address(struct device_node *node,
struct device_node *(*get_parent)(const struct device_node *),
const __be32 *in_addr, const char *rprop,
struct device_node **host)
{
- struct device_node *parent = NULL;
+ struct device_node *dev __free(device_node) = of_node_get(node);
+ struct device_node *parent __free(device_node) = get_parent(dev);
struct of_bus *bus, *pbus;
__be32 addr[OF_MAX_ADDR_CELLS];
int na, ns, pna, pns;
- u64 result = OF_BAD_ADDR;

pr_debug("** translation for device %pOF **\n", dev);

- /* Increase refcount at current level */
- of_node_get(dev);
-
*host = NULL;
- /* Get parent & match bus type */
- parent = get_parent(dev);
+
if (parent == NULL)
- goto bail;
+ return OF_BAD_ADDR;
bus = of_match_bus(parent);

/* Count address cells & copy address locally */
bus->count_cells(dev, &na, &ns);
if (!OF_CHECK_COUNTS(na, ns)) {
pr_debug("Bad cell count for %pOF\n", dev);
- goto bail;
+ return OF_BAD_ADDR;
}
memcpy(addr, in_addr, na * 4);

@@ -533,8 +529,7 @@ static u64 __of_translate_address(struct device_node *dev,
/* If root, we have finished */
if (parent == NULL) {
pr_debug("reached root node\n");
- result = of_read_number(addr, na);
- break;
+ return of_read_number(addr, na);
}

/*
@@ -543,11 +538,11 @@ static u64 __of_translate_address(struct device_node *dev,
*/
iorange = find_io_range_by_fwnode(&dev->fwnode);
if (iorange && (iorange->flags != LOGIC_PIO_CPU_MMIO)) {
- result = of_read_number(addr + 1, na - 1);
+ u64 result = of_read_number(addr + 1, na - 1);
pr_debug("indirectIO matched(%pOF) 0x%llx\n",
dev, result);
- *host = of_node_get(dev);
- break;
+ *host = no_free_ptr(dev);
+ return result;
}

/* Get new parent bus and counts */
@@ -555,7 +550,7 @@ static u64 __of_translate_address(struct device_node *dev,
pbus->count_cells(dev, &pna, &pns);
if (!OF_CHECK_COUNTS(pna, pns)) {
pr_err("Bad cell count for %pOF\n", dev);
- break;
+ return OF_BAD_ADDR;
}

pr_debug("parent bus is %s (na=%d, ns=%d) on %pOF\n",
@@ -563,7 +558,7 @@ static u64 __of_translate_address(struct device_node *dev,

/* Apply bus translation */
if (of_translate_one(dev, bus, pbus, addr, na, ns, pna, rprop))
- break;
+ return OF_BAD_ADDR;

/* Complete the move up one level */
na = pna;
@@ -572,11 +567,8 @@ static u64 __of_translate_address(struct device_node *dev,

of_dump_addr("one level translation:", addr, na);
}
- bail:
- of_node_put(parent);
- of_node_put(dev);

- return result;
+ return OF_BAD_ADDR;
}

u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
@@ -654,19 +646,16 @@ EXPORT_SYMBOL(of_translate_dma_address);
const __be32 *of_translate_dma_region(struct device_node *dev, const __be32 *prop,
phys_addr_t *start, size_t *length)
{
- struct device_node *parent;
+ struct device_node *parent __free(device_node) = __of_get_dma_parent(dev);
u64 address, size;
int na, ns;

- parent = __of_get_dma_parent(dev);
if (!parent)
return NULL;

na = of_bus_n_addr_cells(parent);
ns = of_bus_n_size_cells(parent);

- of_node_put(parent);
-
address = of_translate_dma_address(dev, prop);
if (address == OF_BAD_ADDR)
return NULL;
@@ -688,21 +677,19 @@ const __be32 *__of_get_address(struct device_node *dev, int index, int bar_no,
{
const __be32 *prop;
unsigned int psize;
- struct device_node *parent;
+ struct device_node *parent __free(device_node) = of_get_parent(dev);
struct of_bus *bus;
int onesize, i, na, ns;

- /* Get parent & match bus type */
- parent = of_get_parent(dev);
if (parent == NULL)
return NULL;
+
+ /* match the parent's bus type */
bus = of_match_bus(parent);
- if (strcmp(bus->name, "pci") && (bar_no >= 0)) {
- of_node_put(parent);
+ if (strcmp(bus->name, "pci") && (bar_no >= 0))
return NULL;
- }
+
bus->count_cells(dev, &na, &ns);
- of_node_put(parent);
if (!OF_CHECK_ADDR_COUNT(na))
return NULL;

@@ -888,14 +875,13 @@ static u64 of_translate_ioport(struct device_node *dev, const __be32 *in_addr,
*/
int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
{
- struct device_node *node = of_node_get(np);
+ struct device_node *node __free(device_node) = of_node_get(np);
const __be32 *ranges = NULL;
bool found_dma_ranges = false;
struct of_range_parser parser;
struct of_range range;
struct bus_dma_region *r;
int len, num_ranges = 0;
- int ret = 0;

while (node) {
ranges = of_get_property(node, "dma-ranges", &len);
@@ -905,10 +891,9 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
break;

/* Once we find 'dma-ranges', then a missing one is an error */
- if (found_dma_ranges && !ranges) {
- ret = -ENODEV;
- goto out;
- }
+ if (found_dma_ranges && !ranges)
+ return -ENODEV;
+
found_dma_ranges = true;

node = of_get_next_dma_parent(node);
@@ -916,10 +901,8 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)

if (!node || !ranges) {
pr_debug("no dma-ranges found for node(%pOF)\n", np);
- ret = -ENODEV;
- goto out;
+ return -ENODEV;
}
-
of_dma_range_parser_init(&parser, node);
for_each_of_range(&parser, &range) {
if (range.cpu_addr == OF_BAD_ADDR) {
@@ -930,16 +913,12 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
num_ranges++;
}

- if (!num_ranges) {
- ret = -EINVAL;
- goto out;
- }
+ if (!num_ranges)
+ return -EINVAL;

r = kcalloc(num_ranges + 1, sizeof(*r), GFP_KERNEL);
- if (!r) {
- ret = -ENOMEM;
- goto out;
- }
+ if (!r)
+ return -ENOMEM;

/*
* Record all info in the generic DMA ranges array for struct device,
@@ -957,9 +936,7 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map)
r->size = range.size;
r++;
}
-out:
- of_node_put(node);
- return ret;
+ return 0;
}
#endif /* CONFIG_HAS_DMA */

@@ -1016,24 +993,18 @@ phys_addr_t __init of_dma_get_max_cpu_address(struct device_node *np)
*/
bool of_dma_is_coherent(struct device_node *np)
{
- struct device_node *node;
- bool is_coherent = dma_default_coherent;
-
- node = of_node_get(np);
+ struct device_node *node __free(device_node) = of_node_get(np);

while (node) {
- if (of_property_read_bool(node, "dma-coherent")) {
- is_coherent = true;
- break;
- }
- if (of_property_read_bool(node, "dma-noncoherent")) {
- is_coherent = false;
- break;
- }
+ if (of_property_read_bool(node, "dma-coherent"))
+ return true;
+
+ if (of_property_read_bool(node, "dma-noncoherent"))
+ return false;
+
node = of_get_next_dma_parent(node);
}
- of_node_put(node);
- return is_coherent;
+ return dma_default_coherent;
}
EXPORT_SYMBOL_GPL(of_dma_is_coherent);

@@ -1049,20 +1020,14 @@ EXPORT_SYMBOL_GPL(of_dma_is_coherent);
*/
static bool of_mmio_is_nonposted(struct device_node *np)
{
- struct device_node *parent;
- bool nonposted;
-
if (!IS_ENABLED(CONFIG_ARCH_APPLE))
return false;

- parent = of_get_parent(np);
+ struct device_node *parent __free(device_node) = of_get_parent(np);
if (!parent)
return false;

- nonposted = of_property_read_bool(parent, "nonposted-mmio");
-
- of_node_put(parent);
- return nonposted;
+ return of_property_read_bool(parent, "nonposted-mmio");
}

static int __of_address_to_resource(struct device_node *dev, int index, int bar_no,
diff --git a/drivers/of/property.c b/drivers/of/property.c
index a6358ee99b74..b73daf81c99d 100644
--- a/drivers/of/property.c
+++ b/drivers/of/property.c
@@ -40,15 +40,12 @@
*/
bool of_graph_is_present(const struct device_node *node)
{
- struct device_node *ports, *port;
+ struct device_node *ports __free(device_node) = of_get_child_by_name(node, "ports");

- ports = of_get_child_by_name(node, "ports");
if (ports)
node = ports;

- port = of_get_child_by_name(node, "port");
- of_node_put(ports);
- of_node_put(port);
+ struct device_node *port __free(device_node) = of_get_child_by_name(node, "port");

return !!port;
}
@@ -610,9 +607,9 @@ EXPORT_SYMBOL(of_graph_parse_endpoint);
*/
struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
{
- struct device_node *node, *port;
+ struct device_node *port;
+ struct device_node *node __free(device_node) = of_get_child_by_name(parent, "ports");

- node = of_get_child_by_name(parent, "ports");
if (node)
parent = node;

@@ -626,8 +623,6 @@ struct device_node *of_graph_get_port_by_id(struct device_node *parent, u32 id)
break;
}

- of_node_put(node);
-
return port;
}
EXPORT_SYMBOL(of_graph_get_port_by_id);
@@ -655,14 +650,13 @@ struct device_node *of_graph_get_next_endpoint(const struct device_node *parent,
* parent port node.
*/
if (!prev) {
- struct device_node *node;
+ struct device_node *node __free(device_node) =
+ of_get_child_by_name(parent, "ports");

- node = of_get_child_by_name(parent, "ports");
if (node)
parent = node;

port = of_get_child_by_name(parent, "port");
- of_node_put(node);

if (!port) {
pr_debug("graph: no port node found in %pOF\n", parent);
@@ -1052,15 +1046,13 @@ static int of_fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
struct fwnode_endpoint *endpoint)
{
const struct device_node *node = to_of_node(fwnode);
- struct device_node *port_node = of_get_parent(node);
+ struct device_node *port_node __free(device_node) = of_get_parent(node);

endpoint->local_fwnode = fwnode;

of_property_read_u32(port_node, "reg", &endpoint->port);
of_property_read_u32(node, "reg", &endpoint->id);

- of_node_put(port_node);
-
return 0;
}


--
2.43.0