Hi Bjorn, > -----Original Message----- > From: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > Sent: Sunday, March 29, 2020 12:01 AM > To: pillair@xxxxxxxxxxxxxx > Cc: devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx > Subject: Re: [PATCH v7] arm64: dts: qcom: sc7180: Add WCN3990 WLAN > module device node > > On Sat 28 Mar 05:01 PDT 2020, pillair@xxxxxxxxxxxxxx wrote: > > > Hi Bjorn, > > Comments inline. > > > > > > > -----Original Message----- > > > From: Bjorn Andersson <bjorn.andersson@xxxxxxxxxx> > > > Sent: Saturday, March 28, 2020 4:30 AM > > > To: Rakesh Pillai <pillair@xxxxxxxxxxxxxx> > > > Cc: devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; > > linux- > > > kernel@xxxxxxxxxxxxxxx; linux-arm-msm@xxxxxxxxxxxxxxx > > > Subject: Re: [PATCH v7] arm64: dts: qcom: sc7180: Add WCN3990 WLAN > > > module device node > > > > > > On Thu 26 Mar 03:48 PDT 2020, Rakesh Pillai wrote: > > > > > > > Add device node for the ath10k SNOC platform driver probe > > > > and add resources required for WCN3990 on sc7180 soc. > > > > > > > > Signed-off-by: Rakesh Pillai <pillair@xxxxxxxxxxxxxx> > > > > --- > > > > > > > > Depends on https://patchwork.kernel.org/patch/11455345/ > > > > The above patch adds the dt-bindings for wifi-firmware > > > > subnode > > > > --- > > > > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 8 ++++++++ > > > > arch/arm64/boot/dts/qcom/sc7180.dtsi | 27 > > > +++++++++++++++++++++++++++ > > > > 2 files changed, 35 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > > > b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > > > > index 043c9b9..a6168a4 100644 > > > > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts > > > > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts > > > > @@ -327,6 +327,14 @@ > > > > }; > > > > }; > > > > > > > > +&wifi { > > > > + status = "okay"; > > > > + qcom,msa-fixed-perm; > > > > + wifi-firmware { > > > > + iommus = <&apps_smmu 0xc2 0x1>; > > > > > > How is sc7180 different from sdm845, where the iommus property goes > > > directly in the &wifi node? > > > > Sc7180 IDP is a target without TrustZone support and also with S2 IOMMU > > enabled. > > Since in Trustzone based targets, the iommu SID configuration was done by > > TZ, there was nothing required to be done by driver. > > But in non-TZ based targets, the IOMMU mappings need to be done by the > > driver. > > Since this is the mapping of the firmware memory and to keep it different > > from the driver memory access, a different device has been created for > > firmware and these SIDs are configured. > > > > I see, I missed the fact that 0xc0:1 is used in the &wifi node itself. > > So to confirm, we have streams 0xc0 and 0xc1 for data pipes and 0xc2 and > 0xc3 for some form of firmware access? And in the normal Qualcomm design > implementation the 0c2/0xc3 stream mapping is setup by TZ, and hidden > from Linux using the SMMU virtualisation? > > > Would have been nice to have some better mechanism for describing > multi-connected hardware block, than to sprinkle dummy nodes all over > the DT... Yes, this is the firmware memory. This method is followed in the venus video driver https://patchwork.kernel.org/patch/11315765/ Do you suggest following some other mechanism ? Thanks, Rakesh Pillai. > > Regards, > Bjorn > > > The below ath10k series brings-in this support. > > https://patchwork.kernel.org/project/linux- > wireless/list/?series=261367&stat > > e=* > > > > Thanks, > > Rakesh Pillai. > > > > > > > > Regards, > > > Bjorn > > > > > > > + }; > > > > +}; > > > > + > > > > /* PINCTRL - additions to nodes defined in sc7180.dtsi */ > > > > > > > > &qspi_clk { > > > > diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > index 998f101..2745128 100644 > > > > --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi > > > > @@ -83,6 +83,11 @@ > > > > reg = <0 0x8f600000 0 0x500000>; > > > > no-map; > > > > }; > > > > + > > > > + wlan_fw_mem: memory@94104000 { > > > > + reg = <0 0x94104000 0 0x200000>; > > > > + no-map; > > > > + }; > > > > }; > > > > > > > > cpus { > > > > @@ -835,6 +840,28 @@ > > > > }; > > > > }; > > > > > > > > + wifi: wifi@18800000 { > > > > + compatible = "qcom,wcn3990-wifi"; > > > > + reg = <0 0x18800000 0 0x800000>; > > > > + reg-names = "membase"; > > > > + iommus = <&apps_smmu 0xc0 0x1>; > > > > + interrupts = > > > > + <GIC_SPI 414 IRQ_TYPE_LEVEL_HIGH /* CE0 > > > */ >, > > > > + <GIC_SPI 415 IRQ_TYPE_LEVEL_HIGH /* CE1 > > > */ >, > > > > + <GIC_SPI 416 IRQ_TYPE_LEVEL_HIGH /* CE2 > > > */ >, > > > > + <GIC_SPI 417 IRQ_TYPE_LEVEL_HIGH /* CE3 > > > */ >, > > > > + <GIC_SPI 418 IRQ_TYPE_LEVEL_HIGH /* CE4 > > > */ >, > > > > + <GIC_SPI 419 IRQ_TYPE_LEVEL_HIGH /* CE5 > > > */ >, > > > > + <GIC_SPI 420 IRQ_TYPE_LEVEL_HIGH /* CE6 > > > */ >, > > > > + <GIC_SPI 421 IRQ_TYPE_LEVEL_HIGH /* CE7 > > > */ >, > > > > + <GIC_SPI 422 IRQ_TYPE_LEVEL_HIGH /* CE8 > > > */ >, > > > > + <GIC_SPI 423 IRQ_TYPE_LEVEL_HIGH /* CE9 > > > */ >, > > > > + <GIC_SPI 424 IRQ_TYPE_LEVEL_HIGH /* CE10 > > > */>, > > > > + <GIC_SPI 425 IRQ_TYPE_LEVEL_HIGH /* CE11 > > > */>; > > > > + memory-region = <&wlan_fw_mem>; > > > > + status = "disabled"; > > > > + }; > > > > + > > > > config_noc: interconnect@1500000 { > > > > compatible = "qcom,sc7180-config-noc"; > > > > reg = <0 0x01500000 0 0x28000>; > > > > -- > > > > 2.7.4