[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <62650f39-9703-fdc5-c72a-801b8e9f6470@quicinc.com>
Date: Tue, 7 Nov 2023 13:40:46 +0530
From: Mukesh Ojha <quic_mojha@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Luca Weiss <luca.weiss@...rphone.com>
CC: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>,
Komal Bajaj <quic_kbajaj@...cinc.com>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <quic_nainmeht@...cinc.com>
Subject: Re: [RFC PATCH 2/2] arm64: dts: qcom: qcm6490: Add qcm6490 idp and
rb3 board
On 11/7/2023 4:02 AM, Dmitry Baryshkov wrote:
> On Mon, 6 Nov 2023 at 16:46, Mukesh Ojha <quic_mojha@...cinc.com> wrote:
>>
>>
>>
>> On 11/6/2023 5:24 PM, Dmitry Baryshkov wrote:
>>> On Mon, 6 Nov 2023 at 13:41, Mukesh Ojha <quic_mojha@...cinc.com> 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@...cinc.com> 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/@...ysztof,
>>>>
>>>> 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@quicinc.com/
>>
>>>
>>> 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@quicinc.com/
>>
>> -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@quicinc.com/
>>>>
>>>> [2]
>>>> commit 90c856602e0346ce9ff234062e86a198d71fa723
>>>> Author: Douglas Anderson <dianders@...omium.org>
>>>> 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@...omium.org>
>>>> Reviewed-by: Stephen Boyd <swboyd@...omium.org>
>>>> Reviewed-by: Matthias Kaehlcke <mka@...omium.org>
>>>> Signed-off-by: Bjorn Andersson <bjorn.andersson@...aro.org>
>>>> Link:
>>>> https://lore.kernel.org/r/20220125144316.v2.3.Iac012fa8d727be46448d47027a1813ea716423ce@changeid
>>>>
>>>>
>>>>>
>>>>> Best regards,
>>>>> Krzysztof
>>>>>
>>>
>>>
>>>
>
>
>
Powered by blists - more mailing lists