On Wednesday 02 Dec 2020 at 11:14:02 (+0000), Lukasz Luba wrote: > Hi Ionela, > > On 12/2/20 10:24 AM, Ionela Voinescu wrote: > > Hi Lukasz, > > > > On Wednesday 18 Nov 2020 at 12:03:56 (+0000), Lukasz Luba wrote: > > [snip] > > > > + struct device_node *np = NULL; > > [snip] > > > > + > > > + if (dev->of_node) > > > + np = of_node_get(dev->of_node); > > > + > > > > Should np be checked before use? I'm not sure if it's better to do the > > assign first and then the check on np before use. It depends on the > > consequences of passing a NULL node pointer later on. > > The np is actually dev->of_node (or left NULL, as set at the begging). > The only meaning of the line above is to increment the counter and then > decrement if CONFIG_OF_DYNAMIC was used. > The devfreq_cooling_register() has np = NULL and the registration can > handle it, so we should be OK here as well. >
Yes, I just wanted to make sure later registration can handle np = NULL, or whether we need to bail out. In this case, you can drop both ifs - for (dev->of_node) before get and for np before put below, as of_node_get/of_node_put can handle NULL pointers themselves. Thanks, Ionela. > > > > > + cdev = of_devfreq_cooling_register_power(np, df, dfc_power); > > > + > > > + if (np) > > > + of_node_put(np); > > > + > > [snip] > > > > > > > > Otherwise it looks good to me: > > > > Reviewed-by: Ionela Voinescu <ionela.voine...@arm.com> > > Thank you for the review. > > Regards, > Lukasz > > > > > Ionela. > > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
