Re: [PATCH 03/24] arm64: dts: qcom: Introduce Glymur base dtsi and CRD dts

Konrad Dybcio
2025-09-25T10:16:33Z

On 9/25/25 8:32 AM, Pankaj Patil wrote:
> Introduce initial device tree support for Glymur - Qualcomm's
> next-generation compute SoC and it's associated Compute Reference
> Device (CRD) platform.
>
> The dt describes CPUs, CPU map, GCC and RPMHCC clock controllers,
> geni UART, interrupt controller, TLMM, reserved memory,
> interconnects, SMMU, firmware scm, watchdog, apps rsc, RPMHPD,
> SRAM, PSCI and pmu nodes.
>
> Signed-off-by: Pankaj Patil <pankaj.patil@oss.qualcomm.com>
> ---

[...]

> +#include "glymur.dtsi"
> +
> +/ {
> + model = "Qualcomm Technologies, Inc. Glymur CRD";
> + compatible = "qcom,glymur-crd", "qcom,glymur";
> +
> + aliases {
> + serial0 = &uart21;
> + };
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> +&tlmm {
> + gpio-reserved-ranges = <4 4>, <10 2>, <44 4>; /*Security SPI (TPM)*/

Please add a space between the comment begin/end markers and the content

> +};
> diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
> new file mode 100644
> index 0000000000000000000000000000000000000000..f1c5a0cb483670e9f8044e250950693b4a015479
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
> @@ -0,0 +1,1320 @@
> +// SPDX-License-Identifier: GPL-2.0-only

This is different..

[...]

> + cpu0: cpu0@0 {

"cpu@"

> + device_type = "cpu";
> + compatible = "qcom,oryon";

We've beaten this horse to death, over and over again.

This compatible is meaningless, incorrect and shall not be merged

[...]

> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <&cpu0>;
> + };
> + core1 {

Please ensure a \n between subsequent subnodes

[...]

> + soc: soc@0 {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + compatible = "simple-bus";

compatible goes first

[...]

> + tcsrcc: clock-controller@1fd5044 {
> + compatible = "qcom,glymur-tcsr";
> + reg = <0x0 0x1fd5044 0x0 0x48>;

We can map 0x1fd5000 - 0x1fd5094 inclusive, as that seems like a
logical subblock (this would require adjusting the driver)

There's also a laaaarge pool of various TCSR_ registers between
the previous node and this one.. but we can leave that in case we
need to describe it in a specific way some day
> + #clock-cells = <1>;
> + #reset-cells = <1>;
> + };
> +
> + tcsr: syscon@1fd6000 {
> + compatible = "syscon";

"syscon" alone was not allowed by the dt checker at one point..

> + reg = <0x0 0x1fd6000 0x0 0x20000>;
> + };

[...]

> + frame@17811000 {
> + reg = <0x0 0x17811000 0x0 0x1000>,
> + <0x0 0x17812000 0x0 0x1000>;

Odd spacing (more than 1 space after '=')

Konrad