Re: [PATCH 3/5] arm64: dts: qcom: Add base SC8380XP dtsi and the QCP dts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/25/23 16:24, Sibi Sankar wrote:
From: Rajendra Nayak <quic_rjendra@xxxxxxxxxxx>

Add base dtsi and QCP board (Qualcomm Compute Platform) dts file for
SC8380XP SoC, describing the CPUs, GCC and RPMHCC clock controllers,
geni UART, interrupt controller, TLMM, reserved memory, interconnects,
SMMU and LLCC nodes.

Co-developed-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
Signed-off-by: Abel Vesa <abel.vesa@xxxxxxxxxx>
Signed-off-by: Rajendra Nayak <quic_rjendra@xxxxxxxxxxx>
Co-developed-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
Signed-off-by: Sibi Sankar <quic_sibis@xxxxxxxxxxx>
---
[...]

+&tlmm {
+	gpio-reserved-ranges = <33 3>, <44 4>, <238 1>;
It would be really cool if you added an explanation on why these
GPIOs need to be reserved, especially since you can see what's
connected on there on schematics. So, like:

gpio-reserved-ranges = <33 3>, /* something */
		       <44 4>, /* something else (fp scanner?)
		       <238 1>; /* UFS reset? */


[...]

+			compatible = "qcom,oryon";
Again, this compatible won't fly unless all of these cores
are totally identical and Oryon is only a name for this
generation on this SoC (which I believe not to be the case).

+			reg = <0x0 0x0>;
+			enable-method = "psci";
+			next-level-cache = <&L1_0>;
+
+			L1_0: l1-cache {
+				compatible = "cache";
I'm not sure if L1 is supposed to be described in the DT,
Krzysztof should know.

+				next-level-cache = <&L2_0>;
+
+				L2_0: l2-cache-0 {
+					compatible = "cache";
cache-level?
cache-unified?

[...]

+	memory@80000000 {
+		device_type = "memory";
+		/* We expect the bootloader to fill in the size */
+		reg = <0 0x80000000 0 0x80000000>;
That contradicts the comment you made above. Plus, 2 GiB seems a
bit low for this SoC :D

[...]

+		gunyah_hyp_mem: gunyah-hyp-region@80000000 {
you can probably strip the "-region" part, as this is implied by
being a child of /reserved-memory

+		pld_pep_mem: pld-pep-region@81f30000 {
What's PLD?

What's this region used for? PEP is a Windows invention.

[...]

+		av1_encoder_mem: av1-encoder-region@8e900000 {
Is AV1enc hardware separate from iris?

[...]

+		gcc: clock-controller@100000 {
+			compatible = "qcom,sc8380xp-gcc";
+			reg = <0 0x100000 0 0x200000>;
The address part of reg should be padded to 8 hex digits.

+
+				interconnects = <&clk_virt MASTER_QUP_CORE_2 0 &clk_virt SLAVE_QUP_CORE_2 0>,
QCOM_ICC_TAG_ALWAYS would be nicer than 0 (see sa8775p)

[...]

+
+			interrupts =	<GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
One space after and before '='

Konrad




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux