Re: [PATCH v2 3/6] arm64: dts: qcom: add SC8280XP platform

From: Bjorn Andersson
Date: Wed Jun 22 2022 - 15:42:11 EST


On Wed 22 Jun 13:56 CDT 2022, Stephan Gerhold wrote:

> On Tue, Jun 21, 2022 at 09:12:21PM -0700, Bjorn Andersson wrote:
> > Introduce initial support for the Qualcomm SC8280XP platform, aka 8cx
> > Gen 3. This initial contribution supports SMP, CPUfreq, CPU cluster
> > idling, GCC, TLMM, SMMU, RPMh regulators, power-domains and clocks,
> > interconnects, some QUPs, UFS, remoteprocs, USB, watchdog, LLCC and
> > tsens.
> >
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
> >
> > arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 2145 ++++++++++++++++++++++++
> > 1 file changed, 2145 insertions(+)
> > create mode 100644 arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> > new file mode 100644
> > index 000000000000..ac13965a181e
> > --- /dev/null
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> [...]
> > + reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + memory@80000000 {
> > + reg = <0 0x80000000 0 0x860000>;
> > + no-map;
> > + };
> > +
> > + cmd_db: memory@80860000 {
> > + compatible = "qcom,cmd-db";
> > + reg = <0 0x80860000 0 0x20000>;
> > + no-map;
> > + };
> > +
> > + memory@80880000 {
> > + reg = <0 0x80880000 0 0x80000>;
> > + no-map;
> > + };
> > +
> > + smem_mem: smem@80900000 {
> > + compatible = "qcom,smem";
> > + reg = <0 0x80900000 0 0x200000>;
> > + no-map;
> > + hwlocks = <&tcsr_mutex 3>;
> > + };
> > +
> > + memory@80b00000 {
> > + reg = <0 0x80b00000 0 0x100000>;
> > + no-map;
> > + };
> > +
> > + memory@83b00000 {
> > + reg = <0 0x83b00000 0 0x1700000>;
> > + no-map;
> > + };
> > +
> > + memory@85b00000 {
> > + reg = <0 0x85b00000 0 0xc00000>;
> > + no-map;
> > + };
> > +
> > + pil_adsp_mem: memory@86c00000 {
> > + reg = <0 0x86c00000 0 0x2000000>;
> > + no-map;
> > + };
> > +
> > + pil_nsp0_mem: memory@8a100000 {
> > + reg = <0 0x8a100000 0 0x1e00000>;
> > + no-map;
> > + };
> > +
> > + pil_nsp1_mem: memory@8c600000 {
> > + reg = <0 0x8c600000 0 0x1e00000>;
> > + no-map;
> > + };
> > +
> > + memory@aeb00000 {
> > + reg = <0 0xaeb00000 0 0x16600000>;
> > + no-map;
> > + };
>
> Doesn't memory@ still cause the dtbs_check warnings? Similar to
>
> > soc/qcom/qcom,smem.example.dt.yaml: memory@fa00000: 'device_type' is a required property
> > From schema: dtschema/schemas/memory.yaml
>
> as in [1]. If I understood it correctly there Rob said that memory@
> shouldn't be used for reserved-memory. Perhaps even reserved-memory@
> might be better then.
>
> The device tree specification on the other hand suggests using the
> purpose of the reserved memory, like we did on older SoCs:
>
> > 3.5.2 /reserved-memory/ child nodes
> > Following the generic-names recommended practice, node names should
> > reflect the purpose of the node (ie. “framebuffer” or “dma-pool”).
>
> [1]: https://lore.kernel.org/linux-arm-msm/CAL_Jsq+66j8Y5y+PQ+mezkaxN1pfHFKz524YUF4Lz_OU5E-mZQ@xxxxxxxxxxxxxx/
>

Thanks for the pointer. I stared at these for a while given that we
"shouldn't use memory@", but like with the other platforms I figured we
could fix it later...

I'll update these accordingly.

> > + timer@17c20000 {
> > + compatible = "arm,armv7-timer-mem";
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > + reg = <0x0 0x17c20000 0x0 0x1000>;
> > + clock-frequency = <19200000>;
> [...]
> > + };
> > + timer {
> > + compatible = "arm,armv8-timer";
> > + interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > + <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > + <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>,
> > + <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(8) | IRQ_TYPE_LEVEL_LOW)>;
> > + clock-frequency = <19200000>;
> > + };
>
> Is the "clock-frequency" really needed for these two?
>
> The binding docs are pretty clear that this should be configured by the
> firmware instead:
>
> > Should be present only where necessary to work around broken firmware
> > which does not configure CNTFRQ on all CPUs to a uniform correct
> > value. Use of this property is strongly discouraged; fix your firmware
> > unless absolutely impossible.
>
> I hope Qualcomm's firmware is actually improving on newer platforms
> and not making big steps backwards. :-)
>

I believe I inherited this from somewhere, will check if it's actually
needed.

Thanks,
Bjorn