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

Reply via email to