On Tue, 14 Nov 2023 at 14:49, Mukesh Ojha <quic_mojha@xxxxxxxxxxx> wrote: > > > > On 11/13/2023 9:21 PM, Luca Weiss wrote: > > On Tue Nov 7, 2023 at 9:10 AM CET, Mukesh Ojha wrote: > >> > >> > >> 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. > >> > > > > Hi Mukesh, > > > >> Thanks for the suggestion. > >> Let me add @Luca here for information, if he want to share > >> anything about qcm6490 fp5 memory map. > > > > Not sure I have much to share, just probably that on FP5 the memory > > setup and all the basics just come from a standard QCM6490.LA.3.0 > > release. > > I don't see any hint that our ODM changed something in the memory map > > for the device either. > > > > I'm also aware that other phones also use QCM6490 SoC, so I'm still > > wondering where the distinction between "FP5/ChromeOS memory map" vs > > this new QCM6490 memory map is. > > There's also e.g. this phone using QCM6490, I've not looked into this at > > all, but I'm guessing that phone uses the same memory map as FP5. > > https://www.crosscall.com/en_NL/core-z5-COZ5.MASTER.html > > Was looking for your view on the things about qcm6490.dtsi one common > dtsi file for all qcm6490.dtsi suggested in the mail, but looks like FP5 > is following the memory map based out of sc7280, in that case we have to > replicate the new memory map for all our IOT boards(idp/rb3) based on > this SoC. You can have IoT memory map in the qcm6490.dtsi and have the board-specific memory map in the qcm6490-fp5.dtsi, if that makes life easier. I think the phone DT already provides the memory map, so you just have to add statements to remove conflicting data entries. > > -Mukesh > > > > Regards > > Luca > > > >> > >> -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 > >>>>>>> > >>>>> > >>>>> > >>>>> > >>> > >>> > >>> > > -- With best wishes Dmitry
