Re: [PATCH v22 5/8] dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

Hello Laurentiu,

On Mon Apr 27, 2026 at 4:35 PM CEST, Laurentiu Palcu wrote:
> Hi Luca,
>
> On Mon, Apr 27, 2026 at 02:59:47PM +0200, Luca Ceresoli wrote:
>> Hello Laurentiu,
>>
>> On Fri Apr 24, 2026 at 1:07 PM CEST, Laurentiu Palcu wrote:
>> > From: Sandor Yu <Sandor.yu@nxp.com>
>> >
>> > Add bindings for Freescale iMX8MQ DP and HDMI PHY.
>> >
>> > Signed-off-by: Sandor Yu <Sandor.yu@nxp.com>
>> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
>> > ---
>> >  .../bindings/phy/fsl,imx8mq-hdptx-phy.yaml         | 80 ++++++++++++++++++++++
>> >  1 file changed, 80 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8mq-hdptx-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8mq-hdptx-phy.yaml
>> > new file mode 100644
>> > index 0000000000000..a24435139b8b3
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/phy/fsl,imx8mq-hdptx-phy.yaml
>> > @@ -0,0 +1,80 @@
>> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> > +%YAML 1.2
>> > +---
>> > +$id: http://devicetree.org/schemas/phy/fsl,imx8mq-hdptx-phy.yaml#
>> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> > +
>> > +title: Cadence HDP-TX DP/HDMI PHY for Freescale i.MX8MQ SoC
>> > +
>> > +maintainers:
>> > +  - Sandor Yu <sandor.yu@nxp.com>
>>
>> Based on what you said in the cover, I guess this line will have to be
>> changed. Are you willing to maintain this binding?
>
> Yes, I'll update it in the next iteration.
>
>>
>> > +description:
>> > +  The Cadence HDP-TX DP/HDMI PHY is a child node of the MHDP8501 bridge,
>> > +  sharing the same MMIO region as the parent bridge node.
>> > +
>> > +properties:
>> > +  compatible:
>> > +    const: fsl,imx8mq-hdptx-phy
>> > +
>> > +  clocks:
>> > +    items:
>> > +      - description: PHY reference clock.
>> > +      - description: APB clock.
>> > +
>> > +  clock-names:
>> > +    items:
>> > +      - const: ref
>> > +      - const: apb
>> > +
>> > +  "#phy-cells":
>> > +    const: 0
>> > +
>> > +required:
>> > +  - compatible
>> > +  - clocks
>> > +  - clock-names
>> > +  - "#phy-cells"
>> > +
>> > +additionalProperties: false
>> > +
>> > +examples:
>> > +  - |
>> > +    #include <dt-bindings/clock/imx8mq-clock.h>
>> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
>> > +
>> > +    display-bridge@32c00000 {
>> > +        compatible = "fsl,imx8mq-mhdp8501";
>> > +        reg = <0x32c00000 0x100000>;
>> > +        interrupts = <GIC_SPI 16 IRQ_TYPE_LEVEL_HIGH>,
>> > +                     <GIC_SPI 25 IRQ_TYPE_LEVEL_HIGH>;
>> > +        interrupt-names = "plug_in", "plug_out";
>> > +        clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>;
>> > +        phys = <&dp_phy>;
>> > +
>> > +        ports {
>> > +            #address-cells = <1>;
>> > +            #size-cells = <0>;
>>
>> The ports are not mentioned in the properties. I'm not a DT maintainer, but
>> I think they should, e.g. to mention which port is the input and which is
>> the output.
>>
>> > +
>> > +            port@0 {
>> > +                reg = <0>;
>> > +                endpoint {
>> > +                    remote-endpoint = <&dcss_out>;
>> > +                };
>> > +            };
>> > +
>> > +            port@1 {
>> > +                reg = <1>;
>> > +                endpoint {
>> > +                    data-lanes = <2 1 0 3>;
>>
>> Having a remote-endpoint property would be nice here, to make the example
>> more complete.
>
> The ports and the remote endpoints are documented in the bridge binding...

Ah, my bad, I reviewed too quickly and mixed the phy and bridge bindings in
my mind, sorry.

>
> However, I believe I screwed this example up by adding the entire bridge node,
> instead of just a simple:
>
> mhdp {
>     phy {
>         compatible = "fsl,imx8mq-hdptx-phy";
>         #phy-cells = <0>;
>         clocks = <&hdmi_phy_27m>, <&clk IMX8MQ_CLK_DISP_APB_ROOT>;
>         clock-names = "ref", "apb";
>     };
> };
>
> I'll simplify the example in the next iteration.

Yes, I think it would make sense to do it.

Thanks,
Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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