Re: [PATCH v2 1/4] dt-bindings: mailbox: Add mediatek,gce-props.yaml

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

On 10/01/2024 07:35, Jason-JH.Lin wrote:
> Add mediatek,gce-props.yaml for specific GCE properties for both
> Mailbox Providers and Consumers.
> 
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> ---
>  .../bindings/mailbox/mediatek,gce-props.yaml  | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> 
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> new file mode 100644
> index 000000000000..aac776b74e88
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-props.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mailbox/mediatek,gce-props.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Global Command Engine common propertes for both Mailbox Providers and Consumers.

That's way too long. Please observe coding style wrapping. Just MediaTek
Global Command Engine Common Propertes
(pay attention to capitalization)

> +
> +maintainers:
> +  - Houlong Wei <houlong.wei@mediatek.com>
> +
> +description:
> +  The Global Command Engine (GCE) is used to implement a Command Queue (CMDQ)
> +  driver using the Linux Mailbox framework. The GCE is an instruction based,
> +  multi-threaded, single-core command dispatcher for MediaTek hardware.
> +  We use GCE Mailbox binding to define GCE core properties for GCE Mailbox Provider.
> +  A device that uses the CMDQ driver to confige its hardware registers by requesting

configure?

> +  the Linux Mailbox Channels in the GCE Mailbox Controller is a Mailbox Consumer.
> +  This binding defines the common GCE properties for both Mailbox Providers and Consumers.

Last four sentences: a lot of text yet absolutely no meaning. I still do
not understand why same property is shared between provider and consumer.

Also, wrap according to Linux coding style.

> +
> +properties:
> +  mediatek,gce-events:
> +    description:
> +      Each gce-events is an event id corresponding to a specific hardware event
> +      signal sent to GCE. The event id is defined in the GCE header
> +      include/dt-bindings/gce/<chip>-gce.h of each chips.
> +      CMDQ client drivers have two usage of GCE event signals,
> +      one is sfotware tokens and the other is hardware events.
> +      Software tokens refer to GCE event signals triggered by drivers.
> +      e.g. software drivers append GCE commands to set a GCE event after specific
> +      GCE commands. Or CMDQ client driver uses the CPU to write the event id
> +      into GCE register to trigger the GCE event corresponding to the event id.
> +      Hardware events refer to GCE event signals triggered by hardware engines.
> +      e.g. When OVL fetches all the data in the frame buffer, OVL will send

There is no full stop before "e.g." and no new sentence. Please run this
through grammar check, like Google, ChatGPT or human.

> +      a frame done irq and send a frame done GCE event via hardware bus directly
> +      at the same time.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    minItems: 1
> +    maxItems: 1024


Best regards,
Krzysztof