Re: [PATCH 01/11] dt-bindings: display: rockchip,inno-hdmi: Document RK3128 compatible

[Krzysztof Kozlowski]

On 14/12/2023 16:22, Alex Bee wrote:
> 
> Am 14.12.23 um 08:53 schrieb Krzysztof Kozlowski:
>> On 13/12/2023 20:51, Alex Bee wrote:
>>> Document the compatible for RK3128's HDMI controller block.
>>> The integration for this SoC is somewhat different here: It needs the PHY's
>> Please wrap commit message according to Linux coding style / submission
>> process (neither too early nor over the limit):
>> https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
> OK. Not sure why checkpatch --strict  didn't tell me that I'm over the 
> limit here.
>>
>>> reference clock rate to calculate the ddc bus frequency correctly. This
>>> clock is part of a power-domain (PD_VIO), so this gets added as an optional
>>> property too.
>> If clock is part of power domain, then the power domain must be in the
>> clock controller, not here. So either you put power domain in wrong
>> place or you used incorrect reason for a change.
>   Rockchip defines it's powerdomains per clock and I was little to much 
> in that world when writing this. Actually the controller itself is part 
> of the powerdomain. Will rephrase.

Does it mean you have like 200 different power domains in one SoC? Then
how are they different than clock if there is one-to-one mapping?

>>> Signed-off-by: Alex Bee <knaerzche@xxxxxxxxx>
>>> ---
>>>   .../display/rockchip/rockchip,inno-hdmi.yaml  | 30 +++++++++++++++++--
>>>   1 file changed, 28 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml b/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml
>>> index 96889c86849a..9f00abcbfb38 100644
>>> --- a/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml
>>> +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip,inno-hdmi.yaml
>>> @@ -14,6 +14,7 @@ properties:
>>>     compatible:
>>>       enum:
>>>         - rockchip,rk3036-inno-hdmi
>>> +      - rockchip,rk3128-inno-hdmi
>>>   
>>>     reg:
>>>       maxItems: 1
>>> @@ -22,10 +23,21 @@ properties:
>>>       maxItems: 1
>>>   
>>>     clocks:
>>> -    maxItems: 1
>>> +    minItems: 1
>>> +    items:
>>> +      - description: The HDMI controller main clock
>>> +      - description: The HDMI PHY reference clock
>>>   
>>>     clock-names:
>>> -    const: pclk
>>> +    minItems: 1
>>> +    items:
>>> +      - const: pclk
>>> +      - enum:
>>> +          - pclk
>>> +          - ref
>> That's way overcomplicated. Just items listing the names and minItems:
>> 1. See other bindings how this is done.
> OK.
>>> +
>>> +  power-domains:
>>> +    maxItems: 1
>> Is it relevant to existing device?
> 
> Will move to new variant only.
> 

Definition should be here, but in if:then: it should be disallowed
(:false) for other variants.

Best regards,
Krzysztof