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

[Michal Wilczynski]

On 10/3/26 22:45, Krzysztof Kozlowski wrote:
> On 03/10/2026 17:36, Michal Wilczynski wrote:
>>
>>
>> On 9/30/26 13:01, Krzysztof Kozlowski wrote:
>>> On 25/09/2026 23:05, Michal Wilczynski wrote:
>>>>>> +  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.

[1] - https://doc-en.rvspace.org/JH7110/PDF/JH7110_TRM_StarFive_Preliminary_V2.pdf

> 
> Best regards,
> Krzysztof
> 

Best regards,
-- 
Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>