Re: [PATCH 2/5] arm64: dts: qcom: Add Kalambo SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The bandwidth monitor (`bwmon_cluster0`) for the removed CPU cluster 0 is neither disabled nor removed.
- [High] Thermal sensors (`tsens0`) and thermal zones monitoring the removed CPU cluster 0 are left active.
- [Medium] Incomplete update of SCMI SCP low-priority shared memory register windows creates an asymmetric channel configuration.
--

--- Patch [2]: [PATCH 2/5] arm64: dts: qcom: Add Kalambo SoC ---
commit c5f01045fbad11461a4f9c8629b706f4e0af8944
Author: Gopikrishna Garmidi <gopikrishna.garmidi@oss.qualcomm.com>

arm64: dts: qcom: Add Kalambo SoC

Introduce support for the Qualcomm Kalambo SoC. It is derived from Mahua
with CPU cluster 0 removed, leaving it with just cluster 1. As a result,
the PDP mailbox and SCP low-priority register windows are updated to
match the reduced CPU complex. Everything else should work as-is.

[ ... ]

> diff --git a/arch/arm64/boot/dts/qcom/kalambo.dtsi b/arch/arm64/boot/dts/qcom/kalambo.dtsi
> new file mode 100644
> index 0000000000000..a56ff68b57b47
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/kalambo.dtsi
> @@ -0,0 +1,29 @@
> +// SPDX-License-Identifier: BSD-3-Clause
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#include "mahua.dtsi"
> +
> +/delete-node/ &cluster0_pd;
> +/delete-node/ &cpu_map_cluster0;

[Severity: High]
Since cluster 0 is removed, does the bandwidth monitor (bwmon_cluster0) also
need to be disabled?

The qcom,glymur-cpu-bwmon driver automatically probes bwmon_cluster0 during
boot. If left active, it will attempt to map and access MMIO registers at
0x0100c400 for a physical cluster that no longer exists in the SoC, which
can result in a hardware/bus fault (SError) and system crash.

Looking at glymur.dtsi, the node cannot be safely deleted with /delete-node/
because it houses the cpu_bwmon_opp_table used by the remaining clusters:

		bwmon_cluster0: pmu@100c400 {
			compatible = "qcom,glymur-cpu-bwmon", "qcom,sdm845-bwmon";

Should this be explicitly marked with status = "disabled" instead?

[Severity: High]
Similarly, should the thermal sensors (tsens0) and the thermal zones
monitoring cluster 0 be explicitly removed?

The thermal framework will initialize and poll the active cpu-0-*-thermal
zones, which attempt to read from tsens0 registers corresponding to the
absent cluster 0. This can also trigger an SError crashing the system or
cause fatal thermal subsystem errors.

For example, glymur.dtsi defines these zones:

		cpu-0-0-0-thermal {
			thermal-sensors = <&tsens0 1>;

Should &tsens0 and the un-labeled cluster 0 thermal zones inside
&thermal_zones be deleted?

> +/delete-node/ &cpu0;
> +/delete-node/ &cpu1;
> +/delete-node/ &cpu2;
> +/delete-node/ &cpu3;
> +/delete-node/ &cpu4;
> +/delete-node/ &cpu5;
> +/delete-node/ &cpu_pd0;
> +/delete-node/ &cpu_pd1;
> +/delete-node/ &cpu_pd2;
> +/delete-node/ &cpu_pd3;
> +/delete-node/ &cpu_pd4;
> +/delete-node/ &cpu_pd5;
> +
> +&pdp0_mbox {
> +	reg = <0 0x17610000 0 0x8000>, <0 0x1a980000 0 0x8000>;
> +};
> +
> +&cpu_scp_lpri1 {
> +	reg = <0x180 0x60>;
> +};

[Severity: Medium]
Does the TX window (cpu_scp_lpri0) need to be updated to match the
reduced CPU complex as well?

The commit shrinks the RX window size (cpu_scp_lpri1) from 0x180 to 0x60,
but misses the TX window, which remains oversized at 0x180:

			cpu_scp_lpri0: scp-sram-section@0 {
				compatible = "arm,scmi-shmem";
				reg = <0x0 0x180>;
			};

If the system firmware expects both windows to be symmetrically shrunk and
contiguous, this asymmetric configuration might cause SCMI initialization to
fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718-b4-kalambo-crd-v1-0-a1069b5a2757@oss.qualcomm.com?part=2