Re: [PATCH v2] ath10k: support for multicast rate control

[pradeepc@codeaurora.org]

On 2018-05-09 07:31, Ben Greear wrote:
> On 05/08/2018 11:57 PM, Sebastian Gottschall wrote:
>> 
>> 
>> Am 09.05.2018 um 08:15 schrieb Sven Eckelmann:
>>> On Montag, 7. Mai 2018 18:34:51 CEST Pradeep Kumar Chitrapu wrote:
>>>> Issues wmi command to firmwre when multicast rate change is received
>>>> with the new BSS_CHANGED_MCAST_RATE flag.
>>>> Also fixes the incorrect fixed_rate setting for CCK rates which got
>>>> introduced with addition of ath10k_rates_rev2 enum.
>>>> 
>>>> Tested on QCA9984 with firmware ver 10.4-3.6-00104
>>>> Signed-off-by: Pradeep Kumar Chitrapu <pradeepc@codeaurora.org>
>>>> ---
>>>> v2:
>>>>   - fixed the CCK rates setting for mcast_rate and fixed_rate paths.
>>>>   - set broadcast rate along with multicast rate setting.
>>> These things are only modified in ath10k_bss_info_changed and not 
>>> saved/
>>> restored. What happens when the HW needs to be reset (there are are 
>>> couple of
>>> firmware crashes which can be seen in the wild and are handled by 
>>> ath10k)? I
>>> would guess that not all of them automatically cause an 
>>> .bss_info_changed.
> 
> Yes, that sounds like a good idea to me.
> 
Hi Sven, Thanks for the review.
In case of HW reset, I see ieee80211_reconfig triggers bss_info changed 
and
BSS_CHANGED_MCAST_RATE is already being set in this function.
(https://patchwork.kernel.org/patch/10302175/). isn't this sufficient 
for the
re-configuring the mcast rate? Please let me know if I am missing 
something.

Thanks
Pradeep

>> i have never seen a card properly recovering after a firmware crash, 
>> all firmware crashes i have seen
>> are caused by bugs in firmware handling or the firmware itself. so if 
>> it recovers it crashes immediatly again
>> ending in a endless crashloop since the cause for the crash hasnt been 
>> fixed. they are often triggered by extern clients for instance which 
>> still remain in the field.
>> i think firmware crashes should not be hidden by recovering. this 
>> would also force users to report problems more frequently, than they 
>> do
>> since they dont even take notice of such problems
> 
> We see recovery work often.  If you see repeatable crashes, post them
> to the list.
> 
> Do you know of any particular external clients that cause these 
> crashes?
> 
> Thanks,
> Ben