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 <[email protected]>
>>> ---
>>>
>>> Links to some discussions pointing to need for a fixes patch:
>>> https://lore.kernel.org/all/[email protected]/
>>> https://lore.kernel.org/all/[email protected]/
>>>
>>> Link to v2:
>>> <https://lore.kernel.org/all/[email protected]/>
>>>
>>> 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/[email protected]/>
>>>
>>> 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
