Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b126ba5d-035c-3218-2847-5ac86063ebd6@quicinc.com>
Date:   Wed, 4 May 2022 12:12:30 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.1
Subject: Re: [PATCH v3] ath11k: Fix RX de-fragmentation issue on WCN6750
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
CC:     <ath11k@lists.infradead.org>, <linux-wireless@vger.kernel.org>
References: <20220328061947.28813-1-quic_mpubbise@quicinc.com>
 <87bkwgau67.fsf@kernel.org>
From:   Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
In-Reply-To: <87bkwgau67.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/2/2022 7:55 PM, Kalle Valo wrote:
> Manikanta Pubbisetty <quic_mpubbise@quicinc.com> writes:
> 
>> The offset of REO register where the RX fragment destination ring
>> is configured is different in WCN6750 as compared to WCN6855.
>> Due to this differnce in offsets, on WCN6750, fragment destination
>> ring will be configured incorrectly, leading to RX fragments not
>> getting delivered to the driver. Fix this by defining HW specific
>> offset for the REO MISC CTL register.
>>
>> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00573-QCAMSLSWPLZ-1
>> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
>> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
>> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00192-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Manikanta Pubbisetty <quic_mpubbise@quicinc.com>
> 
> [...]
> 
>> @@ -2229,6 +2229,9 @@ const struct ath11k_hw_regs wcn6855_regs = {
>>   
>>   	/* Shadow register area */
>>   	.hal_shadow_base_addr = 0x000008fc,
>> +
>> +	/* REO MISC CTRL */
>> +	.hal_reo1_misc_ctl = 0x00000630,
>>   };
> 
> I don't really see the benefit from the comment, it doesn't bring any
> extra information.
> 

You are right, let me check if I can add any info or I'll remove the 
comment.

>>   const struct ath11k_hw_regs wcn6750_regs = {
>> @@ -2311,6 +2314,9 @@ const struct ath11k_hw_regs wcn6750_regs = {
>>   
>>   	/* Shadow register area */
>>   	.hal_shadow_base_addr = 0x00000504,
>> +
>> +	/* REO MISC CTRL */
>> +	.hal_reo1_misc_ctl = 0x000005d8,
> 
> Ditto.
> 
> What about other ath11k_hw_regs tables:
> 
> <global>[1906]                 const struct ath11k_hw_regs ipq8074_regs = {
> <global>[1988]                 const struct ath11k_hw_regs qca6390_regs = {
> <global>[2070]                 const struct ath11k_hw_regs qcn9074_regs = {
> 
> You don't mention anything about those in the commit log. If they don't
> use this register (didn't check) we should at least initialise the
> fields to zero, deadbeef or something to make it clear they are unused.
> 

Let me make the changes to initialize the register to 0(zero) for other 
unsupported hardware.

Thanks,
Manikanta