Re: [PATCH v2 02/15] dt-bindings: display: bridge: Add starfive,jh7110-inno-hdmi-controller

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

On 8/28/26 18:47, Conor Dooley wrote:
> On Fri, Aug 28, 2026 at 03:47:26PM +0200, Michal Wilczynski wrote:
>> Add the dt-binding for the StarFive JH7110 Innosilicon HDMI controller
>> (DRM bridge).
>>
>> This device is the second child of the starfive,jh7110-hdmi-subsystem node.
>> It consumes register access clocks (sys, mclk, bclk) from the voutcrg and
>> both the pixel clock (pclk) and the PHY from its hdmi_phy sibling.
>>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
>> ---
>>  .../starfive,jh7110-inno-hdmi-controller.yaml      | 109 +++++++++++++++++++++
>>  1 file changed, 109 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/display/bridge/starfive,jh7110-inno-hdmi-controller.yaml b/Documentation/devicetree/bindings/display/bridge/starfive,jh7110-inno-hdmi-controller.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..0e584e7b756e24e2f99a62daab3117bbf4b472b6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/display/bridge/starfive,jh7110-inno-hdmi-controller.yaml
>> @@ -0,0 +1,109 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: https://protect2.fireeye.com/v1/url?k=af8ec5c4-ce05d0f2-af8f4e8b-74fe485cbff1-93a4f693847d5818&q=1&e=48120ebb-9eda-4ddc-b580-e20f8c69d005&u=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fstarfive%2Cjh7110-inno-hdmi-controller.yaml%23
>> +$schema: https://protect2.fireeye.com/v1/url?k=4b8c4737-2a075201-4b8dcc78-74fe485cbff1-8caced678494fd92&q=1&e=48120ebb-9eda-4ddc-b580-e20f8c69d005&u=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23
>> +
>> +title: StarFive JH7110 Innosilicon HDMI Controller
>> +
>> +maintainers:
>> +  - Michal Wilczynski <m.wilczynski@samsung.com>
>> +
>> +description:
>> +  The controller portion of the StarFive JH7110 INNO HDMI IP.
>> +
>> +properties:
>> +  compatible:
>> +    const: starfive,jh7110-inno-hdmi-controller
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    items:
>> +      - description: System clock for register access
>> +      - description: Module clock
>> +      - description: Bus clock
>> +      - description: Pixel clock from PHY
>> +
>> +  clock-names:
>> +    items:
>> +      - const: sys
>> +      - const: mclk
>> +      - const: bclk
>> +      - const: pclk
> 
> "module" "bus" "pixel" would be more informative than spending most of
> your characters regurgitating what these are by definition?

So I've reread the TRM and mclk and bclk are named as such there and they
are audio clocks: section 5.3 lists a "four channel I2S transmitter bit
clock" among the display subsystem's clock sources, and that is this bclk.
The already merged starfive,jh7110-voutcrg.yaml names the same input
"i2stx0_bclk". So I've kept the names and put the information in the
descriptions instead:

 - description: The HDMI controller main clock
 - description: Audio master clock
 - description: Audio bit clock, from the I2S transmitter
 - description: The pixel clock, generated by the PHY

pclk was wrong. In this binding pclk is the register clock, as in
rockchip,inno-hdmi.yaml, and the generic inno-hdmi driver looks it up by
that name, holds it enabled for its lifetime and derives the DDC divider
from its rate. We had it bound to the pixel clock so the pre-PLL stayed
powered from probe and the divider was sized from 297 MHz instead of
49.5 MHz, putting the DDC bus at 17 kHz. Slow but not broken which
is why it went unnoticed.

So in v3 the register clock takes "pclk" and the PHY clock is named
"pixel".

> 
>> +
>> +  resets:
>> +    maxItems: 1
>> +
>> +  reset-names:
>> +    const: hdmi_tx
>> +
>> +  phys:
>> +    maxItems: 1
>> +
>> +  phy-names:
>> +    const: hdmi-phy
> 
> With the trend of ?pointless? -names properties, I'm surprised you don't
> have interrupt-names? I'd ditch 'em all where there's only a single
> clock. The _ rather than - in the reset name triggers my OCD.
> pw-bot: changes-requested

Will do. reset-names and phy-names will be gone which takes the
underscore in hdmi_tx with them.

Note that in v3 the vout subsystem parent node will be back on Icenowy's
review, so there will be three new DT patches: bindings for
starfive,jh7110-vout-subsystem and starfive,jh7110-vout-syscon, and
starfive,jh7110-dc8200 added to verisilicon,dc.yaml

> 
>> +
>> +  ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +    description:
>> +      A graph node with one input port and one output port.
>> +
>> +required:
>> +  - compatible
>> +  - interrupts
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +  - phys
>> +  - phy-names
>> +  - ports
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  # Shown as a child of the HDMI subsystem node; see
>> +  # Documentation/devicetree/bindings/mfd/starfive,jh7110-hdmi-subsystem.yaml
>> +  # for the full node.
>> +  - |
>> +    #include <dt-bindings/clock/starfive,jh7110-crg.h>
>> +    #include <dt-bindings/reset/starfive,jh7110-crg.h>
>> +
>> +    controller {
> 
> "controller" seems over generic for a node, but I guess if the parent is
> "hdmi-subsystem" or something it makes sense in that context.
> 
> Cheers,
> Conor.
> 
> 
>> +        compatible = "starfive,jh7110-inno-hdmi-controller";
>> +        clocks = <&voutcrg JH7110_VOUTCLK_HDMI_TX_SYS>,
>> +                 <&voutcrg JH7110_VOUTCLK_HDMI_TX_MCLK>,
>> +                 <&voutcrg JH7110_VOUTCLK_HDMI_TX_BCLK>,
>> +                 <&hdmi_phy>;
>> +        clock-names = "sys", "mclk", "bclk", "pclk";
>> +        interrupts = <99>;
>> +        phys = <&hdmi_phy>;
>> +        phy-names = "hdmi-phy";
>> +        resets = <&voutcrg JH7110_VOUTRST_HDMI_TX_HDMI>;
>> +        reset-names = "hdmi_tx";
>> +
>> +        ports {
>> +            #address-cells = <1>;
>> +            #size-cells = <0>;
>> +
>> +            port@0 {
>> +                reg = <0>;
>> +
>> +                endpoint {
>> +                    remote-endpoint = <&dpu_out_dpi0>;
>> +                };
>> +            };
>> +
>> +            port@1 {
>> +                reg = <1>;
>> +
>> +                endpoint {
>> +                    remote-endpoint = <&hdmi_con_in>;
>> +                };
>> +            };
>> +        };
>> +    };
>> +
>> +...
>>
>> -- 
>> 2.34.1
>>

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy