| Message ID | 1444595260-3332-1-git-send-email-christophe.jaillet@wanadoo.fr (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Sun, 11 Oct 2015, Christophe JAILLET wrote: > of_get_next_parent can be used to simplify the while() loop and > avoid the need of a temp variable. Can you do something with the loop in __of_translate_address, in drivers/of/address.c? Is there not an iterator for this? julia > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > arch/powerpc/sysdev/mpc5xxx_clocks.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c b/arch/powerpc/sysdev/mpc5xxx_clocks.c > index f4f0301..5732926 100644 > --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c > +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c > @@ -13,7 +13,6 @@ > > unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) > { > - struct device_node *np; > const unsigned int *p_bus_freq = NULL; > > of_node_get(node); > @@ -22,9 +21,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) > if (p_bus_freq) > break; > > - np = of_get_parent(node); > - of_node_put(node); > - node = np; > + node = of_get_next_parent(node); > } > of_node_put(node); > > -- > 2.1.4 > > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
Le 11/10/2015 22:44, Julia Lawall a écrit : > >> of_get_next_parent can be used to simplify the while() loop and >> avoid the need of a temp variable. > Can you do something with the loop in __of_translate_address, in > drivers/of/address.c? Is there not an iterator for this? > > julia > Hi Julia, There does not seem to be any 'for_each_parent_of_node' or equivalent. Best regards, CJ
On Sun, 2015-11-10 at 20:27:40 UTC, Christophe Jaillet wrote: > of_get_next_parent can be used to simplify the while() loop and > avoid the need of a temp variable. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> > --- > arch/powerpc/sysdev/mpc5xxx_clocks.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c b/arch/powerpc/sysdev/mpc5xxx_clocks.c > index f4f0301..5732926 100644 > --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c > +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c > @@ -13,7 +13,6 @@ > > unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) > { > - struct device_node *np; > const unsigned int *p_bus_freq = NULL; > > of_node_get(node); > @@ -22,9 +21,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) > if (p_bus_freq) > break; > > - np = of_get_parent(node); > - of_node_put(node); > - node = np; > + node = of_get_next_parent(node); > } > of_node_put(node); This conversion is OK, but the logic in the function is still wrong. It uses of_get_property() inside the loop, but then drops the reference to the node before dereferencing the p_bus_freq pointer, which could by then point to junk if the node has been freed. Instead it should use of_property_read_u32() to actually read the property value before dropping the reference. cheers
On Sun, 2015-11-10 at 20:27:40 UTC, Christophe Jaillet wrote: > of_get_next_parent can be used to simplify the while() loop and > avoid the need of a temp variable. > > Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> Applied to powerpc next, thanks. https://git.kernel.org/powerpc/c/b340587e68b479e52039f800 cheers
diff --git a/arch/powerpc/sysdev/mpc5xxx_clocks.c b/arch/powerpc/sysdev/mpc5xxx_clocks.c index f4f0301..5732926 100644 --- a/arch/powerpc/sysdev/mpc5xxx_clocks.c +++ b/arch/powerpc/sysdev/mpc5xxx_clocks.c @@ -13,7 +13,6 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) { - struct device_node *np; const unsigned int *p_bus_freq = NULL; of_node_get(node); @@ -22,9 +21,7 @@ unsigned long mpc5xxx_get_bus_frequency(struct device_node *node) if (p_bus_freq) break; - np = of_get_parent(node); - of_node_put(node); - node = np; + node = of_get_next_parent(node); } of_node_put(node);
of_get_next_parent can be used to simplify the while() loop and avoid the need of a temp variable. Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr> --- arch/powerpc/sysdev/mpc5xxx_clocks.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)