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

From: Jason-JH Lin (林睿祥)
Date: Wed Dec 20 2023 - 02:37:05 EST


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@xxxxxxxxxxxx>
> > ---
> > .../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
>