Re: [PATCH v7 2/8] ath10k: provide firmware crash info via debugfs

[Ben Greear <greearb@candelatech.com>]

On 08/20/2014 12:29 AM, Kalle Valo wrote:
> Ben Greear <greearb@candelatech.com> writes:
>
>>>> +       /* Gather dbg-log */
>>>> +       dump_tlv = (struct ath10k_tlv_dump_data *)(buf + sofar);
>>>> +       dump_tlv->type = cpu_to_le32(ATH10K_FW_CRASH_DUMP_DBGLOG);
>>>> +       dump_tlv->tlv_len = cpu_to_le32(sizeof(crash_data->dbglog_entry_data));
>>>
>>> Hmm should this really be sizeof()? Not next_idx (which effectively
>>> defines number of bytes of the dbglog)?
>>
>> I haven't tried decoding this yet, but we may need to know the next_idx
>> to decode this properly.
>
> I really don't like the idea of having untested code in ath10k. Buggy
> code is okay (preferably document the known bugs when submitting code),
> but untested code is not.

It would be quite a waste of time to keep re-writing a user-space dump tool
while the patch set is in this much churn, I think.

It's not like ath10k is going to blow up, but maybe the decode is less useful
in that we cannot actually get a useful debuglog decode until the problem is
resolved.  We get zero firmware dbglog messages currently, so nothing is lost.

When at least the first patch or two is in, and endian issue is resolved,
I'll write a decode tool (or modify the one I have that parses /var/log/messages
to deal with the new binary dump file).

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com