Re: [PATCH v2 10/10] dt-bindings: Add DSIv2 documentation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 12/02/2015 01:50 PM, Stephen Boyd wrote:
On 11/23, Archit Taneja wrote:


On 11/21/2015 1:29 AM, Rob Herring wrote:
+Stephen

On Wed, Nov 18, 2015 at 9:24 AM, Archit Taneja <architt@xxxxxxxxxxxxxx> wrote:
Hi Rob,

On 11/18/2015 6:48 PM, Rob Herring wrote:

+dt list

On Wed, Nov 18, 2015 at 4:55 AM, Archit Taneja <architt@xxxxxxxxxxxxxx>
wrote:

Add additional property info needed for DSIv2 DT.


Please use get_maintainers.pl.


Sorry about that, missed out doing that posting this time.


Signed-off-by: Archit Taneja <architt@xxxxxxxxxxxxxx>
---
   Documentation/devicetree/bindings/display/msm/dsi.txt | 10 +++++++++-
   1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/display/msm/dsi.txt
b/Documentation/devicetree/bindings/display/msm/dsi.txt
index f344b9e..ca65a34 100644
--- a/Documentation/devicetree/bindings/display/msm/dsi.txt
+++ b/Documentation/devicetree/bindings/display/msm/dsi.txt
@@ -13,18 +13,25 @@ Required properties:
   - power-domains: Should be <&mmcc MDSS_GDSC>.
   - clocks: device clocks
     See Documentation/devicetree/bindings/clocks/clock-bindings.txt for
details.
-- clock-names: the following clocks are required:
+- clock-names: these vary based on the DSI version. For DSI6G:
     * "bus_clk"
     * "byte_clk"
+  * "byte_clk_src


This sounds like the parent of byte_clk. Is that really a clock within
the block?


byte_clk_src isn't in the block, but byte_clk_src's parent is one of
the PLLs in this block. We take this clock so that we can re-parent
it to an appropriate PLL. The decision of what PLL to choose needs to
be done by the DSI block's driver.

Seems like abuse to me. The list of clocks should match what are
inputs to the block, not what the driver happens to need. Without a
full understanding of the clock tree here, I don't have a suggestion.
Maybe Stephen does.

We don't need specify byte_clk_src (and other xyz_clk_src clocks) via
DT. There is a static link set up between byte_clk and byte_clk_src by
our clock driver that never changes. We can retrieve it in the driver
itself using clk_get_parent(byte_clk). This way we stick to only
input clocks.

Stephen, does that sound okay?


I guess so. From the DT perspective it's "correct" so sure.

It would be nice if we could use assigned-clock-parents though.
As far as I can recall that's hard because the clock tree looks
like a cyclic graph when we take a clock provider level view. The
display block consumes the byte_clk and provides the source of it
too. So if we used assigned parents we would need to wait for
both clocks to be registered with the framework before we can
reconfigure the parent of byte_clk_src to be the PLL that the
display clock outputs. Unfortunately, of_clk_set_defaults() is
called during device driver probe, which in the display driver
case would be before the PLL is registered.

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.

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.

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?

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 linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux