On 12/03/2015 12:46 PM, Stephen Boyd wrote:
On 12/02, Archit Taneja wrote:
On 12/02/2015 01:50 PM, Stephen Boyd wrote:
My only thought there would be to make of_clk_set_defaults() wait
until both clocks are registered before it does any parent
setting. But only in the case where the assigned parents contains
a clock that is provided by the node being processed. I suppose
the simplest thing to do would be to skip it during the device
driver probe and handle it when the clk provider is registered.
The assigned-clock-parents stuff you mentioned is needed to set a
default link between the one of the DSI PLLs and the RCG, right? I
just wanted to make clear if we were still discussing the same
issue.
Yes.
From what I understand, we don't need the assigned-clock-parents stuff
to establish a link between byte_clk_src(RCG clock) and
byte_clk(branch clock). That's a fixed link set up by the clock
structs provided in the gcc driver and doesn't need to be specially
assigned, and just a
clk_get_parent in the driver does the job there.
There's only one parent of the byte_clk and that's byte_clk_src.
So yes, there's no need to describe that in DT and
clk_get_parent() works fine.
About assigning a parent to the RCG, wouldn't that be xo by default, and
changed by the drm/msm driver to one of the PLLs when the need arrives?
I didn't get why we need to establish that beforehand in DT?
Yes, it would be XO out of reset, but we have no idea what the
bootloader is doing. I thought the problem was that byte_clk_src
is not actually an input to the DSI device. The proposal was to
have DT specify byte_clk_src and byte_clk in the clocks array so
that byte_clk_src could be reparented to the PLL and the byte_clk
could be enabled/disabled. If we use DT to do the parent
configuring then the DSI node doesn't have the byte_clk_src in
its clocks array and thus DT is reflecting reality.
Okay, I understand your point now.
If we want to dynamically switch the parent of byte_clk_src to be
different PLLs at runtime, then yes we'll need to get the parent
of the byte_clk (which is byte_clk_src) and set the PLL as the
parent. Or we'll need to make clk_set_parent() on the byte_clk
transparently set the grand-parent to be the PLL. In that case we
may need to introduce some sort of flag like
CLK_SET_PARENT_GRANDPARENT to add this type of behavior.
I don't have a huge problem with
clk_set_parent(clk_get_parent(byte_clk), PLL)
except that this fails the abstraction test. It leaks information
about the clock tree into a driver that shouldn't need to know
that on this particular SoC there's a clock in between the PLL
and the byte_clk. Future designs may not have the intermediate
clock and then we'll need to update the driver to handle the
difference, when we could have added the flag and things would
work the same.
I will have to check if we really require dynamic PLL configuration
or not. At the moment, the driver reads another DT param (that
identifies if we are in dual dsi mode) and then sets the appropriate
PLL as parent. So, in a sense, it is relying on DT already for setting
the parent.
I guess we can do the following:
- use assigned-clock-parents in DT to set the default PLL
parent (provided it works out of the box)
- Within the driver, still do the
'clk_set_parent(clk_get_parent(byte_clk), PLL)' thing and
remove it later if dynamic switching isn't needed at all
if the parent PLL is known beforehand in all use cases.
Archit
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, hosted by The Linux Foundation
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
