On Sat, Jul 08, 2017 at 03:37:11PM +0200, Mathias Kresin wrote: > 08.07.2017 13:15, Jonas Gorski: > >to consumers, to have a consistent API across implementations (with > >drivers/clk/clk.c as the reference). > > > >And there don't seem that many, at least searching for clk_get_rate > >gave me only two handful of implementations of which many already > >check for NULL. > > I do not necessarily see an error in the archs legacy clock > implementation. It rather looks like it works with the common clock > framework due the lucky coincident that someone has added (an extra) > null check. > > I only had a brief look at the common clock framework, but it seams > to me one should not pass the error returned by clk_get() and/or > call clk_get_rate() at all if there was an error. That is what I've > already seen and mentioned in the commit message. > The only difference here is that we do not have the error check and > the clk_get_rate() call in the same function. rt2800_clk_is_20mhz() > isn't aware that clk_get() failed. rt2x00dev->clk is set back to > NULL in rt2x00soc_probe() in case of an clk_get error. If we need to check clk before clk_get_rate() call, we can remove NULL assignment from rt2x00soc_probe() and use IS_ERR_OR_NULL() on rt2800_clk_is_20mhz() . Thanks Stanislaw