On 18 August 2014 13:39, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Michal Kazior <michal.kazior@xxxxxxxxx> writes:
>
>> On 9 August 2014 20:08, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
>>> From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>>
>>> Store the firmware crash registers and last 128 or so
>>> firmware debug-log ids and present them to user-space
>>> via debugfs.
>>>
>>> Should help with figuring out why the firmware crashed.
>>>
>>> Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>> Signed-off-by: Kalle Valo <kvalo@xxxxxxxxxxxxxxxx>
>>> ---
>>> drivers/net/wireless/ath/ath10k/core.h | 27 +++
>>> drivers/net/wireless/ath/ath10k/debug.c | 273 +++++++++++++++++++++++++++++++
>>> drivers/net/wireless/ath/ath10k/debug.h | 22 ++
>>> drivers/net/wireless/ath/ath10k/hw.h | 30 +++
>>> drivers/net/wireless/ath/ath10k/pci.c | 133 ++++++++++++++-
>>> drivers/net/wireless/ath/ath10k/pci.h | 3
>>> 6 files changed, 478 insertions(+), 10 deletions(-)
>> [...]
>>
>>> +struct ath10k_dump_file_data {
>>> + /* dump file information */
>>> +
>>> + /* "ATH10K-FW-DUMP" */
>>> + char df_magic[16];
>>> +
>>> + u32 len;
>>> +
>>> + /* 0x1 if host is big-endian */
>>> + u32 big_endian;
>>
>> This isn't entirely correct. Depending on host endianess you'll end up
>> with 0x1 or 0x1000000. This will still work if you do a boolean
>> evaluation of it in userspace or compare it to 0, but god forbid to
>> compare it with 0x1.
>
> That's true. Didn't you at one point suggest just always using little
> endian? I think that's simplest approach here.
Yes. I did point that out at some time ago.
>>> + /* some info we can get from ath10k struct that might help */
>>> +
>>> + u8 uuid[16];
>>> +
>>> + u32 chip_id;
>>> +
>>> + /* 0 for now, in place for later hardware */
>>> + u32 bus_type;
>>
>> Maybe we should have an enum for that instead of using a hardcoded 0?
>
> We had that but you removed it in 3a0861fffd223 =)
.. right :-) I suppose we could just remove the bus_type then? We do
have an unused[128] for future expansion, don't we?
>> and/or the memory should be allocated outside this function completely
>> and make it consume the crashed_since_read (i.e. set it to false) once
>> it's done (while the data_lock is still held).
>
> You mean like allocating the memory in advance, for example during
> module probe time? Then we would have a big chunk of memory we don't use
> for anything most of the time.
I meant just moving it up in the call stack, e.g. to
ath10k_fw_crash_dump_open().
Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
