Re: [PATCH v4 02/17] dt-bindings: gpu: Add Imagination Technologies PowerVR GPU

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

On 18/07/2023 13:32, Frank Binns wrote:
> Hi Krzysztof,
> 
> On Mon, 2023-07-17 at 09:29 +0200, Krzysztof Kozlowski wrote:
>> On 14/07/2023 16:25, Sarah Walker wrote:
>>> Add the device tree binding documentation for the Series AXE GPU used in
>>> TI AM62 SoCs.
>>>
>>
>> ...
>>
>>> +
>>> +  clocks:
>>> +    minItems: 1
>>> +    maxItems: 3
>>> +
>>> +  clock-names:
>>> +    items:
>>> +      - const: core
>>> +      - const: mem
>>> +      - const: sys
>>> +    minItems: 1
>>
>> Why clocks for this device vary? That's really unusual to have a SoC IP
>> block which can have a clock physically disconnected, depending on the
>> board (not SoC!).
> 
> By default, this GPU IP (Series AXE) operates on a single clock (the core
> clock), but the SoC vendor can choose at IP integration time to run the memory
> and SoC interfaces on separate clocks (mem and sys clocks respectively). We also
> have IP, such as the Series 6XT, that requires all 3 clocks.

Currently you have only one SoC vendor with only one SoC, so the clocks
do not vary. Describing the clocks for all possible variants is a good
idea, but then this should be clear that this implementation uses subset.

> 
> So the situation here is that Series AXE may have 1 or 3 clocks, but the TI
> implementation being added only has 1.
> 
> I guess we need to add something like:
> 
>   allOf:
>     - if:
>         properties:
>           compatible:
>             contains:
>               const: ti,am62-gpu
>       then:
>         properties:
>           clocks:
>             maxItems: 1
> 
> Or should we be doing something else?

Yes. clock-names as well..


Best regards,
Krzysztof