[RFC 02/12] ath10k: htc: rx trailer lookahead support

[Valo, Kalle]

Erik Stromdahl <erik.stromdahl at gmail.com> writes:

> On 11/15/2016 10:57 AM, Michal Kazior wrote:
>> On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromdahl at gmail.com> wrote:
>>> The RX trailer parsing is now capable of parsing lookahead reports.
>>> This is needed by SDIO/mbox.
>> 
>> It'd be useful to know what exactly lookahead will be used for. What
>> payload does it carry.
>> 
> It carries the 4 first bytes of the next RX message, i.e. the first 4
> bytes of an HTC header.
>
> It is used by the sdio interrupt handler to know if the next packet is a
> part of an RX bundle, which endpoint it belongs to and how long it is
> (so we know how many bytes to allocate).

Please add that to the commit log, it's useful information. Or maybe a
code comment would be better?

>>> +static int
>>> +ath10k_htc_process_lookahead(struct ath10k_htc *htc,
>>> +                            const struct ath10k_htc_lookahead_report *report,
>>> +                            int len,
>>> +                            enum ath10k_htc_ep_id eid,
>>> +                            u32 *next_lk_ahds,
>> 
>> next_lk_ahds should be u8 or void. From what I understand by glancing
>> through the code it is an agnostic buffer that carries payload which
>> is later interpreted either as eid or htc header, right? void is
>> probably most suitable in this case for passing around and u8 for
>> inline-based storage.
>> 
> Sounds reasonable, I'll change to void pointer.
>
>> I'd also avoid silly abbreviations. Probably "lookahead" alone is enough.
>> 
> Ok, this abbreviation was actually taken from the ath6kl code. I think
> the intention was to reduce line lengths.

Most likely that comes from the staging ath6kl driver which again is a
fork of the Atheros internal driver. I agree with Michal, better to
avoid silly abbreviations as much as possible. Readability is most
important.

-- 
Kalle Valo