[PATCH 0/3] Add mediatek,gce-events definition to mediatek,gce-mailbox bindings

[PATCH 0/3] Add mediatek,gce-events definition to mediatek,gce-mailbox bindings

[Jason-JH.Lin]

From: Jason-jh Lin <jason-jh.lin@mediatek.corp-partner.google.com>

Since mediatek,gce-events property is a HW event signal from GCE,
it should be defined in mediatek,gce-mailbox.yaml.

Change the description of mediatek,gce-events property existed in
other bindings to reference mediatek,gce-mailbox.yaml.

Jason-JH.Lin (3):
  dt-bindings: mailbox: mediatek,gce-mailbox: Add mediatek,gce-events
    definition
  dt-bindings: media: mediatek-mdp: Change the description of gce-events
  dt-bindings: soc: mediatek: Change the description of gce-events

 .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml  | 7 +++++++
 .../devicetree/bindings/media/mediatek,mdp3-rdma.yaml      | 5 +----
 .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml       | 5 +----
 .../devicetree/bindings/media/mediatek,mdp3-wrot.yaml      | 5 +----
 .../devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml   | 5 +----
 .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml   | 5 +----
 .../devicetree/bindings/soc/mediatek/mediatek,wdma.yaml    | 5 +----
 7 files changed, 13 insertions(+), 24 deletions(-)

-- 
2.18.0

[PATCH 1/3] dt-bindings: mailbox: mediatek,gce-mailbox: Add mediatek,gce-events definition

[Jason-JH.Lin]

Since mediatek,gce-events property is a HW event signal from GCE,
it should be defined in mediatek,gce-mailbox.yaml and change the
description of mediatek,gce-events property in other bindings to
reference mediatek,gce-mailbox.yaml.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml  | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
index cef9d7601398..e4da0a58c943 100644
--- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
+++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
@@ -49,6 +49,13 @@ properties:
     items:
       - const: gce
 
+  mediatek,gce-events:
+    description:
+      The event id which is mapping to the specific hardware event signal
+      to gce. The event id is defined in the gce header
+      include/dt-bindings/gce/<chip>-gce.h of each chips.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 required:
   - compatible
   - "#mbox-cells"
-- 
2.18.0

Re: [PATCH 1/3] dt-bindings: mailbox: mediatek,gce-mailbox: Add mediatek,gce-events definition

[Krzysztof Kozlowski]

On 18/12/2023 09:36, Jason-JH.Lin wrote:
> Since mediatek,gce-events property is a HW event signal from GCE,
> it should be defined in mediatek,gce-mailbox.yaml and change the

Why? None of the bindings reference it. Did you just move property from
one device to completely different one?

> description of mediatek,gce-events property in other bindings to
> reference mediatek,gce-mailbox.yaml.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml  | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> index cef9d7601398..e4da0a58c943 100644
> --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> @@ -49,6 +49,13 @@ properties:
>      items:
>        - const: gce
>  
> +  mediatek,gce-events:
> +    description:
> +      The event id which is mapping to the specific hardware event signal
> +      to gce. The event id is defined in the gce header
> +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

Where are the constraints?

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: mailbox: mediatek,gce-mailbox: Add mediatek,gce-events definition

[Jason-JH Lin (林睿祥)]

Hi Krzysztof,

Thanks for the reviews.

On Wed, 2023-12-20 at 08:07 +0100, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 18/12/2023 09:36, Jason-JH.Lin wrote:
> > Since mediatek,gce-events property is a HW event signal from GCE,
> > it should be defined in mediatek,gce-mailbox.yaml and change the
> 
> Why? None of the bindings reference it. Did you just move property
> from
> one device to completely different one?
> 
> > description of mediatek,gce-events property in other bindings to
> > reference mediatek,gce-mailbox.yaml.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  .../devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml  | 7
> +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git
> a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml 
> b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> > index cef9d7601398..e4da0a58c943 100644
> > --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> > +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-
> mailbox.yaml
> > @@ -49,6 +49,13 @@ properties:
> >      items:
> >        - const: gce
> >  
> > +  mediatek,gce-events:
> > +    description:
> > +      The event id which is mapping to the specific hardware event
> signal
> > +      to gce. The event id is defined in the gce header
> > +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Where are the constraints?
> 
Since the GCE HW event id rage is 0~1023,
I would add minItems: 1 and maxItems: 1024 here.

Regards
Jason-JH.Lin

> Best regards,
> Krzysztof
> 

[PATCH 2/3] dt-bindings: media: mediatek-mdp: Change the description of gce-events

[Jason-JH.Lin]

