Re: [PATCH v2 04/15] dt-bindings: display: mediatek: padding: Add documentation for MT8188

[Shawn Sung (宋孝謙)]

Hi Krzysztof,

Got it, the new title will be “dt-bindings: display: mediatek: padding:
Add MT8188”.

Since “PADDING” is not an acronym but just padding, in this case is
used to pad or stuff pixels to layers, I’ll change all of them to
“Padding” in the next version.

For “MediaTek Foo Bar Padding” suggestion, I changed it to “MediaTek
Display Padding”, hope it could make more sense.

Thanks,
Hsiao Chien Sung

On Thu, 2023-06-15 at 10:32 +0200, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 14/06/2023 09:31, Hsiao Chien Sung wrote:
> > PADDING is a new hardware module on MediaTek MT8188,
> > Add device tree bindings documentation for it.
> > 
> 
> A nit, subject: drop second/last, redundant "documentation for". The
> "dt-bindings" prefix is already stating that these are bindings and
> documentation.
> 
> > Signed-off-by: Hsiao Chien Sung <shawn.sung@xxxxxxxxxxxx>
> > ---
> >  .../display/mediatek/mediatek,padding.yaml    | 81
> +++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek,padding.y
> aml
> > 
> > diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,padding
> .yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding
> .yaml
> > new file mode 100644
> > index 000000000000..390a518fa2cf
> > --- /dev/null
> > +++
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,padding
> .yaml
> > @@ -0,0 +1,81 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: 
> http://devicetree.org/schemas/display/mediatek/mediatek,padding.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek PADDING
> 
> MediaTek Foo Bar Padding
> 
> Please explain what is this. PADDING does not look like acronym. If
> it
> is, expand it.
> 
> > +
> > +maintainers:
> > +  - Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx>
> > +  - Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> > +
> > +description:
> > +  MediaTek PADDING provides ability to VDOSYS1 to add pixels to
> width and height
> 
> Expand the acronym.
> 
> > +  of a layer with a specified color.
> > +  Since MIXER in VDOSYS1 requires the width of a layer to be 2-
> pixel-align, or
> > +  4-pixel-align when ETHDR is enabled, we need PADDING to deal
> with odd width.
> > +  Please notice that even if the PADDING is in bypass mode,
> settings in the
> > +  registers must be cleared to 0, or undefined behaviors could
> happen.
> > +
> > +properties:
> > +  compatible:
> > +    const: mediatek,mt8188-padding
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: RDMA Clock
> > +
> > +  mediatek,gce-client-reg:
> > +    description:
> > +      GCE (Global Command Engine) is a multi-core micro processor
> that helps
> > +      its clients to execute commands without interrupting CPU.
> This property
> > +      describes GCE client's information that is composed by 4
> fields.
> > +      1. pHandle of the GCE (there may be several GCE processors)
> > +      2. Sub-system ID defined in the dt-binding like a user ID
> > +         (Please refer to include/dt-bindings/gce/<chip>-gce.h)
> > +      3. Offset from base address of the subsys you are at
> > +      4. Size of the register the client needs
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    items:
> > +      items:
> > +        - description: pHandle of the GCE
> 
> Phandle (if first in sentence) or phandle. It's not a pH unit. Fix it
> in
> other places as well.
> 
> 
> > +        - description: Subsys ID defined in the dt-binding
> > +        - description: Offset from base address of the subsys
> > +        - description: Size of register
> 
> 
> Best regards,
> Krzysztof
>