[RFC 1/4] ath10k: provide firmware crash info via debugfs.
kvalo at qca.qualcomm.com
Thu Jun 5 23:13:09 PDT 2014
Ben Greear <greearb at candelatech.com> writes:
>>>>> +/* This will store at least the last 128 entries. */
>>>>> +#define ATH10K_DBGLOG_DATA_LEN (128 * 7 * 4)
>>>> Where does the 7 and 4 come from? Can't we use sizeof() to compute this?
>>> It is from the guts of how the firmware does debug logs.
>>> Each entry is a max of 7 32-bit integers in length.
>>>> The 128 should probably be a separate #define?
>>> I don't see why...
>> To make the code more readable.
>>> dbglog messages are variable number of 32-bit integers in length, so
>>> the 128 is fairly arbitrary.
>> A person should immeaditely understand where the numbers are coming from
>> and not start googling about it. The minimum is to put your description
>> above to the comment. And it would be clearer to replace 4 with
> Would the comments I put in the PATCH 1/4 combined with the sizeof(4)
> be sufficient, or do you want actual defines?
What you wrote about should be enough, I think you can skip the defines
for now. Something like:
/* Each entry is a max of 7 32-bit integers in length, let's choose
* arbitrarily 128 entries */
More information about the ath10k