[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <332f7a0a-d9df-49bd-81d5-cc04c50183b9@quicinc.com>
Date: Mon, 28 Oct 2024 16:49:43 +0530
From: Viken Dadhaniya <quic_vdadhani@...cinc.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>, <andersson@...nel.org>,
<konradybcio@...nel.org>, <robh@...nel.org>, <krzk+dt@...nel.org>,
<conor+dt@...nel.org>, <linux-arm-msm@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: <quic_msavaliy@...cinc.com>, <quic_anupkulk@...cinc.com>
Subject: Re: [PATCH v1] arm64: dts: qcom: qcs615: Add QUPv3 configuration
On 10/26/2024 1:10 AM, Konrad Dybcio wrote:
> On 11.10.2024 12:33 PM, Viken Dadhaniya wrote:
>> Add DT support for QUPv3 Serial Engines.
>>
>> Co-developed-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
>> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@...cinc.com>
>> Signed-off-by: Viken Dadhaniya <quic_vdadhani@...cinc.com>
>> ---
>
> [...]
>
>> + qup_opp_table: opp-table-qup {
>> + compatible = "operating-points-v2";
>
> opp-shared;
Added in v2.
>
>> +
>> + opp-75000000 {
>> + opp-hz = /bits/ 64 <75000000>;
>> + required-opps = <&rpmhpd_opp_low_svs>;
>> + };
>> +
>> + opp-100000000 {
>> + opp-hz = /bits/ 64 <100000000>;
>> + required-opps = <&rpmhpd_opp_svs>;
>> + };
>> +
>> + opp-128000000 {
>> + opp-hz = /bits/ 64 <128000000>;
>> + required-opps = <&rpmhpd_opp_nom>;
>> + };
>> + };
>> +
>> psci {
>> compatible = "arm,psci-1.0";
>> method = "smc";
>> @@ -392,6 +427,24 @@
>> #size-cells = <1>;
>> };
>>
>> + gpi_dma0: qcom,gpi-dma@...000 {
>> + compatible = "qcom,sdm845-gpi-dma";
>
> You must define a new compatible for qcs615, sdm845 is used as a fallback
> (so that we don't have to add new driver entries). You will however need
> to submit a separate dt-bindings change.
We have added sdm845 in compatible due to below comment in driver file
File:
https://github.com/torvalds/linux/blob/81983758430957d9a5cb3333fe324fd70cf63e7e/drivers/dma/qcom/gpi.c#L2284
/*
* Do not grow the list for compatible devices. Instead use
* qcom,sdm845-gpi-dma (for ee_offset = 0x0) or qcom,sm6350-gpi-dma
* (for ee_offset = 0x10000).
*/
Do we still require new compatible for qcs615 ?
>
>> + reg = <0x0 0x800000 0x0 0x60000>;
>> + #dma-cells = <3>;
>> + interrupts = <GIC_SPI 244 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 245 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 246 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 249 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 250 IRQ_TYPE_LEVEL_HIGH>,
>> + <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>;
>> + dma-channels = <8>;
>> + dma-channel-mask = <0xf>;
>> + iommus = <&apps_smmu 0xd6 0x0>;
>> + status = "disabled";
>
> Any reason?
By default, we are disabling all nodes. In the target-specific file, we
will update them based on the required use case.
>
>> + };
>> +
>> qupv3_id_0: geniqup@...000 {
>> compatible = "qcom,geni-se-qup";
>> reg = <0x0 0x8c0000 0x0 0x6000>;
>> @@ -400,6 +453,7 @@
>> <&gcc GCC_QUPV3_WRAP_0_S_AHB_CLK>;
>> clock-names = "m-ahb",
>> "s-ahb";
>> + iommus = <&apps_smmu 0xc3 0x0>;
>
> This looks like a separate fix
One QUP and UART (console) related change has been added as part of the
base DTSI. Now, we are adding all remaining QUP and I2C/SPI/UART
configurations with IOMMUs and other properties.
Is there any specific reason to make this a separate fix?
>
>> #address-cells = <2>;
>> #size-cells = <2>;
>> status = "disabled";
>> @@ -412,13 +466,377 @@
>> pinctrl-0 = <&qup_uart0_tx>, <&qup_uart0_rx>;
>> pinctrl-names = "default";
>> interrupts = <GIC_SPI 601 IRQ_TYPE_LEVEL_HIGH>;
>> - interconnects = <&aggre1_noc MASTER_QUP_0 0
>> - &mc_virt SLAVE_EBI1 0>,
>> - <&gem_noc MASTER_APPSS_PROC 0
>> - &config_noc SLAVE_QUP_0 0>;
>> + interconnects = <&aggre1_noc MASTER_QUP_0 0 &mc_virt SLAVE_EBI1 0>,
>> + <&gem_noc MASTER_APPSS_PROC 0 &config_noc SLAVE_QUP_0 0>;
>
> Why?
>
> Also, please use QCOM_ICC_TAG_ALWAYS instead of zeroes
Updated in v2.
>
> Konrad
Powered by blists - more mailing lists