| Message ID | 20241011195622.6349-8-rosenp@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ibm: emac: more cleanups | expand |
On Fri, Oct 11, 2024 at 12:56:22PM -0700, Rosen Penev wrote: > Cleaner than using of_find_all_nodes and then of_match_node. > > Also modified EMAC_BOOT_LIST_SIZE check to run before of_node_get to > avoid having to call of_node_put on failure. > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > --- > drivers/net/ethernet/ibm/emac/core.c | 10 +++------- > 1 file changed, 3 insertions(+), 7 deletions(-) > > diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c > index faa483790b29..5265616400c2 100644 > --- a/drivers/net/ethernet/ibm/emac/core.c > +++ b/drivers/net/ethernet/ibm/emac/core.c > @@ -3253,21 +3253,17 @@ static void __init emac_make_bootlist(void) > int cell_indices[EMAC_BOOT_LIST_SIZE]; > > /* Collect EMACs */ > - while((np = of_find_all_nodes(np)) != NULL) { > + while((np = of_find_matching_node(np, emac_match))) { > u32 idx; > > - if (of_match_node(emac_match, np) == NULL) > - continue; > if (of_property_read_bool(np, "unused")) > continue; > if (of_property_read_u32(np, "cell-index", &idx)) > continue; > cell_indices[i] = idx; > - emac_boot_list[i++] = of_node_get(np); > - if (i >= EMAC_BOOT_LIST_SIZE) { > - of_node_put(np); > + if (i >= EMAC_BOOT_LIST_SIZE) > break; > - } > + emac_boot_list[i++] = of_node_get(np); Reading the Kernel doc for of_find_matching_node() it seems that of_node_put() needs to called each time it (and thus of_find_matching_node() returns a np. But that doesn't seem to be the case here. Am I mistaken? > } > max = i; > > -- > 2.47.0 >
On Sat, Oct 12, 2024 at 6:21 AM Simon Horman <horms@kernel.org> wrote: > > On Fri, Oct 11, 2024 at 12:56:22PM -0700, Rosen Penev wrote: > > Cleaner than using of_find_all_nodes and then of_match_node. > > > > Also modified EMAC_BOOT_LIST_SIZE check to run before of_node_get to > > avoid having to call of_node_put on failure. > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com> > > --- > > drivers/net/ethernet/ibm/emac/core.c | 10 +++------- > > 1 file changed, 3 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c > > index faa483790b29..5265616400c2 100644 > > --- a/drivers/net/ethernet/ibm/emac/core.c > > +++ b/drivers/net/ethernet/ibm/emac/core.c > > @@ -3253,21 +3253,17 @@ static void __init emac_make_bootlist(void) > > int cell_indices[EMAC_BOOT_LIST_SIZE]; > > > > /* Collect EMACs */ > > - while((np = of_find_all_nodes(np)) != NULL) { > > + while((np = of_find_matching_node(np, emac_match))) { > > u32 idx; > > > > - if (of_match_node(emac_match, np) == NULL) > > - continue; > > if (of_property_read_bool(np, "unused")) > > continue; > > if (of_property_read_u32(np, "cell-index", &idx)) > > continue; > > cell_indices[i] = idx; > > - emac_boot_list[i++] = of_node_get(np); > > - if (i >= EMAC_BOOT_LIST_SIZE) { > > - of_node_put(np); > > + if (i >= EMAC_BOOT_LIST_SIZE) > > break; > > - } > > + emac_boot_list[i++] = of_node_get(np); > > Reading the Kernel doc for of_find_matching_node() it seems > that of_node_put() needs to called each time it (and thus > of_find_matching_node() returns a np. But that doesn't seem > to be the case here. Am I mistaken? Bad change. Will remove. > > > > } > > max = i; > > > > -- > > 2.47.0 > >
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c index faa483790b29..5265616400c2 100644 --- a/drivers/net/ethernet/ibm/emac/core.c +++ b/drivers/net/ethernet/ibm/emac/core.c @@ -3253,21 +3253,17 @@ static void __init emac_make_bootlist(void) int cell_indices[EMAC_BOOT_LIST_SIZE]; /* Collect EMACs */ - while((np = of_find_all_nodes(np)) != NULL) { + while((np = of_find_matching_node(np, emac_match))) { u32 idx; - if (of_match_node(emac_match, np) == NULL) - continue; if (of_property_read_bool(np, "unused")) continue; if (of_property_read_u32(np, "cell-index", &idx)) continue; cell_indices[i] = idx; - emac_boot_list[i++] = of_node_get(np); - if (i >= EMAC_BOOT_LIST_SIZE) { - of_node_put(np); + if (i >= EMAC_BOOT_LIST_SIZE) break; - } + emac_boot_list[i++] = of_node_get(np); } max = i;
Cleaner than using of_find_all_nodes and then of_match_node. Also modified EMAC_BOOT_LIST_SIZE check to run before of_node_get to avoid having to call of_node_put on failure. Signed-off-by: Rosen Penev <rosenp@gmail.com> --- drivers/net/ethernet/ibm/emac/core.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)