Re: [PATCH v2] dt-bindings: arm: mediatek: mmsys: refine power and gce properties

Re: [PATCH v2] dt-bindings: arm: mediatek: mmsys: refine power and gce properties

[Krzysztof Kozlowski]

On 29/04/2022 10:30, Jason-JH.Lin wrote:
> Power:
>   Refine description and add item number for power-domains property.
> 
> GCE:
>   Refine description and add item number for mboxes property and
>   mediatek,gce-client-reg property.
> 
> Fixes: 1da90b8a7bae ("dt-bindings: arm: mediatek: mmsys: add power and gce properties")
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [PATCH v2] dt-bindings: arm: mediatek: mmsys: refine power and gce properties

[Krzysztof Kozlowski]

On 29/04/2022 10:30, Jason-JH.Lin wrote:
> Power:
>   Refine description and add item number for power-domains property.
> 
> GCE:
>   Refine description and add item number for mboxes property and
>   mediatek,gce-client-reg property.
> 
> Fixes: 1da90b8a7bae ("dt-bindings: arm: mediatek: mmsys: add power and gce properties")
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
> Change in v2:
>   1. Add maxItems: 24 for mboxes property
>   2. Add description and $refs for mediatek,gce-client-reg property
> ---
>  .../bindings/arm/mediatek/mediatek,mmsys.yaml | 23 ++++++++++++-------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> index 6ad023eec193..2b27577f98cc 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> @@ -43,17 +43,19 @@ properties:
>      maxItems: 1
>  
>    power-domains:
> +    maxItems: 1
>      description:
> -      A phandle and PM domain specifier as defined by bindings
> -      of the power controller specified by phandle. See
> -      Documentation/devicetree/bindings/power/power-domain.yaml for details.
> +      Each mmsys belongs to a power-domains. If mmsys wants to use PM
> +      interface to control the power controller of mmsys, it should have
> +      this property.
>  
>    mboxes:
> +    minItems: 1
> +    maxItems: 24

Wait, no, I wrote it should be 16 for one device and 24 for other, not
24 for all...

Best regards,
Krzysztof

Re: [PATCH v2] dt-bindings: arm: mediatek: mmsys: refine power and gce properties

[Jason-JH Lin]

Hi Krzysztof,

Thanks for the reviews.

On Fri, 2022-04-29 at 22:58 +0200, Krzysztof Kozlowski wrote:
> On 29/04/2022 10:30, Jason-JH.Lin wrote:
> > Power:
> >   Refine description and add item number for power-domains
> > property.
> > 
> > GCE:
> >   Refine description and add item number for mboxes property and
> >   mediatek,gce-client-reg property.
> > 
> > Fixes: 1da90b8a7bae ("dt-bindings: arm: mediatek: mmsys: add power
> > and gce properties")
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>

[snip]
 
> >    mboxes:
> > +    minItems: 1
> > +    maxItems: 24
> 
> Wait, no, I wrote it should be 16 for one device and 24 for other,
> not
> 24 for all...
> 

Ok, I thought maxItems: 24 contained 16, but it's not clear enough.


I'm not sure how to describe different maxItems for different
platforms. Does it come like this?

1. Remove the maxItems in mboxes property.
2. Add the allOf condition before the line "additionalProperties:
false". E.g.

allOf:
  - if:
      properties:
        compatible:
          contains:
            const: mediatek,mt8173-mmsys
    then:
      properties:
        mboxes:
          maxItems: 16
    else:
      properties:
        mboxes:
          maxItems: 24

additionalProperties: false
...


Regards,
Jason-JH.Lin

> Best regards,
> Krzysztof
-- 
Jason-JH Lin <jason-jh.lin@mediatek.com>

Re: [PATCH v2] dt-bindings: arm: mediatek: mmsys: refine power and gce properties

[Krzysztof Kozlowski]

On 01/05/2022 10:26, Jason-JH Lin wrote:
> Hi Krzysztof,
> 
> Thanks for the reviews.
> 
> On Fri, 2022-04-29 at 22:58 +0200, Krzysztof Kozlowski wrote:
>> On 29/04/2022 10:30, Jason-JH.Lin wrote:
>>> Power:
>>>   Refine description and add item number for power-domains
>>> property.
>>>
>>> GCE:
>>>   Refine description and add item number for mboxes property and
>>>   mediatek,gce-client-reg property.
>>>
>>> Fixes: 1da90b8a7bae ("dt-bindings: arm: mediatek: mmsys: add power
>>> and gce properties")
>>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> 
> [snip]
>  
>>>    mboxes:
>>> +    minItems: 1
>>> +    maxItems: 24
>>
>> Wait, no, I wrote it should be 16 for one device and 24 for other,
>> not
>> 24 for all...
>>
> 
> Ok, I thought maxItems: 24 contained 16, but it's not clear enough.
> 
> 
> I'm not sure how to describe different maxItems for different
> platforms. Does it come like this?
> 
> 1. Remove the maxItems in mboxes property.

mboxes:
  maxItems: 24

> 2. Add the allOf condition before the line "additionalProperties:
> false". E.g.
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: mediatek,mt8173-mmsys
>     then:
>       properties:
>         mboxes:
>           maxItems: 16
>     else:
>       properties:
>         mboxes:
>           maxItems: 24

and you can skip the "else". In general looks fine.

Best regards,
Krzysztof