On Wed, 2021-03-31 at 12:25 -0700, Stephen Boyd wrote: > Quoting Geert Uytterhoeven (2021-03-31 00:05:00) > > On Wed, Mar 31, 2021 at 4:22 AM Stephen Boyd <sb...@kernel.org> wrote: > > > > > Does it have any use? > > > > > > > > of_clk_del_provider() removes the first provider found with node == > > > > NULL. > > > > If there are two drivers calling of_clk_add_hw_provider(), and one of > > > > hem calls of_clk_del_provider() later, the wrong provider may be > > > > removed from the list. > > > > > > > > > > So you're saying we shouldn't add a NULL device node pointer to the list > > > so that this can't happen? That doesn't mean returning an error from > > > of_clk_add_hw_provider() would be useful though. > > > of_clk_add_hw_provider() can return 0 if np == NULL and > > > of_clk_del_provider() can return early if np == NULL too. > > > > I don't know if I grasp all meanings of the above. > > > > The main question is if it is valid for a driver to call > > of_clk_add_hw_provider() > > with np == NULL. > > - If yes, should that register the provider? > > No it should not register the provider. That would be bad as you pointed > out. > > > - If yes, how to handle two drivers calling of_clk_add_hw_provider() > > with np = NULL, as their unregistration order is not guaranteed to > > be correct. > > > > If no, is that something to ignore (0), or a bug (error)? > > This is my question above. Is there a use to having > of_clk_add_hw_provider() return an error value when np == NULL? I doubt > it. > > Returning 0 would reduce the if conditions in driver code in this case > and be consistent with the CONFIG_OF=n inline stub that returns 0 when > CONFIG_OF is disabled. The only case an error would be returned is if we > couldn't allocate memory or if the assigned clocks code failed. Seems > sane to me. The downside is that drivers would maybe register clkdev > lookups when they don't need to and waste some memory. I'm fine with > that until we have some sort of non-DT based clk provider lookup > mechanism that could unify the two methods.
What about devm_of_clk_add_hw_provider() users, do we care that a seemingly empty managed resource will be created? Regards, Nicolas
signature.asc
Description: This is a digitally signed message part