Messages in this thread | | | Date | Fri, 13 Oct 2023 19:43:49 +0530 | Subject | Re: [PATCH v3 2/2] arm64: dts: qcom: qcm6490: Add qcm6490 dts file | From | Mukesh Ojha <> |
| |
On 10/12/2023 4:25 AM, Konrad Dybcio wrote: > > > On 10/11/23 15:40, Mukesh Ojha wrote: >> >> >> On 10/11/2023 3:17 PM, Konrad Dybcio wrote: >>> >>> >>> On 10/11/23 07:40, Mukesh Ojha wrote: >>>> >>>> >>>> On 10/7/2023 5:02 AM, Konrad Dybcio wrote: >>>>> On 3.10.2023 19:54, Komal Bajaj wrote: >>>>>> Add qcm6490 devicetree file for QCM6490 SoC and QCM6490 IDP >>>>>> platform. QCM6490 is derived from SC7280 meant for various >>>>>> form factor including IoT. >>>>>> >>>>>> Supported features are, as of now: >>>>>> * Debug UART >>>>>> * eMMC >>>>>> * USB >>>>>> >>>>>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com> >>>>>> --- >>>>> [...] >>>>> >>>>>> diff --git a/arch/arm64/boot/dts/qcom/qcm6490.dtsi >>>>>> b/arch/arm64/boot/dts/qcom/qcm6490.dtsi >>>>>> new file mode 100644 >>>>>> index 000000000000..b93270cae9ae >>>>>> --- /dev/null >>>>>> +++ b/arch/arm64/boot/dts/qcom/qcm6490.dtsi >>>>>> @@ -0,0 +1,94 @@ >>>>>> +// SPDX-License-Identifier: BSD-3-Clause >>>>>> +/* >>>>>> + * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights >>>>>> reserved. >>>>>> + */ >>>>>> + >>>>>> +#include "sc7280.dtsi" >>>>>> + >>>>>> +/* >>>>>> + * Delete unused sc7280 memory nodes and define the memory regions >>>>>> + * required by qcm6490 >>>>>> + */ >>>>>> +/delete-node/ &rmtfs_mem; >>>>>> +/delete-node/ &wlan_ce_mem; >>>>>> + >>>>>> +/{ >>>>>> + reserved-memory { >>>>>> + cdsp_secure_heap_mem: cdsp-secure-heap@81800000 { >>>>>> + reg = <0x0 0x81800000 0x0 0x1e00000>; >>>>>> + no-map; >>>>>> + }; >>>>>> + >>>>>> + camera_mem: camera@84300000 { >>>>> Uhh.. this is totally not the same memory map that I have on a >>>>> random msm-5.4 source+devicetree drop (which does in turn align >>>>> with the one on QCM6490 Fairphone 5, as it should because it's >>>>> a rebadged reference device for the most part).. >>>>> >>>>> Did you guys *really* redo it between software releases? >>>> >>>> QCM6490 fairphone is special case where same SOC is used for mobile >>>> product and it uses sc7280 memory map. >>>> >>>> Current patch adds support for the same SOC marketed for IOT segment >>>> [1] and very active in the development and soon going to freeze its >>>> memory map, so we are deriving memory map from sc7280 and creating >>>> a new memory map for all IOT product with qcm6490.dtsi . >>> Stop reinventing the wheel. I'm not going to accept patches that are >>> supposed to define ABI for products that are still in development. >>> Not unless Qualcomm changes their attitude towards unilaterally >>> breaking things for no good reason. >>> >>>> >>>> [1] >>>> https://www.qualcomm.com/products/internet-of-things/industrial/building-enterprise/qcm6490 >>>> >>>>> >>>>> This SoC family has been on the market for quite some time, >>>>> breaking software expectations like that is not cool, especially >>>>> on a product with a promised lifespan of 10 years or whatever! >>>> >>>> I agree, but we are not changing anything for product which are there >>>> in the market instead defining a new memory map what is going to come >>>> with qcm6490.dtsi for IOT. >>> Why would the OS care about the market segment you're targeting? >>> Why would the firmware you're building care about the market segment >>> you're targeting? The LE vs LA vs LU vs WP vs whatever split is so >>> unnecessary and arbitrary on the firmware/kernel side.. > First of all, I vented off on you very heavily in response to seeing > something I don't like, even though you didn't have anything to do with > it. Please accept my apology.
That's fine, Np..
> > There are some difficulties with integrating certain things upstream to > work out on a broader scale, but me screaming at engineers in public > won't help much with that. > >> Forgive me, if i ask some very basic question, just trying to put my >> thought, >> >> I agree, OS should not worry about the market segment, but through the >> DT firmware, we can better optimize memory to either give more memory to >> user or give more memory to certain DSP's to enable certain feature >> through the firmware like some logging infra etc., and due to which >> certain gaps can get created where certain memory region need to be >> move up or down due to increase in the carve-out. > This is totally fine from a generic standpoint, however Qualcomm has a > history (and you can see that in most SoC DTSIs) of having a common (or > almost common) memory map on the vast majority of devices based on a > given family of SoCs. We've been steadily taking advantage of that for > quite some time. > > Here, we have an established compute SoC (7280-Chrome) with a memory > setup that roughly matches its mobile counterpart (6490-LA or 778G or > whatever different derivatives).
I understand..
> > IIUC you're tweaking the software for the "new IoT BSP" and resizing > some regions resulted in many differences (as PIL regions tend to be > contiguous one-to-another).
This is correct. There are some other differences like cdsp/adsp support that we shall be pushing soon which sc7280 doesn't use.
> The real issue here is that if we express > this changed memory map in qcm6490.dtsi, all devices that have already > shipped with the older-than-"new IoT BSP" software will differ rather > significantly.
Yes, I see your point. Subjective to this product segment there may be other BSP related additions.
> You mentioned that there are going to be multiple users of *this new* > configuration, perhaps qcm6490-iot-common.dtsi (similar to > sc7280-chrome-common.dtsi) could facilitate the new bsp changes instead, > making it less ambiguous.
Yeah, so IIUC to avoid any ambiguity (like mentioned in your previous comment) this might be a better option.
>> >> Let's say X Soc released with some memory map, any derivative SoC Y >> should follow X's memory map if it is including X dtsi ? and the >> reason why Y want to include X is solely the work done for X and most >> of peripheral memory addresses is matching. >> >> But 'Y' could be different product, right? and it could have different >> firmware and it is not like 'X' firmware will run on 'Y' ? > Right, historically that hasn't happened very often but it could be like > that.
This is what we are looking for..
> >> Now a days, most of our firmware are relocatable. > And we should totally take advantage of that. Stephan Gerhold has > submitted some improvements that made it possible to dynamically > allocate memory regions on 8916, this should probably be reused and > expanded for other SoCs. Would it be possible for you to try out > dynamic PIL region allocation on this board? See [1] for example.
You mean adapting this qcm6490.dtsi change to dynamic region? Can we do that without touching sc7280.dtsi memory map itself.
> And the last thing is, I would like for you to give us some sort of a > stability promise for this. You mentioned this SoC spin is "very active > in the development", which makes me worried for DT compatibility with > future METAs. We have unfortunately historically had to deal with > different firmware packages behaving in divergent ways, and not always > consistently between devices (but the last point may be just vendor > modifications).
We are checking and will come back on this. Outside these BSP dependent things, don't see a challenge in maintaining SoC support compatibility.
> We are supposed to be able to boot any future version of Linux with this > initial devicetree, unless there's some fatal flaw that needs > retroactive fixing (like when we tried to express LLCC as a contiguous > region instead of a set of slices up until 8550 release or so). Please > have that in mind, we've tried so hard to keep this ABI-like.
Yes, the plan is to maintain this SoC on moving latest kernel tips.
> And the last-last (I promise..) question, is this the final SoC silicon > revision? And is it any different from the QCM6490 that has landed in > some Android devices physically? Or does it simply ship with a different > sw stack?
I am not aware of this. In case the SoC is revised then the support need to be extended for new revision as well maintaining compatibility for older one.
-Mukesh
> > Konrad > > [1] > https://lore.kernel.org/linux-arm-msm/20230911-msm8916-rmem-v1-4-b7089ec3e3a1@gerhold.net/#t
| |