Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:5 as permitted sender) client-ip=2620:137:e000::3:5;
Message-ID: <01c7a346-1e8b-1767-7594-c8adcd4823c3@quicinc.com>
Date:   Wed, 11 Oct 2023 10:35:11 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v3 2/2] arm64: dts: qcom: qcm6490: Add qcm6490 dts file
Content-Language: en-US
To:     Konrad Dybcio <konrad.dybcio@linaro.org>,
        Mukesh Ojha <quic_mojha@quicinc.com>,
        Komal Bajaj <quic_kbajaj@quicinc.com>, <agross@kernel.org>,
        <andersson@kernel.org>, <robh+dt@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <conor+dt@kernel.org>
CC:     <linux-arm-msm@vger.kernel.org>, <devicetree@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <luca.weiss@fairphone.com>
References: <20231003175456.14774-1-quic_kbajaj@quicinc.com>
 <20231003175456.14774-3-quic_kbajaj@quicinc.com>
 <5da2ba4f-5bf7-46ff-8204-0c169042dbfa@linaro.org>
 <3fd31aaa-f6bf-8440-6b08-fca2803171d9@quicinc.com>
 <dba83334-3971-46e9-9342-1344c5858be8@linaro.org>
From:   Trilok Soni <quic_tsoni@quicinc.com>
In-Reply-To: <dba83334-3971-46e9-9342-1344c5858be8@linaro.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 10/11/2023 2:47 AM, 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..
> 
> The firmware should either be fully relocatable (so that dynamic memory reservation can be used), unified so that there's no changes or better yet stored in separate memory so that q6 cores don't steal the RAM that the user paid for and you can do whatever ugly magic you please in there.
> 
> This arbitrary segmentation makes it impossible to have a common base, or to explain what device should go where to a newcomer.

Konrad it is possible to use the same SOC with the multiple segments w/ the different memory maps. 

Memory map here is how you organize the DDR and give it to various S/W and DSP regions etc; 

Also these SOCs are around for sometime and it is possible that new segments may use it. We can't solve
or know all the new segments need when the SOCs come out. Memory maps does provide that flexibility
and they don't change often. OEMs has also some flexibility to change the memory map if needed to optimize. 

This SOC is around for quite sometime new usecases are expected to emerge. I don't see it as
way to stop us from taking these contributions into the linux-arm-msm. 

---Trilok Soni


-- 
---Trilok Soni