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

Luca Ceresoli
2026-04-28T12:21:47Z

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