Message-ID: <e040a41c-8004-a5c0-eb3d-e1b2b06eeb1a@quicinc.com>
Date:   Mon, 30 Jan 2023 13:51:52 -0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [RFC PATCH v2 00/22] Introduce QC USB SND audio offloading
 support
Content-Language: en-US
To:     Mathias Nyman <mathias.nyman@linux.intel.com>,
        <srinivas.kandagatla@linaro.org>, <mathias.nyman@intel.com>,
        <perex@perex.cz>, <lgirdwood@gmail.com>, <andersson@kernel.org>,
        <krzysztof.kozlowski+dt@linaro.org>, <gregkh@linuxfoundation.org>,
        <Thinh.Nguyen@synopsys.com>, <broonie@kernel.org>,
        <bgoswami@quicinc.com>, <tiwai@suse.com>, <robh+dt@kernel.org>,
        <agross@kernel.org>
CC:     <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>,
        <alsa-devel@alsa-project.org>, <devicetree@vger.kernel.org>,
        <linux-usb@vger.kernel.org>, <quic_jackp@quicinc.com>,
        <quic_plai@quicinc.com>
References: <20230126031424.14582-1-quic_wcheng@quicinc.com>
 <1e889754-6492-4c53-27f0-796002d5680c@linux.intel.com>
From:   Wesley Cheng <quic_wcheng@quicinc.com>
In-Reply-To: <1e889754-6492-4c53-27f0-796002d5680c@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi Mathias,

On 1/26/2023 1:23 AM, Mathias Nyman wrote:
> On 26.1.2023 5.14, Wesley Cheng wrote:
>> Changes in v2:
>>
>> XHCI:
>> - Replaced XHCI and HCD changes with Mathias' XHCI interrupter changes
>> in his tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters 
>>
>>
> 
> I'll submit the first three patches from that branch myself to usb-next, 
> might modify
> them slightly.
> Just need to make sure they don't cause regression. Those are changes I 
> want done anyway.
> 

Sounds good! Thanks!

>> Adjustments made to Mathias' changes:
>>    - Created xhci-intr.h to export/expose interrupter APIs versus 
>> exposing xhci.h.

Do you think using the xhci-intr.h is a viable solution for class 
drivers to request for a secondary interrupter?

>>      Moved dependent structures to this file as well. (so clients can 
>> parse out
>>      information from "struct xhci_interrupter")
>>    - Added some basic locking when requesting interrupters.
>>    - Fixed up some sanity checks.
>>    - Removed clearing of the ERSTBA during freeing of the interrupter. 
>> (pending
>>      issue where SMMU fault occurs if DMA addr returned is 64b - TODO)
> 
> Was this solvable by first clearing high 32 bits and then low 32 bits?
> 

During the freeing of the secondary interrupter, the SMMU fault wasn't 
resolvable with clearing the high bits first.  This does somewhat give 
me the notion that the xHC is attempting to access the event ring base 
address every time the ERSTBA is written.  I believe the hi-lo write 
didn't work, as this time we are zero'ing out the base address. (SMMU 
FAR=0x0)

As stated in Table 5-40 in the XHCI spec, when we write a 0 to the 
secondary interrupter ERSTSZ, it should disable that event ring.  In 
this case, do we really need to explicitly clear the base address 
register?  If I don't clear the ERSTBA (during free), then I don't see a 
SMMU fault even after the event ring has been freed. (ie event ring 
memory has been unmapped from the SMMU)  So this should mean the xHC 
hasn't attempted to access that unmapped region for the memory address 
stored in the ERSTBA.

Likewise, we'll write the ERSTBA again during the alloc phase to a valid 
and mapped address.

Thanks
Wesley Cheng