lkml.org 
[lkml]   [2023]   [Oct]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/2] arm64: dts: qcom: qcm6490: Add qcm6490 dts file
From


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

\
 
 \ /
  Last update: 2023-10-13 16:15    [W:0.441 / U:0.040 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site