Re: [PATCH v1] arm64: dts: qcom: qcs615: Add QUPv3 configuration

[Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>]

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@quicinc.com>
> Signed-off-by: Mukesh Kumar Savaliya <quic_msavaliy@quicinc.com>
> Signed-off-by: Viken Dadhaniya <quic_vdadhani@quicinc.com>
> ---

[...]

> +	qup_opp_table: opp-table-qup {
> +		compatible = "operating-points-v2";

opp-shared;

> +
> +		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@800000  {
> +			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.

> +			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?

> +		};
> +
>  		qupv3_id_0: geniqup@8c0000 {
>  			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

>  			#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

Konrad