[EXT] Re: [PATCH v17 3/8] dt-bindings: display: bridge: Add Cadence MHDP8501

[Sandor Yu]

> 
> On 29/09/2024 04:36, Sandor Yu wrote:
> > Hi Krzysztof，
> >
> > Thanks for your comments,
> >
> >>
> >>
> >> On Tue, Sep 24, 2024 at 03:36:48PM +0800, Sandor Yu wrote:
> >>> Add bindings for Cadence MHDP8501 DisplayPort/HDMI bridge.
> >>>
> >>> Signed-off-by: Sandor Yu <Sandor.yu at nxp.com>
> >>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski at linaro.org>
> >>
> >> Drop
> > OK, I will remove it in the next version.
> >
> >>
> >>> ---
> >>> v16->v17:
> >>> - Add lane-mapping property
> >>
> >> That's a significant change.
> > OK.
> >
> >>
> >>>
> >>> v9->v16:
> >>>  *No change
> >>>
> >>> .../display/bridge/cdns,mhdp8501.yaml         | 109
> >> ++++++++++++++++++
> >>>  1 file changed, 109 insertions(+)
> >>>  create mode 100644
> >>> Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
> >>>
> >>> diff --git
> >>>
> a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yam
> >>> l
> >>>
> b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yam
> >>> l
> >>> new file mode 100644
> >>> index 0000000000000..3f79f328c7425
> >>> --- /dev/null
> >>> +++
> b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501
> >>> +++ .y
> >>> +++ aml
> >>
> >>> @@ -0,0 +1,109 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> >>> +---
> >>> +$id:
> >>>
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> >>>
> +vi%2F&data=05%7C02%7Csandor.yu%40nxp.com%7C3c6bfe07c23945a1052
> 608dc
> >>>
> +e13ffde7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386329
> 121918
> >>>
> +65139%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
> luMzIiL
> >>>
> +CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=yrG%2FJsGK5
> czDeRwf
> >>> +A8e6%2F5r0Ui5v95uiC4OlK7DJ5QY%3D&reserved=0
> >>>
> >>
> +cetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fcdns%2Cmhdp8501.yaml%
> 2
> >> 3&dat
> >>>
> >>
> +a=05%7C02%7CSandor.yu%40nxp.com%7C40a6bd4ff1cd4d934da008dcdc72
> >> 9fd0%7C
> >>>
> >>
> +686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63862763207998660
> >> 8%7CUnkno
> >>>
> >>
> +wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1
> h
> >> aWwi
> >>>
> >>
> +LCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Uw%2FQQ0Qg36Y8Q6wFPC7Z
> g
> >> LzLHvOj8GjH1
> >>> +k8McgcjrqI%3D&reserved=0
> >>> +$schema:
> >>>
> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fde
> >>>
> +vi%2F&data=05%7C02%7Csandor.yu%40nxp.com%7C3c6bfe07c23945a1052
> 608dc
> >>>
> +e13ffde7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6386329
> 121918
> >>>
> +95630%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2
> luMzIiL
> >>>
> +CJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=%2BKccVMx
> nf5oUwZgf
> >>> +smPXiKN2VycnLCdDKx6FZFBBAJM%3D&reserved=0
> >>>
> >>
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C02%7CSandor.
> y
> >> u%40n
> >>>
> >>
> +xp.com%7C40a6bd4ff1cd4d934da008dcdc729fd0%7C686ea1d3bc2b4c6fa9
> 2
> >> cd99c5
> >>>
> >>
> +c301635%7C0%7C0%7C638627632080031124%7CUnknown%7CTWFpbGZs
> >> b3d8eyJWIjoi
> >>>
> >>
> +MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0
> %
> >> 7C%7C%
> >>>
> >>
> +7C&sdata=IG9Em3AWOwzlpR1Wk0Vw%2FF161YcOxuMHbA4Y%2FzftgYA%3
> D
> >> &reserved=
> >>> +0
> >>> +
> >>> +title: Cadence MHDP8501 DP/HDMI bridge
> >>> +
> >>> +maintainers:
> >>> +  - Sandor Yu <Sandor.yu at nxp.com>
> >>> +
> >>> +description:
> >>> +  Cadence MHDP8501 DisplayPort/HDMI interface.
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - fsl,imx8mq-mhdp8501
> >>> +
> >>> +  reg:
> >>> +    maxItems: 1
> >>> +
> >>> +  clocks:
> >>> +    maxItems: 1
> >>> +    description: MHDP8501 DP/HDMI APB clock.
> >>> +
> >>> +  phys:
> >>> +    maxItems: 1
> >>> +    description:
> >>> +      phandle to the DP/HDMI PHY
> >>> +
> >>> +  interrupts:
> >>> +    items:
> >>> +      - description: Hotplug cable plugin.
> >>> +      - description: Hotplug cable plugout.
> >>> +
> >>> +  interrupt-names:
> >>> +    items:
> >>> +      - const: plug_in
> >>> +      - const: plug_out
> >>> +
> >>> +  lane-mapping:
> >>> +    description: lane mapping for HDMI or DisplayPort interface.
> >>
> >> Where is the definition of this property? I do not see any $ref here,
> >> so did you add it to dtschema?
> >
> > My apologies, the $ref is missing, will add it in the next version..
> 
> And that's different than existing properties, e.g. data-lanes or lane-polarities?
> There is no description here except copying property name, which is not
> useful at all.
> 
> Come with solution matching other bridges and media devices.

The data-lanes property in media/video-interfaces.yaml supports lane reordering, 
which is what I expected. I'll use data-lanes to replace lane-mapping in the next version.

Thanks
Sandor

> 
> Best regards,
> Krzysztof