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

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

[Krzysztof Kozlowski]

On 28/04/2022 03:27, jason-jh.lin wrote:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> 
> 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>
> ---
>  .../bindings/arm/mediatek/mediatek,mmsys.yaml | 29 +++++++++----------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> index 6ad023eec193..6722f1b724ef 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yaml
> @@ -43,28 +43,27 @@ 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, surely you cannot have infinite number of mailboxes to talk with?

>      description:
> -      Using mailbox to communicate with GCE, it should have this
> -      property and list of phandle, mailbox specifiers. See
> -      Documentation/devicetree/bindings/mailbox/mtk-gce.txt for details.
> -    $ref: /schemas/types.yaml#/definitions/phandle-array
> +      If using mailbox to communicate with GCE, it should have this
> +      property. GCE will help configure the hardware settings for the
> +      current mmsys data pipeline.
>  
>    mediatek,gce-client-reg:
> -    description:
> -      The register of client driver can be configured by gce with 4 arguments
> -      defined in this property, such as phandle of gce, subsys id,
> -      register offset and size.
> -      Each subsys id is mapping to a base address of display function blocks
> -      register which is defined in the gce header
> -      include/dt-bindings/gce/<chip>-gce.h.
> -    $ref: /schemas/types.yaml#/definitions/phandle-array

Why removing ref? Does your binding work after such change?

>      maxItems: 1
> +    items:
> +      - items:
> +          - description: phandle to GCE
> +          - description: subsys id
> +          - description: register offset
> +          - description: register size
>  
>    "#clock-cells":
>      const: 1


Best regards,
Krzysztof

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

[Jason-JH Lin]

Hi Krzysztof,

Thanks for the reviews.

On Thu, 2022-04-28 at 09:16 +0200, Krzysztof Kozlowski wrote:
> On 28/04/2022 03:27, jason-jh.lin wrote:
> > From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
> > 
> > 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>
> > ---
> >  .../bindings/arm/mediatek/mediatek,mmsys.yaml | 29 +++++++++----
> > ------
> >  1 file changed, 14 insertions(+), 15 deletions(-)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > index 6ad023eec193..6722f1b724ef 100644
> > ---
> > a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > +++
> > b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.yam
> > l
> > @@ -43,28 +43,27 @@ 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, surely you cannot have infinite number of mailboxes to talk
> with?

Yes, we cannot have infinite number for this, but we can use not only
one mbox channel for one mmsys.

Its maximum number is equal to GCE HW thread number.
The maximum number in mt8195 is 24 and mt8173 is 16.

But we currently using 1.
So I'm not sure if I need to set the maxItems for this?

> 
> >      description:
> > -      Using mailbox to communicate with GCE, it should have this
> > -      property and list of phandle, mailbox specifiers. See
> > -      Documentation/devicetree/bindings/mailbox/mtk-gce.txt for
> > details.
> > -    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +      If using mailbox to communicate with GCE, it should have
> > this
> > +      property. GCE will help configure the hardware settings for
> > the
> > +      current mmsys data pipeline.
> >  
> >    mediatek,gce-client-reg:
> > -    description:
> > -      The register of client driver can be configured by gce with
> > 4 arguments
> > -      defined in this property, such as phandle of gce, subsys id,
> > -      register offset and size.
> > -      Each subsys id is mapping to a base address of display
> > function blocks
> > -      register which is defined in the gce header
> > -      include/dt-bindings/gce/<chip>-gce.h.
> > -    $ref: /schemas/types.yaml#/definitions/phandle-array
> 
> Why removing ref? Does your binding work after such change?

I removed it unexpectedly, I'll add it back.
> 
> >      maxItems: 1
> > +    items:
> > +      - items:
> > +          - description: phandle to GCE
> > +          - description: subsys id
> > +          - description: register offset
> > +          - description: register size
> >  
> >    "#clock-cells":
> >      const: 1
> 
> 
> Best regards,
> Krzysztof

Regards,
Jason-JH.Lin
-- 
Jason-JH Lin <jason-jh.lin@mediatek.com>

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

[Krzysztof Kozlowski]

On 29/04/2022 06:10, Jason-JH Lin wrote:
> Yes, we cannot have infinite number for this, but we can use not only
> one mbox channel for one mmsys.
> 
> Its maximum number is equal to GCE HW thread number.
> The maximum number in mt8195 is 24 and mt8173 is 16.
> 
> But we currently using 1.
> So I'm not sure if I need to set the maxItems for this?

Yes. 24 for mt8195 and 16 for mt8173.

