Re: [PATCH v5 5/5] arm64: dts: imx8mq-librem5: Correct link frequency list

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The link frequency validation loop incorrectly tests dummy modes that are unsupported by the active MIPI lane configuration, demanding dummy frequencies in the device tree.
- [Medium] The driver validates link frequencies before obtaining its clock, causing the validation check to evaluate to 0 and silently bypass error handling.
--

--- Patch [5]: [PATCH v5 5/5] arm64: dts: imx8mq-librem5: Correct link frequency list ---
commit 7374317f64e34ad7703aeaa4a94012403d0aca4d
Author: Pengyu Luo <mitltlatltl@gmail.com>

arm64: dts: imx8mq-librem5: Correct link frequency list

This patch updates the link frequencies for the hi846 camera sensor on the
imx8mq-librem5 board to 187.5MHz and 375MHz based on the datasheet.

> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> index f5d529c5baf3e..cc5604172192c 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> @@ -1122,7 +1122,7 @@ port {
>  			camera1_ep: endpoint {
>  				data-lanes = <1 2>;
>  				link-frequencies = /bits/ 64
> -					<80000000 200000000 300000000>;
> +					<187500000 375000000>;

[Severity: Medium]
Does the driver actually validate these new device tree link frequencies?

Looking at hi846_probe() in drivers/media/i2c/hi846.c, it calls
hi846_parse_dt() before obtaining hi846->clock. During parsing,
hi846_check_link_freqs() calls clk_get_rate(hi846->clock) on a NULL pointer,
returning 0.

When 0 is not found in the DT, it is returned as an unsupported frequency:

drivers/media/i2c/hi846.c:hi846_parse_dt() {
    ...
    if (fq) {
        dev_err(dev, "Link frequency of %lld is not supported\n", fq);
        ret = -EINVAL;
        goto check_hwcfg_error;
    }
    ...
}

Since fq evaluates to 0, the error handler is skipped and the driver silently
bypasses link frequency validation.

[Severity: High]
If the clock ordering bug in the driver is fixed, will probe fatally fail
for this 2-lane configuration?

The librem5 has a 2-lane setup, but hi846_check_link_freqs() in the driver
iterates over all supported_modes without filtering out modes that are
exclusive to 4-lanes:

drivers/media/i2c/hi846.c:hi846_check_link_freqs() {
    ...
    for (i = 0; i < freqs_count; i++) {
        link_freq = hi846_get_link_freq(hi846, &supported_modes[i]);
    ...
}

For 4-lane modes, the 2-lane divider is a dummy value. This causes
hi846_get_link_freq() to calculate an invalid 750MHz frequency:

drivers/media/i2c/hi846.c:hi846_get_link_freq() {
    ...
    return mul_u64_u32_div(mclk, 90, 3 * mipi_clk_div);
    ...
}

Because 750MHz is rightfully absent from these updated link-frequencies,
probe will abort once the NULL clock issue is resolved. Could the driver
be updated to filter modes by active lane count?

>  				remote-endpoint = <&mipi1_sensor_ep>;
>  			};
>  		};

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