Re: [PATCH] ath10k: Remove ATH10K_STATE_RESTARTED in simulate fw crash

[Kalle Valo <kvalo@codeaurora.org>]

Wen Gong <wgong@qti.qualcomm.com> writes:

>> > > > It is because the state has not changed to ATH10K_STATE_ON
>> > > > immediately, then it will have more than two simulate crash
>> > > > process running meanwhile, and complete/wakeup some field twice,
>> > > > it destroy the normal recovery process.
>> > >
>> > > This was intended to allow testing not only firmware crash path (and
>> > > recovery) but also firmware crash while recovering from a firmware crash.
>> > >
>> > If firmware is recovering from crash, then simulate a new crash will trigger
>> error.
>> > So remove it.
>> 
>> That's actually a feature, not a bug. If firmware crashes while driver is
>> restarting after a crash then its likely going to fail again and again causing a
>> crash-restart loop which can affect system performance and responsiveness.
>> It's better to give up and let the system admin take over.
>> 
>> If it's still bothering you then please consider a crash counter threshold so
>> that, e.g. after 5 crash-while-restarting it's going to give up. However I doubt
>> it's worth the effort. My experience tells me firmware crashes during
>> recovery are rarely, if at all, transient.
>> 
>> The simulated fw crash is not representative here. It's a mere tool to test
>> driver code.
>
> The simulated fw crash is only a tool for user to trigger fw crash
> with command

I think Michal knows what simulate_fw_crash as he is the one who
implemented it in commit 278c4a85e626 :)

> This change's purpose is to disallow user to trigger fw crash if the fw is not in a
> Normal state.
>
> If the fw is in recovering state triggered by user's command or by fw, then it will
> disallow user to run command to trigger fw crash again until fw become to a normal
> State.

I agree with Michal here and his proposal about having a crash counter
sounds like a good to me. So I'm dropping this patch.

-- 
Kalle Valo