Re: [PATCH v2] ath10k: Set DMA address mask to 35 bit for WCN3990

From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: Rakesh Pillai <pillair@codeaurora.org>,
	ath10k@lists.infradead.org, linux-wireless@vger.kernel.org
Subject: Re: [PATCH v2] ath10k: Set DMA address mask to 35 bit for WCN3990
Date: Thu, 31 Jan 2019 07:35:37 -0800	[thread overview]
Message-ID: <20190131153537.GC2387@tuxbook-pro> (raw)
In-Reply-To: <871s4tj5p9.fsf@kamboji.qca.qualcomm.com>

On Thu 31 Jan 05:51 PST 2019, Kalle Valo wrote:

> Bjorn Andersson <bjorn.andersson@linaro.org> writes:
> 
> > On Mon 03 Sep 09:37 PDT 2018, Rakesh Pillai wrote:
> >
> >> WCN3990 is a 37-bit target but can address memory range
> >> only upto 35 bits. The 36th bit is used to control the
> >> smmu/iommu translation and the 37th bit is used by the
> >> internal bus masters to access the wifi subsystem internal
> >> SRAM. With the DMA mask set to 37i-bit, the host driver
> >> can get 37-bit dma address, which leads to incorrect
> >> address access in the target.
> >> 
> >> Hence the host driver can used addresses upto 35-bit
> >> for WCN3990. Fix the dma mask for wcn3990 to 35-bit,
> >> instead of 37-bit.
> >> 
> >> Tested HW: WCN3990
> >> Tested FW: WLAN.HL.2.0-01188-QCAHLSWMTPLZ-1
> >> 
> >> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> >
> > This solves the problem I'm seeing on my SDM845, where I see a
> > translation fault on a 32-bit address from the IOMMU, which we
> > previously mapped the 36 bit version of (my dma-ranges is set to 36
> > bits).
> >
> > So:
> >
> > Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Thanks for the reminder. This got piled up in my deferred queue in
> patchwork and I hadn't looked at it yet. I'll queue this for -next.
> 

Thanks

> > However, some of the changes in this patch and the fact that I get a
> > translation error on the lower 32 bits of the mapped iova, makes me
> > suspect that while the hardware is capable of 37 bits, the driver only
> > dealt with the lower 32.  And if that's the case I would like to see
> > that mentioned in the commit message.
> 
> Rakesh mentioned that it's actually 35 bits. Should something changed in
> the commit log still? I can do that if needed.
> 

Right, so the patch and commit message matches. What I'm asking about is
that in my testing I saw, from the IOMMU translation errors, that the
addresses accessed by the hardware wasn't 35 bit, they where only 32
bits.

Given that the patch also makes changes to how it writes addresses to
the ring I was wondering if the patch actually takes the limit from 32
to 35, not 37 to 35.

Regardless, I'm fine with the end result, you have my T-b and I would be
happy to see this in v5.1.

Regards,
Bjorn