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

[Kalle Valo <kvalo@qca.qualcomm.com>]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 18 August 2014 13:39, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> On 9 August 2014 20:08, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>>
>>>> +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.

Ok. I started converting to use little endian already.

>>>> +       /* 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 :-) 

Sorry, I couldn't resist :)

> I suppose we could just remove the bus_type then? We do have an
> unused[128] for future expansion, don't we?

We could, but then we would have to modify the crash dump tools. I would
rather be prepared for this, it's only a u32 anyway. If we have to do
something, I would prefer to get back the enum.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k