Thread (2 messages) 2 messages, 2 authors, 2016-12-12

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.c
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help