Hey Konrad,
On 10/26/23 16:06, Konrad Dybcio wrote:
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? */
will do.
[...]
+ 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?
ack will remove the l1 and use ^^ appropriately.
[...]
+ 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
will fix this.
[...]
+ 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
ok, will do but all the previous SoCs do it differently.
+ pld_pep_mem: pld-pep-region@81f30000 {
What's PLD?
What's this region used for? PEP is a Windows invention.
We list all the possible reserved memory regions from the reference doc.
I can remove the unused regions in the platforms dts later once this
series lands.
[...]
+ av1_encoder_mem: av1-encoder-region@8e900000 {
Is AV1enc hardware separate from iris?
no, it isn't separate from iris AFAIK.
[...]
+ 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.
thanks for catching this.
+
+ 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)
will do.
[...]
+
+ interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>,
One space after and before '='
ack
Konrad
