RE: [PATCH v7] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
From: pillair
Date: Sun Mar 29 2020 - 07:07:37 EST
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