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: <100ef4a2-5050-9ed1-f50b-54b14153e959@quicinc.com>
Date:   Wed, 24 May 2023 15:26:35 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH v3] wifi: ath11k: Add rx histogram stats
Content-Language: en-US
To:     Kalle Valo <kvalo@kernel.org>
CC:     Tyler Stachecki <stachecki.tyler@gmail.com>,
        "open list:QUALCOMM ATHEROS ATH11K WIRELESS DRIVER" 
        <ath11k@lists.infradead.org>,
        "open list:NETWORKING DRIVERS (WIRELESS)" 
        <linux-wireless@vger.kernel.org>, <quic_akalaise@quicinc.com>,
        <quic_mpubbise@quicinc.com>
References: <CAC6wqPVvke=Gy59yvYcaps2dwVzO=wfwdSTGNxFzAOWc5OpoDw@mail.gmail.com>
 <d6f794e3-7463-da7d-721e-00869faf4c17@quicinc.com>
 <87h6s2kr5b.fsf@kernel.org>
From:   Maharaja Kennadyrajan <quic_mkenna@quicinc.com>
In-Reply-To: <87h6s2kr5b.fsf@kernel.org>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


On 5/24/2023 3:06 PM, Kalle Valo wrote:
> Maharaja Kennadyrajan <quic_mkenna@quicinc.com> writes:
>
>> On 5/21/2023 11:21 PM, Tyler Stachecki wrote:
>>
>>> Was this really tested on QCN9074 as the commit text suggests...?
>>>
>>>> const struct ath11k_hw_ops ipq6018_ops = {
>>>> @@ -1132,6 +1147,7 @@ const struct ath11k_hw_ops wcn6750_ops = {
>>>>    .rx_desc_get_msdu_payload = ath11k_hw_qcn9074_rx_desc_get_msdu_payload,
>>>>    .reo_setup = ath11k_hw_wcn6855_reo_setup,
>>>>    .mpdu_info_get_peerid = ath11k_hw_ipq8074_mpdu_info_get_peerid,
>>>> + .mpdu_info_get_mpdu_len = ath11k_hw_qcn9074_mpdu_info_get_mpdu_len,
>>> ...
>>>
>>>> +static u32 ath11k_hal_rx_mpduinfo_get_mpdu_len(struct ath11k_base *ab,
>>>> +       struct hal_rx_mpdu_info *mpdu_info)
>>>> +{
>>>> + return ab->hw_params.hw_ops->mpdu_info_get_mpdu_len(mpdu_info);
>>>> +}
>>> I think you want to put this under qcn9074_ops. As of now, when
>>> QCN9074 is present, it attempts to jump to a NULL pointer as
>>> mpdu_info_get_mpdu_len remains uninitialized for qcn9074_ops.
>>>
>>> And, do you not need to define mpdu_info_get_mpdu_len for all the
>>> other hw_ops? If so, please be careful about defining it for
>>> WCN6855/WCN6750 as there was a recent regression due to how the RX
>>> MPDU info is provided by those firmwares as it differed from
>>> IPQ8074/QCN9074. I personally do not have the appropriate literature
>>> to determine whether or not this is consequential or not here as well,
>>> though it seems like it would be:
>>> https://lore.kernel.org/linux-wireless/20230404072234.18503-3-quic_youghand@quicinc.com/
>>>
>>> Tyler
>> Thanks for your comments. Will fix this in the upcoming patchset.
> I have some comments as well but I haven't been able to send them yet. I
> recommend waiting for them before sending the next version.
Sure, Kalle.