Hi Andy, On 03 December 2018 13:31 Andy Shevchenko wrote: > On Mon, Dec 03, 2018 at 11:13:08AM +0000, Phil Edworthy wrote: > > It's not immediately obvious from the code that failure to get a clock > > provider can return either -ENOENT or -EINVAL. Therefore, add a > > comment to highlight this. > > > +/* > > + * Beware the return values when np is valid, but no clock provider is > found. > > + * If name = NULL, the function returns -ENOENT. > > + * If name != NULL, the function returns -EINVAL. This is because > > +__of_clk_get() > > I would start new sentence from new line (this will emphasize the possible > variants) > > * This is ... I disagree, the explanation is specifically related to the case where the function returns -EINVAL. Though this is a nit, so I'm not really bothered either way.
Thanks for the review! Phil > Otherwise looks good to me: > > Reviewed-by: Andy Shevchenko <[email protected]> > > > + * is called even if of_property_match_string() returns an error. > > + */ > > static struct clk *__of_clk_get_by_name(struct device_node *np, > > const char *dev_id, > > const char *name) > > -- > > 2.17.1 > > > > -- > With Best Regards, > Andy Shevchenko >
