08.07.2017 13:15, Jonas Gorski:

Hi,

On 8 July 2017 at 09:27, Mathias Kresin [off-list ref] wrote:

quoted

If clk_get returns an error, rt2x00dev->clk is set to NULL. In
contrast to the common clock framework provided clk_get_rate(), at
least the ramips and bcm63xx legacy implementation of the clk API
access the rate member of the clk struct without a NULL check. This
results into a kernel panic if we do not have a (SoC) clock.

Call clk_get_rate only if we have a clock to fix the issues. This
approach is similar to what is done in the kernel at various places.
Usually clk_get_rate() is only called if clk_get_rate() doesn't return

Did you mean clk_get() as the second one?

Yes I do. ... is only called if clk_get() doesn't return an error.

quoted

an error.

Signed-off-by: Mathias Kresin <redacted>

Tbh, I'd rather have this fixed at the source than adding a workaround
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.

Mathias