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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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@nxp.com>
>>> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@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.yaml
>>> b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml
>>> 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%2Fdevi
>>>
>> +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%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
>> aWwi
>>>
>> +LCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Uw%2FQQ0Qg36Y8Q6wFPC7Zg
>> LzLHvOj8GjH1
>>> +k8McgcjrqI%3D&reserved=0
>>> +$schema:
>>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
>>>
>> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C02%7CSandor.y
>> u%40n
>>>
>> +xp.com%7C40a6bd4ff1cd4d934da008dcdc729fd0%7C686ea1d3bc2b4c6fa92
>> cd99c5
>>>
>> +c301635%7C0%7C0%7C638627632080031124%7CUnknown%7CTWFpbGZs
>> b3d8eyJWIjoi
>>>
>> +MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%
>> 7C%7C%
>>>
>> +7C&sdata=IG9Em3AWOwzlpR1Wk0Vw%2FF161YcOxuMHbA4Y%2FzftgYA%3D
>> &reserved=
>>> +0
>>> +
>>> +title: Cadence MHDP8501 DP/HDMI bridge
>>> +
>>> +maintainers:
>>> +  - Sandor Yu <Sandor.yu@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.

Best regards,
Krzysztof