Re: [PATCH v3 7/7] arm64: dts: imx8dxl-evk: add lcdif overlay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: imx8dxl-evk: invalid and ignored bus-width in adma_lcdif endpoint
- [Medium] arm64: dts: imx8dxl-evk: incomplete audio disabling in lcdif overlay
--

commit 99ed2360e1f19a7e9e6652f9b6385f7a7a06174f
Author: Frank Li <Frank.Li@nxp.com>

arm64: dts: imx8dxl-evk: add lcdif overlay

This commit adds a device tree overlay to support the waiken,101wx001-wct LCD
panel on the i.MX8DXL EVK board, configuring the LCDIF controller pins and
disabling conflicting peripherals.

> diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-evk-lcdif-101wx001.dtso b/arch/arm64/boot/dts/freescale/imx8dxl-evk-lcdif-101wx001.dtso
> new file mode 100644
> index 0000000000000..39abf9b3884ea
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/imx8dxl-evk-lcdif-101wx001.dtso
> @@ -0,0 +1,119 @@

[ ... ]

> +&acm {
> +	status = "disabled";
> +};
> +
> +&adma_lcdif {
> +	pinctrl-0 = <&pinctrl_lcdif>, <&lcd_0_fun>, <&lcd_1_fun>, <&lcd_2_fun>, <&lcd_3_fun>;
> +	pinctrl-names = "default";
> +	status = "okay";
> +
> +	port {
> +		lcdif_out: endpoint {
> +			bus-width = <18>;
> +			remote-endpoint = <&panel_in>;
> +		};
> +	};
> +};

[Severity: High]
Is bus-width = <18> in lcdif_out valid for the fsl,lcdif binding or handled
by the mxsfb driver?

In Documentation/devicetree/bindings/display/fsl,lcdif.yaml, the port
property references /schemas/graph.yaml#/properties/port directly without
video-interfaces.yaml:

  port:
    $ref: /schemas/graph.yaml#/properties/port
    description: The LCDIF output port

Because /schemas/graph.yaml#/properties/port enforces additionalProperties:
false on endpoint nodes, adding bus-width = <18> causes dtbs_check schema
validation to fail on imx8dxl-evk-lcdif-101wx001.dtb.

In addition, neither mxsfb, drm_panel_bridge, nor panel-simple parses
bus-width from the endpoint node. When mxsfb_crtc_atomic_enable() calls
mxsfb_set_formats() in drivers/gpu/drm/mxsfb/mxsfb_kms.c, the hardware bus
width is selected from bridge_state->input_bus_cfg.format:

	switch (bus_format) {
	case MEDIA_BUS_FMT_RGB565_1X16:
		ctrl |= CTRL_BUS_WIDTH_16;
		break;
	case MEDIA_BUS_FMT_RGB666_1X18:
		ctrl |= CTRL_BUS_WIDTH_18;
		break;
	case MEDIA_BUS_FMT_RGB888_1X24:
		ctrl |= CTRL_BUS_WIDTH_24;
		break;

Since waiken_101wx001_panel in drivers/gpu/drm/panel/panel-simple.c sets
bus_format to MEDIA_BUS_FMT_RGB888_1X24:

static const struct panel_desc waiken_101wx001_panel = {
	...
	.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
	.connector_type = DRM_MODE_CONNECTOR_DPI,
	.bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE,
};

mxsfb_set_formats() will program CTRL_BUS_WIDTH_24 instead of
CTRL_BUS_WIDTH_18 even though only 18 data lines (D00..D17) are wired in
pinctrl_lcdif, dropping the upper 6 bits of each pixel.

[ ... ]

> +&sai0 {
> +	status = "disabled";
> +};
> +
> +&sai1 {
> +	status = "disabled";
> +};
> +
> +&wm8960_1 {
> +	status = "disabled";
> +};
> +
> +&wm8960_2 {
> +	status = "disabled";
> +};
> +
> +&wm8960_3 {
> +	status = "disabled";
> +};

[Severity: Medium]
Should this overlay also disable the remaining audio nodes in
arch/arm64/boot/dts/freescale/imx8dxl-evk.dts and
arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi that depend on &acm, &sai0,
&sai1, and &wm8960_1..&wm8960_3?

Because of_link_to_phandle() in drivers/of/property.c does not create
fw_devlink links to suppliers with status = "disabled", the enabled consumer
devices still attempt to probe when this overlay is applied:

1. The audio LPCG nodes in arch/arm64/boot/dts/freescale/imx8-ss-audio.dtsi
(spdif0_lpcg, sai0_lpcg, sai1_lpcg, sai2_lpcg, sai3_lpcg, mclkout0_lpcg,
mclkout1_lpcg, and mqs0_lpcg) remain enabled and reference disabled &acm. In
drivers/clk/imx/clk-imx8qxp-lpcg.c, imx8qxp_lpcg_clk_probe() ->
imx_lpcg_parse_clks_from_dt() fails with -ENODEV:

	ret = of_clk_parent_fill(np, parent_names, count);
	if (ret != count) {
		dev_err(&pdev->dev, "failed to get clock parent names\n");
		return count;
	}

2. &asrc0, &sai2, and &sai3 remain enabled in
arch/arm64/boot/dts/freescale/imx8dxl-evk.dts and fail clock acquisition in
fsl_asrc_probe() and fsl_sai_probe() because their parent clocks in &acm,
sai2_lpcg, and sai3_lpcg are unavailable.

3. The four sound card nodes in arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
(sound-bt-sco, sound-wm8960-1, sound-wm8960-2, and sound-wm8960-3) remain
enabled while referencing the disabled &sai0, &sai1, and &wm8960_1..&wm8960_3
nodes:

	sound-bt-sco {
		compatible = "simple-audio-card";
		...
		btcpu: simple-audio-card,cpu {
			sound-dai = <&sai0>;
		...
	};

	sound-wm8960-1 {
		compatible = "fsl,imx-audio-wm8960";
		model = "wm8960-audio";
		audio-cpu = <&sai1>;
		audio-codec = <&wm8960_1>;
		audio-asrc = <&asrc0>;

causing simple_probe(), fsl_asoc_card_probe(), and graph_probe() to defer
probe indefinitely on boot.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-dxl_lcdif-v3-0-adc944659e14@nxp.com?part=7