Re: [PATCH v13 2/2] arm64: dts: rockchip: Add ALIENTEK QuarkPi-CA2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] arm64: dts: rockchip: non-power-of-two ramoops sizes in quarkpi-ca2
- [Low] arm64: dts: rockchip: unreferenced combophy regulators and unused include
- [Low] arm64: dts: rockchip: duplicate headphone DAPM routes in QuarkPi-CA2
--

commit b00e8823ca510ece58a0c11ef8ab0946698d2e72
Author: Bob Hui <bg9oxa@163.com>

arm64: dts: rockchip: Add ALIENTEK QuarkPi-CA2

This commit adds the device tree for the ALIENTEK QuarkPi-CA2, an RK3588S
based card computer, describing its regulators, storage, display, audio,
USB, PCIe, and expansion interfaces.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts
> new file mode 100644
> index 0000000000000..df9887d5dc925
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts
> @@ -0,0 +1,1211 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2021 Rockchip Electronics Co., Ltd.
> + */
> +
> +/dts-v1/;
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +#include <dt-bindings/media/video-interfaces.h>
> +#include <dt-bindings/pinctrl/rockchip.h>

[ ... ]

> +	analog_sound: analog-sound {
> +		compatible = "simple-audio-card";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&hp_det>;
> +		simple-audio-card,name = "QuarkPi-CA2 ES8388";
> +		simple-audio-card,format = "i2s";
> +		simple-audio-card,bitclock-master = <&sound_cpu>;
> +		simple-audio-card,frame-master = <&sound_cpu>;
> +		simple-audio-card,mclk-fs = <256>;
> +		simple-audio-card,aux-devs = <&hp_amp>;
> +		simple-audio-card,hp-det-gpios = <&gpio1 RK_PC4 GPIO_ACTIVE_HIGH>;
> +		simple-audio-card,pin-switches = "Headphones", "Headset Mic";
> +		simple-audio-card,widgets =
> +			"Headphone", "Headphones",
> +			"Microphone", "Headset Mic";
> +		simple-audio-card,routing =
> +			"Headphones", "LOUT1",
> +			"Headphones", "ROUT1",
> +			"Headphones", "Headphone Power OUTL",
> +			"Headphones", "Headphone Power OUTR",
> +			"Headphone Power INL", "LOUT1",
> +			"Headphone Power INR", "ROUT1",
> +			"LINPUT1", "Headset Mic",
> +			"RINPUT1", "Headset Mic";

[Severity: Low]
As reported by Sashiko on v12 [1], in analog_sound, when
simple_util_parse_routing() parses simple-audio-card,routing, it registers
both direct routes from LOUT1 and ROUT1 to Headphones and amplified routes
through the hp_amp simple-audio-amplifier (LOUT1/ROUT1 -> Headphone Power
INL/INR -> Headphone Power OUTL/OUTR -> Headphones).

Since the headphone outputs are routed through hp_amp, should the direct
"Headphones", "LOUT1" and "Headphones", "ROUT1" entries be dropped so the
DAPM graph does not contain duplicate parallel paths in
dapm_is_connected_ep()?

[ ... ]

> +	combophy_avdd0v85: regulator-combophy-avdd0v85 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "combophy_avdd0v85";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <850000>;
> +		regulator-max-microvolt = <850000>;
> +		vin-supply = <&vdd_0v85_s0>;
> +	};
> +
> +	combophy_avdd1v8: regulator-combophy-avdd1v8 {
> +		compatible = "regulator-fixed";
> +		regulator-name = "combophy_avdd1v8";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <1800000>;
> +		regulator-max-microvolt = <1800000>;
> +		vin-supply = <&avcc_1v8_s0>;
> +	};

[Severity: Low]
Are combophy_avdd0v85 and combophy_avdd1v8 needed here? Neither regulator is
referenced by any consumer in rk3588s-quarkpi-ca2.dts or rk3588s.dtsi, and
both are always-on fixed regulators without GPIO control that simply mirror
their parent supplies (&vdd_0v85_s0 and &avcc_1v8_s0).

Also, the <dt-bindings/media/video-interfaces.h> include at the top of
rk3588s-quarkpi-ca2.dts appears to be unused. Could these unreferenced
regulator nodes and the unused header include be removed?

[ ... ]

> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		ramoops: ramoops@118000 {
> +			compatible = "ramoops";
> +			reg = <0 0x118000 0 0xd8000>;
> +			console-size = <0x80000>;
> +			ftrace-size = <0x00000>;
> +			pmsg-size = <0x30000>;
> +			record-size = <0x14000>;
> +		};
> +	};

[Severity: High]
Can these non-power-of-two pmsg-size (0x30000) and record-size (0x14000)
values in ramoops@118000 cause misconfigured zone sizes and alignment faults
at boot when CONFIG_PSTORE_RAM is enabled?

In fs/pstore/ram.c, ramoops_probe() rounds non-power-of-two zone sizes down
to powers of two:

    if (pdata->record_size && !is_power_of_2(pdata->record_size))
        pdata->record_size = rounddown_pow_of_two(pdata->record_size);
    ...
    if (pdata->pmsg_size && !is_power_of_2(pdata->pmsg_size))
        pdata->pmsg_size = rounddown_pow_of_two(pdata->pmsg_size);

This reduces pmsg_size from 0x30000 to 0x20000 and record_size from 0x14000
to 0x10000, leaving dump_mem_sz = 0xd8000 - 0x80000 - 0x20000 = 0x38000.

When ramoops_probe() -> ramoops_init_przs() initializes the dump zones, it
computes:

    *cnt = mem_sz / record_size; /* 0x38000 / 0x10000 = 3 */
    zone_sz = mem_sz / *cnt;
    zone_sz = ALIGN_DOWN(zone_sz, 2); /* 0x12aaa */

Advancing paddr by 0x12aaa places subsequent persistent_ram_buffer headers at
2-byte-aligned physical addresses (0x12aaaa, 0x14fffe, and 0x1cfffe, with the
latter two straddling 4K page boundaries). When persistent_ram_new() ->
persistent_ram_post_init() -> buffer_size() calls atomic_read() on the start
and size fields of struct persistent_ram_buffer, arm64 kernels built with
CONFIG_LTO emit load-acquire instructions (ldar/ldapr) that require 4-byte
alignment and fault on 2-byte-aligned addresses.

Should console-size, pmsg-size, and record-size be power-of-two values that
divide the ramoops reg size into power-of-two zones?

[1] https://lore.kernel.org/r/20261102-quarkpi-ca2-v12-2-d749d9bb2bf9@163.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-quarkpi-ca2-v13-0-ad8c30cdb67e@163.com?part=2