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

[Michal Wilczynski <m.wilczynski@samsung.com>]

On 9/17/26 08:51, Krzysztof Kozlowski wrote:
> On Tue, Sep 15, 2026 at 05:32:09PM +0200, Michal Wilczynski wrote:
>> Add the dt-binding for the StarFive JH7110 Innosilicon HDMI PHY.
>>
>> This device is a child of the starfive,jh7110-hdmi-subsystem node. It
>> functions as both a PHY provider for the controller and as a clock
>> provider for the variable pixel clock (hdmi_pclk), which it generates
>> from its refoclk.
>>
>> Reviewed-by: Joshua Peisach <jpeisach@ubuntu.com>
> 
> While I appreciate other people looking at this, I don't think review
> here applies. It was a blanket tag, given to the entire series, with
> exception that it is from newbie, thus I want to ask: was this binding
> REALLY reviewed by Joshua?
> 
> Are the rest of the patches actually reviewed or just "looks good to
> me"? I see no comments from Joshua on v3, v2 and v1, which is usual sign
> of doing review.
> 
> 
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  .../phy/starfive,jh7110-inno-hdmi-phy.yaml         | 49 ++++++++++++++++++++++
>>  1 file changed, 49 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml b/Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..f838fcc238ec7db0a8e794234bdceec36cb6d9e7
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/phy/starfive,jh7110-inno-hdmi-phy.yaml
>> @@ -0,0 +1,49 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/v1/url?k=1b336029-7ab87530-1b32eb66-74fe485cbfec-59631fa23f87244c&q=1&e=7e2eb06f-7b14-4bec-84e5-b60bb954d80a&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fphy%2Fstarfive%2Cjh7110-inno-hdmi-phy.yaml%23
>> +$schema: https://protect2.fireeye.com/v1/url?k=46734e4d-27f85b54-4672c502-74fe485cbfec-9b6e35999c41723e&q=1&e=7e2eb06f-7b14-4bec-84e5-b60bb954d80a&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>> +
>> +title: StarFive JH7110 Innosilicon INNO HDMI PHY
>> +
>> +maintainers:
>> +  - Michal Wilczynski <m.wilczynski@samsung.com>
>> +
>> +description:
>> +  The PHY portion of the StarFive JH7110 INNO HDMI IP.
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-inno-hdmi-phy
>> +
>> +  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
functions with different clock inputs and one of them feeds back into
the SoC clock tree. The PHY generates hdmitx0_pixelclk which voutcrg
consumes the controller consumes pclk/mclk/bclk from voutcrg. Folded
into one node that node is both a provider to and a consumer of
voutcrg which is a cycle in the hardware description, not just in Linux.

> 
> 
> Best regards,
> Krzysztof
> 
> 

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