Re: [RFC PATCH 2/2] arm64: dts: qcom: qcm6490: Add qcm6490 idp and rb3 board

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

 





On 11/7/2023 4:02 AM, Dmitry Baryshkov wrote:
On Mon, 6 Nov 2023 at 16:46, Mukesh Ojha <quic_mojha@xxxxxxxxxxx> wrote:



On 11/6/2023 5:24 PM, Dmitry Baryshkov wrote:
On Mon, 6 Nov 2023 at 13:41, Mukesh Ojha <quic_mojha@xxxxxxxxxxx> wrote:


On 11/5/2023 6:38 PM, Krzysztof Kozlowski wrote:
On 03/11/2023 23:22, Dmitry Baryshkov wrote:
On Fri, 3 Nov 2023 at 20:49, Komal Bajaj <quic_kbajaj@xxxxxxxxxxx> wrote:

Add qcm6490 devicetree file for QCM6490 IDP and QCM6490 RB3
platform. QCM6490 is derived from SC7280 meant for various
form factor including IoT.

Supported features are, as of now:
* Debug UART
* eMMC (only in IDP)
* USB


...

+
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-iot-common.dtsi b/arch/arm64/boot/dts/qcom/qcm6490-iot-common.dtsi
new file mode 100644
index 000000000000..01adc97789d0
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/qcm6490-iot-common.dtsi

I have mixed feelings towards this file. Usually we add such 'common'
files only for the phone platforms where most of the devices are
common.
Do you expect that IDP and RB3 will have a lot of common code other
than these regulator settings?

I agree here. What exactly is common in the real hardware between IDP
and RB3? Commit msg does not explain it, so I do not see enough
justification for common file. Just because some DTS looks similar for
different hardware does not mean you should creat common file.

@Dmitry/@Krzysztof,

Thank you for reviewing the RFC, we wanted to continue the
suggestion/discussion given on [1] , where we discussed that this
qcm6490 is going to be targeted for IOT segment and will have different
memory map and it is going to use some of co-processors like adsp/cdsp
which chrome does not use.

So to your question what is common between RB3 and IDP, mostly they will
share common memory map(similar to [2]) and regulator settings and both
will use adsp/cdsp etc., we will be posting the memory map changes as
well in coming weeks once this RFC is acked.

Is the memory map going to be the same as the one used on Fairphone5?

No, Fairphone5 looks to be using chrome memory map and i suggested
here to move them into sc7280.dtsi

https://lore.kernel.org/lkml/d5d53346-ca3b-986a-e104-d87c37115b62@xxxxxxxxxxx/


Are ADSP and CDSP physically present on sc7280?

Yes, they are present but not used.

So ADSP and CDSP should go into sc7280.dtsi. They will anyway have
status = "disabled";



I think that your goal should be to:
- populate missing device in sc7280.dtsi
- maybe add qcm6490.dtsi which defines SoC-level common data (e.g. memory map)
- push the rest to board files.

Agree to all of the point.
We started with the same thought at[3] but it got lost in discussion
due to its differentiation with mobile counter part(fairphone) which
follow chrome memory map and hence we came up with qcm6490-iot-common.
Do you think, qcm6490-iot.dtsi should be good ?

No. DT describes hardware, and -iot is not a hardware abstraction / unification.
If you consider your memory map to be generic for the qcm6490 (and FP5
being the only exception), add it to the qcm6490.dtsi (and let FP5
override it, like some of the phones do). If it can not be considered
generic for the SoC, then you have no other choice than to replicate
it to all board files.

Thanks for the suggestion.
Let me add @Luca here for information, if he want to share
anything about qcm6490 fp5 memory map.

-Mukesh


[3]
https://lore.kernel.org/linux-arm-msm/20231003175456.14774-3-quic_kbajaj@xxxxxxxxxxx/

-Mukesh

I don't think that putting regulators to the common file is a good
idea. Platforms will further change and limit voltage limits and
modes, so they usually go to the board file.



Thanks,
Mukesh

[1]
https://lore.kernel.org/linux-arm-msm/d97ebf74-ad03-86d6-b826-b57be209b9e2@xxxxxxxxxxx/

[2]
commit 90c856602e0346ce9ff234062e86a198d71fa723
Author: Douglas Anderson <dianders@xxxxxxxxxxxx>
Date:   Tue Jan 25 14:44:20 2022 -0800

       arm64: dts: qcom: sc7280: Factor out Chrome common fragment

       This factors out a device tree fragment from some sc7280 device
       trees. It represents the device tree bits that should be included for
       "Chrome" based sc7280 boards. On these boards the bootloader (Coreboot
       + Depthcharge) configures things slightly different than the
       bootloader that Qualcomm provides. The modem firmware on these boards
       also works differently than on other Qulacomm products and thus the
       reserved memory map needs to be adjusted.

       NOTES:
       - This is _not_ quite a no-op change. The "herobrine" and "idp"
         fragments here were different and it looks like someone simply
         forgot to update the herobrine version. This updates a few numbers
         to match IDP. This will also cause the `pmk8350_pon` to be disabled
         on idp/crd, which I belive is a correct change.
       - At the moment this assumes LTE skus. Once it's clearer how WiFi SKUs
         will work (how much of the memory map they can reclaim) we may add
         an extra fragment that will rejigger one way or the other.

       Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
       Reviewed-by: Stephen Boyd <swboyd@xxxxxxxxxxxx>
       Reviewed-by: Matthias Kaehlcke <mka@xxxxxxxxxxxx>
       Signed-off-by: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx>
       Link:
https://lore.kernel.org/r/20220125144316.v2.3.Iac012fa8d727be46448d47027a1813ea716423ce@changeid



Best regards,
Krzysztof











[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