Re: [PATCH v4 01/20] dt-bindings: phy: Add starfive,jh7110-inno-hdmi-phy

From
Krzysztof Kozlowski <krzk@kernel.org>
Date
2026-10-04 07:15:28
Also in
dri-devel, linux-clk, linux-devicetree, linux-phy, linux-riscv, linux-rockchip, lkml
Reply to
Michal Wilczynski, 2026-10-03
On 04/10/2026 00:35, Michal Wilczynski wrote:

On 10/3/26 22:45, Krzysztof Kozlowski wrote:
quoted
On 03/10/2026 17:36, Michal Wilczynski wrote:
quoted

On 9/30/26 13:01, Krzysztof Kozlowski wrote:
quoted
On 25/09/2026 23:05, Michal Wilczynski wrote:
quoted
quoted
quoted
+  clocks:
+    maxItems: 1
+    description: Reference oscillator.
This barely counts as a resource, so usual question: no resources here?
no MMIO? Even the user of this phy is the block itself.

This makes me wonder if this should be a device node in the first place
(instead folded into the parent).
The PHY has no reg because the reg is shared with the controller and
owned by the parent - patch 9 lets the bridge take its regmap from
there.

The user of the PHY is not only the block itself. It is the pixel clock
provider for the whole display subsystem, voutcrg takes hdmitx0_pixelclk
as the parent of its DC8200 pixel MUXes, and while HDMI output is active
it is the only intended source for that clock. The parent has to be
assigned explicitly so the general PLL does not end up driving the pixel
clock, and so a DSI user does not reach the HDMI PHY clock generator.

So it has to be its own node. The HDMI block has two independent
I do not see the logic which lead to this conclusion. Pixel clock
provider, so a clock controller, cannot be a user of a phy. Clock
controller does not have a physical layer.
Sorry I think the wording was not perfect. I meant the PHY node is a
pixel clock provider. voutcrg consumes a clock not a PHY.

voutcrg: clock-controller@295c0000 {
         clocks = <&syscrg ...>, <&hdmi_phy>;
         clock-names = ...,"hdmitx0_pixelclk";
};

The consumer of the PHY is only the hdmi controller.

What matters for the node layout is where that clock goes. voutcrg is the
SoC display clock controller - it is not part of the HDMI block:

hdmi_phy -- pixel clock -> voutcrg
                              |
                              - pclk/mclk/bclk -> hdmi_controller
                              - pix0/pix1 -> dc8200

hdmi_phy and hdmi_controller are the same register block with one reg
owned by the parent. So if that block is described as a single node:

hdmi_node - pixel clock -> voutcrg
     ^                          |
     --- pclk/mclk/bclk --------

the node provides a clock to voutcrg and consumes three clocks from
voutcrg. That is a cycle in the device tree description.
There is no cycle. Internal signals to the block are not represented in DT.

What you have is a driver problem and you create some sort of DT
structure to solve that. DT purpose is NOT to solve your driver
dependencies or circular connections.
The structure is not something I added. The voutcrg binding has required
an input clock named hdmitx0_pixelclk since it was merged in 2023
a097a5ec14df ("dt-bindings: clock: Add StarFive JH7110 Video-Output
clock and reset generator") and this series does not touch that file:

Documentation/devicetree/bindings/clock/starfive,jh7110-voutcrg.yaml
       - const: hdmitx0_pixelclk

Mainline satisfies it with a placeholder, because so far nothing provided the
real clock:

hdmitx0_pixelclk: hdmitx0-pixel-clock {
            compatible = "fixed-clock";                                                                          
            clock-output-names = "hdmitx0_pixelclk";
            #clock-cells = <0>;
};

All this series does is point that existing input at the device that actually
generates it.

TRM [1] table 5-3  on page 529 lists the display CRG's input clocks. Most come from
dom_vout_top, but three come from IP blocks inside the subsystem:

clk_hdmitx0_pixelclk        297 MHz   u0_hdmi_tx.clk_pix
clk_mipitx_dphy_rxesc        10 MHz   u0_mipitx_dphy.clk_rxesc
clk_mipitx_dphy_txbytehs    297 MHz   u0_mipitx_dphy.clk_txbytehs

and on page 530 the CRG offers clk_hdmitx0_pixelclk as Source1 for:

u0_dc8200.clk_pix0      Source0 clk_dc8200_pix0  Source1 clk_hdmitx0_pixelclk
u0_dc8200.clk_pix1      Source0 clk_dc8200_pix0  Source1 clk_hdmitx0_pixelclk
u0_cdns_dsiTx.clk_dpi   Source0 clk_dc8200_pix0  Source1 clk_hdmitx0_pixelclk

The DC8200 and the DSI TX are separate devices, so the pixel clock
crosses two block boundaries to reach them.

That is also why the DT has to be able to name it. Those MUXes have two
parents and the selection is made in DT:

&dc8200 {
          assigned-clocks = <&voutcrg JH7110_VOUTCLK_DC8200_PIX0>, ...;
          assigned-clock-parents = <&hdmi_phy>, <&hdmi_phy>;
};

and voutcrg takes it as an input clock because that is what the hardware
does. If the PHY is folded into the HDMI node, that input phandle points at
a node which also consumes pclk/mclk/bclk from voutcrg. That is where the
cycle comes from - the hardware topology not the driver.
Which is perfectly "fine" - present in many other SoCs as well. This is
not different and your arguments are insufficient to give here
exceptions. Why? Because again you use as an argument a Linux driver
problem (cyclic dependency). Basically to solve Linux problem you
introduce new empty nodes.

Best regards,
Krzysztof
Thread (59 messages) 59 messages, 9 authors, last reply 17d ago
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help