Re: [PATCH] Fix returns of some CLK API calls, if !CONFIG_HAVE_CLOCK

[Tom Bogendoerfer]

On Tue, Apr 25, 2017 at 05:31:37PM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 25, 2017 at 02:30:07PM +0200, Thomas Bogendoerfer wrote:
> > If CONFIG_HAVE_CLOCK is not set, return values of clk_get(),
> > devm_clk_get(), devm_get_clk_from_child(), clk_get_parent()
> > and clk_get_sys() are wrong. According to spec these functions
> > should either return a pointer to a struct clk or a valid IS_ERR
> > condition. NULL is neither, so returning ERR_PTR(-ENODEV) makes
> > more sense.
> 
> That's wrong.  When the clk API is disabled, the expected behaviour is
> that drivers will not fail.  Returning ERR_PTR(-ENODEV) will cause them
> to fail, so will break platforms.
>
> NAK.
> 

then we have to go through all drivers, which could work with and
without HAVE_CLK and replace the IS_ERR() checks with something
like IS_ERR_OR_NULL().  Easy for the part I'm interested in the first place.

> > Without this change serial console on SNI RM400 machines (MIPS arch)
> > is broken, because sccnxp driver doesn't get a valid clock rate.
> 
> So the driver needs to depeond on HAVE_CLOCK.

the driver worked without HAVE_CLOCK before just fine, and while it
got invaded by CLK API it got broken.

No problem to fix that, but just looking at include/linux/clk.h:

/**
 * devm_clk_get - lookup and obtain a managed reference to a clock producer.
 * @dev: device for clock "consumer"
 * @id: clock consumer ID
 *
 * Returns a struct clk corresponding to the clock producer, or
 * valid IS_ERR() condition containing errno.  The implementation

I would expect either no replacement for that, if !HAVE_CLOCK
or simple a sane result code... and NULL isn't sane at least
with the description above...

But still it's your call.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]