Erik Stromdahl <erik.stromdahl@xxxxxxxxx> writes: > On 11/15/2016 10:57 AM, Michal Kazior wrote: >> On 14 November 2016 at 17:33, Erik Stromdahl <erik.stromdahl@xxxxxxxxx> 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