Change the description of mediatek,gce-events property to reference
mediatek,gce-mailbox.yaml.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../devicetree/bindings/media/mediatek,mdp3-rdma.yaml        | 5 +----
 .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml         | 5 +----
 .../devicetree/bindings/media/mediatek,mdp3-wrot.yaml        | 5 +----
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
index 7032c7e15039..adb3df4731f6 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
@@ -40,10 +40,7 @@ properties:
 
   mediatek,gce-events:
     description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
 
   power-domains:
     maxItems: 1
diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
index 78f9de6192ef..ddf66e3d5ad7 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rsz.yaml
@@ -36,10 +36,7 @@ properties:
 
   mediatek,gce-events:
     description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
 
   clocks:
     minItems: 1
diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
index 0baa77198fa2..ca37b45e24b4 100644
--- a/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
+++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-wrot.yaml
@@ -36,10 +36,7 @@ properties:
 
   mediatek,gce-events:
     description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
 
   power-domains:
     maxItems: 1
-- 
2.18.0

Re: [PATCH 2/3] dt-bindings: media: mediatek-mdp: Change the description of gce-events

[Krzysztof Kozlowski]

On 18/12/2023 09:36, Jason-JH.Lin wrote:
> Change the description of mediatek,gce-events property to reference
> mediatek,gce-mailbox.yaml.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../devicetree/bindings/media/mediatek,mdp3-rdma.yaml        | 5 +----
>  .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml         | 5 +----
>  .../devicetree/bindings/media/mediatek,mdp3-wrot.yaml        | 5 +----
>  3 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> index 7032c7e15039..adb3df4731f6 100644
> --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-rdma.yaml
> @@ -40,10 +40,7 @@ properties:
>  
>    mediatek,gce-events:
>      description:
> -      The event id which is mapping to the specific hardware event signal
> -      to gce. The event id is defined in the gce header
> -      include/dt-bindings/gce/<chip>-gce.h of each chips.
> -    $ref: /schemas/types.yaml#/definitions/uint32-array
> +      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.

There is no reference here. You allow now any type. This does not make
any sense.

Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: media: mediatek-mdp: Change the description of gce-events

[Jason-JH Lin (林睿祥)]

Hi 

Thanks for the reviews.

On Wed, 2023-12-20 at 08:06 +0100, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 18/12/2023 09:36, Jason-JH.Lin wrote:
> > Change the description of mediatek,gce-events property to reference
> > mediatek,gce-mailbox.yaml.
> > 
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > ---
> >  .../devicetree/bindings/media/mediatek,mdp3-rdma.yaml        | 5
> +----
> >  .../devicetree/bindings/media/mediatek,mdp3-rsz.yaml         | 5
> +----
> >  .../devicetree/bindings/media/mediatek,mdp3-wrot.yaml        | 5
> +----
> >  3 files changed, 3 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml b/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml
> > index 7032c7e15039..adb3df4731f6 100644
> > --- a/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml
> > +++ b/Documentation/devicetree/bindings/media/mediatek,mdp3-
> rdma.yaml
> > @@ -40,10 +40,7 @@ properties:
> >  
> >    mediatek,gce-events:
> >      description:
> > -      The event id which is mapping to the specific hardware event
> signal
> > -      to gce. The event id is defined in the gce header
> > -      include/dt-bindings/gce/<chip>-gce.h of each chips.
> > -    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      Reference to
> Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
> 
> There is no reference here. You allow now any type. This does not
> make
> any sense.

Since this property for the GCE HW are MediaTek specific, I will try
to refer to nvidia,tegra210-quad-peripheral-props.yaml and send a
series for creating a mediatek,gce-props.yaml to list all these
properties related to GCE. And change all these properties in mailbox
consumers to reference mediatek,gce-props.yaml.

allOf:
  - $ref: mediatek,gce-props.yaml


Regards,
Jason-JH.Lin

> 
> Best regards,
> Krzysztof
> 

[PATCH 3/3] dt-bindings: soc: mediatek: Change the description of gce-events

[Jason-JH.Lin]

Change the description of mediatek,gce-events property to reference
mediatek,gce-mailbox yaml.

Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
---
 .../devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml     | 5 +----
 .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml     | 5 +----
 .../devicetree/bindings/soc/mediatek/mediatek,wdma.yaml      | 5 +----
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
index 4380b98b0dfe..64c0f99fcbce 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
@@ -36,10 +36,7 @@ properties:
 
   mediatek,gce-events:
     description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
 
   clocks:
     minItems: 1
diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
index ba2014a8725c..34b313154534 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,mutex.yaml
@@ -55,10 +55,7 @@ properties:
 
   mediatek,gce-events:
     description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
 
   mediatek,gce-client-reg:
     $ref: /schemas/types.yaml#/definitions/phandle-array
diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
index 69afb329e5f4..ff515cf8394b 100644
--- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
+++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,wdma.yaml
@@ -37,10 +37,7 @@ properties:
 
   mediatek,gce-events:
     description:
-      The event id which is mapping to the specific hardware event signal
-      to gce. The event id is defined in the gce header
-      include/dt-bindings/gce/<chip>-gce.h of each chips.
-    $ref: /schemas/types.yaml#/definitions/uint32-array
+      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.
 
   power-domains:
     maxItems: 1
-- 
2.18.0

Re: [PATCH 3/3] dt-bindings: soc: mediatek: Change the description of gce-events

[Krzysztof Kozlowski]

On 18/12/2023 09:36, Jason-JH.Lin wrote:
> Change the description of mediatek,gce-events property to reference
> mediatek,gce-mailbox yaml.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml     | 5 +----
>  .../devicetree/bindings/soc/mediatek/mediatek,mutex.yaml     | 5 +----
>  .../devicetree/bindings/soc/mediatek/mediatek,wdma.yaml      | 5 +----
>  3 files changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
> index 4380b98b0dfe..64c0f99fcbce 100644
> --- a/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
> +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,ccorr.yaml
> @@ -36,10 +36,7 @@ properties:
>  
>    mediatek,gce-events:
>      description:
> -      The event id which is mapping to the specific hardware event signal
> -      to gce. The event id is defined in the gce header
> -      include/dt-bindings/gce/<chip>-gce.h of each chips.
> -    $ref: /schemas/types.yaml#/definitions/uint32-array
> +      Reference to Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml.

NAK, test your bindings. You allow now any type.


Best regards,
Krzysztof

Re: [PATCH 0/3] Add mediatek,gce-events definition to mediatek,gce-mailbox bindings

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1005 bytes --]

On Mon, Dec 18, 2023 at 04:36:01PM +0800, Jason-JH.Lin wrote:
> From: Jason-jh Lin <jason-jh.lin@mediatek.corp-partner.google.com>
> 
> Since mediatek,gce-events property is a HW event signal from GCE,
> it should be defined in mediatek,gce-mailbox.yaml.
> 
> Change the description of mediatek,gce-events property existed in
> other bindings to reference mediatek,gce-mailbox.yaml.

I don't understand this series. I would understand it if the property
should be related to the mailbox provider and it is moved there from the
mailbox consumer, but this series does not do that. Instead the series
now documents this property for both consumers and providers.

Secondly it removes the typedef from the consumers, which makes no sense
if this is a valid property there.

Is your intention to document a property that should be common across
all consumers in a single place? If that is your goal, then something
like spi-peripheral-props.yaml is what you need here.

Confused,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/3] Add mediatek,gce-events definition to mediatek,gce-mailbox bindings

[Jason-JH Lin (林睿祥)]

Hi Conor,

Thanks for the reviews.

On Tue, 2023-12-19 at 16:54 +0000, Conor Dooley wrote:
> On Mon, Dec 18, 2023 at 04:36:01PM +0800, Jason-JH.Lin wrote:
> > From: Jason-jh Lin <jason-jh.lin@mediatek.corp-partner.google.com>
> > 
> > Since mediatek,gce-events property is a HW event signal from GCE,
> > it should be defined in mediatek,gce-mailbox.yaml.
> > 
> > Change the description of mediatek,gce-events property existed in
> > other bindings to reference mediatek,gce-mailbox.yaml.
> 
> I don't understand this series. I would understand it if the property
> should be related to the mailbox provider and it is moved there from
> the
> mailbox consumer, but this series does not do that. Instead the
> series
> now documents this property for both consumers and providers.
I want to move properties related to MediaTek's proprietary GCE HW to a
common place and let it referenced by mailbox providers and consumers.

And I also want to add this property into a mailbox provider, but maybe
not in this series. I'll move this modification into another series.

> 
> Secondly it removes the typedef from the consumers, which makes no
> sense
> if this is a valid property there.
> 
OK, I will keep the $ref for the consumers.

> Is your intention to document a property that should be common across
> all consumers in a single place? If that is your goal, then something
> like spi-peripheral-props.yaml is what you need here.
> 
Thanks for your example.

Since these properties for the GCE HW are MediaTek specific, I will try
to send a series to create a mediatek,gce-props.yaml to list all these
properties. And modify all these properties in mailbox consumers.

Regards,
Jason-JH.Lin

> Confused,
> Conor.