> 
>>
>>>      description:
>>> -      Using mailbox to communicate with GCE, it should have this
>>> -      property and list of phandle, mailbox specifiers. See
>>> -      Documentation/devicetree/bindings/mailbox/mtk-gce.txt for
>>> details.
>>> -    $ref: /schemas/types.yaml#/definitions/phandle-array
>>> +      If using mailbox to communicate with GCE, it should have
>>> this
>>> +      property. GCE will help configure the hardware settings for
>>> the
>>> +      current mmsys data pipeline.
>>>  
>>>    mediatek,gce-client-reg:
>>> -    description:
>>> -      The register of client driver can be configured by gce with
>>> 4 arguments
>>> -      defined in this property, such as phandle of gce, subsys id,
>>> -      register offset and size.
>>> -      Each subsys id is mapping to a base address of display
>>> function blocks
>>> -      register which is defined in the gce header
>>> -      include/dt-bindings/gce/<chip>-gce.h.
>>> -    $ref: /schemas/types.yaml#/definitions/phandle-array
>>
>> Why removing ref? Does your binding work after such change?
> 
> I removed it unexpectedly, I'll add it back.

Just be sure you test it before submitting...


Best regards,
Krzysztof

Re: [PATCH] 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 07:55 +0200, Krzysztof Kozlowski wrote:
> On 29/04/2022 06:10, Jason-JH Lin wrote:
> > Yes, we cannot have infinite number for this, but we can use not
> > only
> > one mbox channel for one mmsys.
> > 
> > Its maximum number is equal to GCE HW thread number.
> > The maximum number in mt8195 is 24 and mt8173 is 16.
> > 
> > But we currently using 1.
> > So I'm not sure if I need to set the maxItems for this?
> 
> Yes. 24 for mt8195 and 16 for mt8173.
> 
OK, I'll add maxItems: 24.
> > 
> > > 
> > > >      description:
> > > > -      Using mailbox to communicate with GCE, it should have
> > > > this
> > > > -      property and list of phandle, mailbox specifiers. See
> > > > -      Documentation/devicetree/bindings/mailbox/mtk-gce.txt
> > > > for
> > > > details.
> > > > -    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > +      If using mailbox to communicate with GCE, it should have
> > > > this
> > > > +      property. GCE will help configure the hardware settings
> > > > for
> > > > the
> > > > +      current mmsys data pipeline.
> > > >  
> > > >    mediatek,gce-client-reg:
> > > > -    description:
> > > > -      The register of client driver can be configured by gce
> > > > with
> > > > 4 arguments
> > > > -      defined in this property, such as phandle of gce, subsys
> > > > id,
> > > > -      register offset and size.
> > > > -      Each subsys id is mapping to a base address of display
> > > > function blocks
> > > > -      register which is defined in the gce header
> > > > -      include/dt-bindings/gce/<chip>-gce.h.
> > > > -    $ref: /schemas/types.yaml#/definitions/phandle-array
> > > 
> > > Why removing ref? Does your binding work after such change?
> > 
> > I removed it unexpectedly, I'll add it back.
> 
> Just be sure you test it before submitting...
> 
I'm sorry about this.
I think something wrong in my local scanning environment.
I'll fix this and test it before submitting the next version.

Regards,
Jason-JH.Lin
> 
> Best regards,
> Krzysztof
> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> 
https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!3EBAk7zcr3iMc-Todq-efcLmYCZNkxH9gYnUmY1tHbPiVm9yNYhrI-Yj0I16f8jGnA6B$
>  
-- 
Jason-JH Lin <jason-jh.lin@mediatek.com>

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

[Krzysztof Kozlowski]

On 29/04/2022 09:54, Jason-JH Lin wrote:
> Hi Krzysztof,
> 
> Thanks for the reviews.
> 
> On Fri, 2022-04-29 at 07:55 +0200, Krzysztof Kozlowski wrote:
>> On 29/04/2022 06:10, Jason-JH Lin wrote:
>>> Yes, we cannot have infinite number for this, but we can use not
>>> only
>>> one mbox channel for one mmsys.
>>>
>>> Its maximum number is equal to GCE HW thread number.
>>> The maximum number in mt8195 is 24 and mt8173 is 16.
>>>
>>> But we currently using 1.
>>> So I'm not sure if I need to set the maxItems for this?
>>
>> Yes. 24 for mt8195 and 16 for mt8173.
>>
> OK, I'll add maxItems: 24.

and 16 for mt8173....

Best regards,
Krzysztof