Re: [PATCH 3/3] clk: keystone: Add sci-clk driver support
From: Stephen Boyd <hidden>
Date: 2016-12-12 19:38:02
Also in:
linux-arm-kernel, linux-clk
On 12/09, Tero Kristo wrote:
On 08/12/16 23:10, Stephen Boyd wrote:quoted
On 12/08, Tero Kristo wrote:quoted
On 08/12/16 02:13, Stephen Boyd wrote:quoted
On 10/21, Tero Kristo wrote:quoted
diff --git a/drivers/clk/keystone/sci-clk.c b/drivers/clk/keystone/sci-clk.c new file mode 100644 index 0000000..f6af5bd --- /dev/null +++ b/drivers/clk/keystone/sci-clk.cquoted
quoted
+ + handle = devm_ti_sci_get_handle(dev); + if (IS_ERR(handle)) + return PTR_ERR(handle); + + provider = devm_kzalloc(dev, sizeof(*provider), GFP_KERNEL); + if (!provider) + return -ENOMEM; + + provider->clocks = data; + + provider->sci = handle; + provider->ops = &handle->ops.clk_ops; + provider->dev = dev; + + ti_sci_init_clocks(provider);And if this fails?Yea this is kind of controversial. ti_sci_init_clocks() can fail if any of the clocks registered will fail. I decided to have it this way so that at least some clocks might work in failure cause, and you might have a booting device instead of total lock-up. Obviously it could be done so that if any clock fails, we would de-register all clocks at that point, but personally I think this is a worse option. ti_sci_init_clocks could probably be modified to continue registering clocks when a single clock fails though. Currently it aborts at first failure.That sounds like a better approach if we don't care about failures to register a clock. Returning a value from a function and not using it isn't really a great design. I worry that if we start returning errors from clk_hw_register() that something will go wrong though, so really I don't know why we want to ignore errors at all. Just for debugging a boot hang? Can't we use early console to at least see that this driver is failing to probe and debug that way?Early console can be used to debug that, but it is kind of annoying to recompile most of the kernel when you suddenly need to use it.
I thought SERIAL_EARLYCON was selected by drivers that support it? So there shouldn't be any rebuilding required.
How about modifying the ti_sci_init_clocks func to print an error for each failed clock?
Ok that's fine too. I'd prefer the function had a return type of void if we're not planning on using the return value, that's all. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project