Re: [PATCH 2/7] ath6kl: add tracing points for sdio transfers

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2012-12-27 at 13:44 +0200, Kalle Valo wrote:

> +	if (request & HIF_WRITE)
> +		trace_ath6kl_sdio_wr(addr, request, buf, len);
> +	else
> +		trace_ath6kl_sdio_rd(addr, request, buf, len);

It would be more efficient to use a single tracepoint and have a
"direction" field or so -- then the if doesn't have to be executed when
tracing is disabled.


> +	for (i = 0; i < scat_req->scat_entries; i++) {
> +		if (scat_req->req & HIF_WRITE)
> +			trace_ath6kl_sdio_wr(scat_req->addr,
> +					     scat_req->req,
> +					     scat_req->scat_list[i].buf,
> +					     scat_req->scat_list[i].len);
> +		else
> +			trace_ath6kl_sdio_rd(scat_req->addr,
> +					     scat_req->req,
> +					     scat_req->scat_list[i].buf,
> +					     scat_req->scat_list[i].len);
> +	}

Same here, although it would be even better to move the loop into the
tracepoint ... is there a small upper bound on "scat_entries"? If yes,
you could do something like here:

http://git.kernel.org/?p=linux/kernel/git/iwlwifi/iwlwifi-next.git;a=blob;f=drivers/net/wireless/iwlwifi/iwl-devtrace.h;hb=HEAD#l325

where I put a function call into the __entry to figure out how much
space is needed.

johannes