Re: [PATCH v3] dt-bindings: drm/bridge: Update reg-name and reg description list for cdns,mhdp8546 compatible

[Krzysztof Kozlowski <krzk@kernel.org>]

On 25/11/2025 06:26, Harikrishna shenoy wrote:
> 
> 
> On 23/11/25 15:30, Krzysztof Kozlowski wrote:
>> On Fri, Nov 21, 2025 at 06:04:37PM +0530, Harikrishna Shenoy wrote:
>>> Remove j721e-intg register name from reg-name list for cdns,mhdp8546
>>> compatible. The j721e-integ registers are specific to TI SoCs, so they
>>> are not required for compatibles other than ti,j721e-mhdp8546.
>>>
>>> Update reg and reg-names top level constraints with lists according
>>> to compatibles.
>>>
>>> Move the register name constraints and reg description list to the
>>> appropriate compatibility sections to ensure the correct register
>>> names are used with each compatible value also adding the DSC register
>>> to make bindings align with what the hardware supports.
>>>
>>> Fixes: 7169d082e7e6 ("dt-bindings: drm/bridge: MHDP8546 bridge binding changes for HDCP")
>>> Signed-off-by: Harikrishna Shenoy <h-shenoy@ti.com>
>>> ---
>>>
>>> Links to some discussions pointing to need for a fixes patch:
>>> https://lore.kernel.org/all/20250903220312.GA2903503-robh@kernel.org/
>>> https://lore.kernel.org/all/d2367789-6b54-4fc2-bb7c-609c0fe084d3@ti.com/
>>>
>>> Link to v2:
>>> <https://lore.kernel.org/all/20251119122447.514729-1-h-shenoy@ti.com/>
>>>
>>> Changelog v2 --> v3:
>>> -Add the reg description list and reg-name list in top level constraints
>>> using oneOf for either of compatible.
>>> Logs after testing some cases: https://gist.github.com/h-shenoy/a422f7278859cd95447e674963caabd9
>>>
>>> Link to v1:
>>> <https://lore.kernel.org/all/20251107131535.1841393-1-h-shenoy@ti.com/>
>>>
>>> Changelog v1 --> v2:
>>> -Update the reg description list for each compatible and add register space
>>> for dsc to make the bindings reflect what hardware supports although
>>> the driver doesn't support dsc yet.
>>>
>>> Note: j721e-integ are not optional registers for ti-compatible.
>>>
>>>   .../display/bridge/cdns,mhdp8546.yaml         | 85 ++++++++++++++-----
>>>   1 file changed, 66 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
>>> index c2b369456e4e2..632595ef32f63 100644
>>> --- a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
>>> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8546.yaml
>>> @@ -17,23 +17,45 @@ properties:
>>>         - ti,j721e-mhdp8546
>>>   
>>>     reg:
>>> -    minItems: 1
>>> -    items:
>>> -      - description:
>>> -          Register block of mhdptx apb registers up to PHY mapped area (AUX_CONFIG_P).
>>> -          The AUX and PMA registers are not part of this range, they are instead
>>> -          included in the associated PHY.
>>> -      - description:
>>> -          Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7 SoCs.
>>> -      - description:
>>> -          Register block of mhdptx sapb registers.
>>> +    oneOf:
>>> +      - minItems: 2
>>> +      - items:
>>
>> This is wrong syntax. You created here a list, so you now allow
>> anything with minItems 2.
> Hi Krzysztof,
> 
> The list defined here restricts what lists are accepted, so for 

No. It does not.

Look - you defined list of ONLY minItems:2, without anything else. The
problem is that your oneOf consist of four separate cases and you wanted
two cases, so only:
oneOf:
 - foo
   whatever goes here

 - bar
   further schema

Not four.

> cdns,mhdp8546 compatible anything more than 3 items is rejected 
> (example: 
> https://gist.github.com/h-shenoy/a422f7278859cd95447e674963caabd9). 
> Could you please help me with an
> example where you think the bindings are incorrect?
> 
>>
>>> +          - description:
>>> +              Register block of mhdptx apb registers up to PHY mapped area (AUX_CONFIG_P).
>>> +              The AUX and PMA registers are not part of this range, they are instead
>>> +              included in the associated PHY.
>>> +          - description:
>>> +              Register block for DSS_EDP0_INTG_CFG_VP registers in case of TI J7 SoCs.
>>> +          - description:
>>> +              Register block of mhdptx sapb registers.
>>> +          - description:
>>> +              Register block for mhdptx DSC encoder registers.
>>> +
>>> +      - minItems: 1
>>
>> Actually anything with minItems 1... I asked for list of TWO, not FOUR,
>> items. Or if syntax is getting to complicated, just min and maxItems.
>>
>>
>>> +      - items:
>>> +          - description:
>>> +              Register block of mhdptx apb registers up to PHY mapped area (AUX_CONFIG_P).
>>> +              The AUX and PMA registers are not part of this range, they are instead
>>> +              included in the associated PHY.
>>> +          - description:
>>> +              Register block of mhdptx sapb registers.
>>> +          - description:
>>> +              Register block for mhdptx DSC encoder registers.
>>>   
>>>     reg-names:
>>> -    minItems: 1
>>> -    items:
>>> -      - const: mhdptx
>>> -      - const: j721e-intg
>>> -      - const: mhdptx-sapb
>>> +    oneOf:
>>> +      - minItems: 2
>>> +      - items:
>>
>> Also wrong.
>>
>>> +          - const: mhdptx
>>> +          - const: j721e-intg
>>> +          - const: mhdptx-sapb
>>> +          - const: dsc
>>> +
>>> +      - minItems: 1
>>> +      - items:
>>> +          - const: mhdptx
>>> +          - const: mhdptx-sapb
>>> +          - const: dsc
>>>   
>>>     clocks:
>>>       maxItems: 1
>>> @@ -100,18 +122,43 @@ allOf:
>>>         properties:
>>>           reg:
>>>             minItems: 2
>>> -          maxItems: 3
>>
>> Your commit msg says you "remove" but here you ADD one more item, thus
>> growing this 3->4.
>>
>> How remove can result in 3 becoming 4?
>>
> Yes, remove is for j721e-intg for cdns,mhdp8546 compatible, and to make
> bindings complete have added dsc reg-blocks, these changes reflects 
> correct capabilities of hardware, have mentioned these in commit message 
> as well.


Again, how a "remove" can result in growing?


Best regards,
Krzysztof