[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0a193a56-0197-486b-9d36-20aeb0d5860a@quicinc.com>
Date: Wed, 26 Feb 2025 23:51:22 +0530
From: Raj Kumar Bhagat <quic_rajkbhag@...cinc.com>
To: Bjorn Andersson <andersson@...nel.org>
CC: Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<ath12k@...ts.infradead.org>, <linux-wireless@...r.kernel.org>
Subject: Re: [PATCH v2] arm64: dts: qcom: add wifi node for IPQ5332 based
RDP441
On 2/26/2025 9:22 AM, Bjorn Andersson wrote:
> On Thu, Jan 30, 2025 at 10:29:00AM +0530, Raj Kumar Bhagat wrote:
>> RDP441 is based on IPQ5332. It has inbuilt AHB bus based IPQ5332 WiFi
>> device.
>>
>> Describe and add WiFi node for RDP441. Also, reserve the memory
>> required by IPQ5332 firmware.
>>
>> Depends-On: [PATCH V2 0/2] mailbox: tmel-qmp: Introduce QCOM TMEL QMP mailbox driver
>> Depends-On: [PATCH V3 0/8] Add new driver for WCSS secure PIL loading
>> Link: https://lore.kernel.org/lkml/20241231054900.2144961-1-quic_srichara@quicinc.com/
>> Link: https://lore.kernel.org/lkml/20250107101647.2087358-1-quic_gokulsri@quicinc.com/
>
> Please run "git log --grep 'Depends-on'" to see how this tag is expected
> to be used. As you can see, dependencies on other patches in flight does
> not go into the git history, and should as such be mentioned only below
> the --- line below.
>
Thanks will take care for next submission.
> Please wait for these dependencies to be accepted before resubmitting
> this Devicetree change.
>
Sure
>>
>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@...cinc.com>
>> ---
>> NOTE:
>> The DT binding changes for this patch has been posted along with the
>> driver ath12k AHB series.
>> Link: https://lore.kernel.org/all/20250130043508.1885026-1-quic_rajkbhag@quicinc.com/
>>
>> v2:
>> - Dropped 'dt-bindings: net: wireless: describe the ath12k AHB module'.
>> This DT binding is posted along with the ath12k AHB driver changes.
>> - Reserved four memory regions required by IPQ5332 ath12k firmware in DTS.
>> These memory regions are also referenced in the WiFi node.
>> - Moved 'qcom,smem-states' and 'qcom,smem-states' to the ipq5332.dtsi file.
>> - Used lowercase hex values for the reg property.
>>
>> v1: https://lore.kernel.org/all/20250128091012.2574478-1-quic_rajkbhag@quicinc.com/
>> ---
>> arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts | 64 ++++++++++-
>> arch/arm64/boot/dts/qcom/ipq5332.dtsi | 113 ++++++++++++++++++++
>> 2 files changed, 176 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
>> index 846413817e9a..4b7a75e450ca 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
>> +++ b/arch/arm64/boot/dts/qcom/ipq5332-rdp441.dts
>> @@ -2,7 +2,7 @@
>> /*
>> * IPQ5332 AP-MI01.2 board device tree source
>> *
>> - * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
>> + * Copyright (c) 2022-2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> */
>>
>> /dts-v1/;
>> @@ -12,6 +12,61 @@
>> / {
>> model = "Qualcomm Technologies, Inc. IPQ5332 MI01.2";
>> compatible = "qcom,ipq5332-ap-mi01.2", "qcom,ipq5332";
>> +
>> + /* Default Profile
>
> Why is the title "Default Profile", is this expected to be changed by
> someone? At what point?
>
>> + * +============+==============+=====================+
>> + * | | | |
>> + * | Region | Start Offset | Size |
>
> "Start Offset" relative to address 0 is typically called "base address".
>
>> + * | | | |
>> + * +------------+--------------+---------------------+
>> + * | | | |
>> + * | | | |
>> + * | | | |
>> + * | WLAN Q6 | 0x4A900000 | 43MB |
>> + * | | | |
>> + * | | | |
>> + * +------------+--------------+---------------------+
>> + * | M3 Dump | 0x4D400000 | 1MB |
>> + * +------------+--------------+---------------------+
>> + * | Q6 caldb | 0x4D500000 | 5MB |
>> + * +------------+--------------+---------------------+
>> + * | MLO | 0x4DB00000 | 18MB |
>
> 18MB is the same size in your graph as the 1MB segment, but somehow the
> 43MB segment is 5 times the size of the 18MB segment. So the graph isn't
> to scale...
>
>> + * +============+==============+=====================+
>> + * | |
>> + * | |
>> + * | |
>> + * | Rest of memory for Linux |
>> + * | |
>> + * | |
>> + * | |
>> + * +=================================================+
>
> Doesn't DDR start on a more even address, such as 0x40000000? I presume
> the purpose of your picture was to give a good overview of the memory
> layout of this system, but the only thing it added to the table below is
> a curious question about what happened to the first 169MB of DDR.
>
Yes this graph provide the overview for memory related to wireless
components(rproc and ath12k firmware memory).
>
> Please omit the graph, it doesn't add value beyond what's provided by
> the below text form of the "same" data.
>
Sure, will remove in next version.
>> + */
>> +
>> + reserved-memory {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> + ranges;
>> +
>> + q6_region: wcss@...00000 {
>> + reg = <0x0 0x4a900000 0x0 0x02b00000>;
>> + no-map;
>> + };
>> +
>> + m3_dump: m3-dump@...00000 {
>> + reg = <0x0 0x4d400000 0x0 0x100000>;
>> + no-map;
>> + };
>> +
>> + q6_caldb: q6-caldb@...00000 {
>> + reg = <0x0 0x4d500000 0x0 0x500000>;
>> + no-map;
>> + };
>> +
>> + mlo_mem: mlo-global-mem@...00000 {
>> + reg = <0x0 0x4db00000 0x0 0x01200000>;
>> + no-map;
>> + };
>> + };
>> };
>>
>> &blsp1_i2c1 {
>> @@ -63,3 +118,10 @@ data-pins {
>> };
>> };
>> };
>> +
>> +&wifi0 {
>> + memory-region = <&q6_region>, <&m3_dump>, <&q6_caldb>, <&mlo_mem>;
>> + memory-region-names = "q6-region", "m3-dump", "q6-caldb", "mlo-global-mem";
>
> As you are resubmitting this, it would be nice to have these two
> properties wrapped (one entry per line), to increase readability...
>
will update in next version.
Powered by blists - more mailing lists