Re: [PATCH] dt-bindings: display: imx: Document fsl,pcr

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: display: imx: fsl,pcr documented on controller node
--

commit 135b52931f21eeadf62c2f0174408c4c3faaf98d
Author: Fabio Estevam <festevam@gmail.com>

dt-bindings: display: imx: Document fsl,pcr

This commit documents the fsl,pcr property in fsl,imx-lcdc.yaml to address
dt-core.yaml validation warnings on i.MX25 and i.MX27 device trees.

> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> index 84610b5690c49..f1931436a4d81 100644
> --- a/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx-lcdc.yaml
> @@ -65,6 +65,12 @@ properties:
>      description:
>        LCDC Sharp Configuration Register value.
>  
> +  fsl,pcr:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      LCDC Panel Configuration Register value used in the display node
> +      referenced by the display property.

[Severity: Low]
Should fsl,pcr be defined here under the top-level properties of
fsl,imx-lcdc.yaml?

In drivers/video/fbdev/imxfb.c, controller properties such as fsl,lpccr,
fsl,lscr1, and fsl,dmacr are read from the LCD controller node in
imxfb_init_fbinfo(), whereas imxfb_probe() resolves the display phandle
and passes that separate node to imxfb_of_read_mode():

drivers/video/fbdev/imxfb.c:imxfb_of_read_mode() {
	...
	ret = of_property_read_u32(np, "bits-per-pixel", &bpp);
	ret |= of_property_read_u32(np, "fsl,pcr", &pcr);
	...
}

As the description notes and as shown in the second example of
fsl,imx-lcdc.yaml (display0), fsl,pcr is placed in the separate display node
referenced by the display phandle rather than in the LCD controller node
itself.

Defining fsl,pcr in the top-level properties of fsl,imx-lcdc.yaml silences
the dt-core.yaml type warning globally, but it also permits fsl,pcr on the
LCD controller node where the driver never reads it. In addition, the allOf
conditional for fsl,imx1-lcdc and fsl,imx21-lcdc disables display, fsl,dmacr,
fsl,lpccr, and fsl,lscr1, but does not set fsl,pcr: false.

Would it be more accurate to document the display node schema separately (or
disallow fsl,pcr on the controller node itself)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103224830.377009-1-festevam@gmail.com?part=1