On 10/4/26 09:15, Krzysztof Kozlowski wrote:
> On 04/10/2026 00:35, Michal Wilczynski wrote:
>>
>>
>> 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.
>
> 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
>
OK probe ordering is a Linux concern and can't be used it to justify the
binding.
We spoke briefly about this on Monday and I came away thinking the node
is acceptable as long as it is justified by what it describes rather
than by probe order. Is that the right reading of what you meant?
On that basis the PHY node:
- is a clock provider, and voutcrg consumes that clock and
routes it on to the DC8200 pixel MUXes and the DSI TX
- is a phy provider for the controller
- takes a different input clock than the controller - xin24m, against
pclk/mclk/bclk from voutcrg
- is separate IP - the same Innosilicon PHY that RK3328 instantiates at its
own address (phy@ff430000), which here shares a register window with the
controller
The reg is the only thing it does not have of its own, and that is shared
with the controller and owned by the parent.
Best regards,
--
Michal Wilczynski <[email protected]>
