Re: [PATCH v2 09/21] ath10k: print fw debug messages in hex.

[Ben Greear <greearb@xxxxxxxxxxxxxxx>]

On 09/14/2016 07:18 AM, Valo, Kalle wrote:
> greearb@xxxxxxxxxxxxxxx writes:
>
> > From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
> >
> > This allows user-space tools to decode debug-log
> > messages by parsing dmesg or /var/log/messages.
> >
> > Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>
> Don't tracing points already provide the same information?

Tracing tools are difficult to set up and may not be available on
random embedded devices.  And if we are dealing with bug reports from
the field, most users will not be able to set it up regardless.

There are similar ways to print out hex, but the logic below creates
specific and parseable logs in the 'dmesg' output and similar.

I have written a tool that can decode these messages into useful human-readable
text so that I can debug firmware issues both locally and from field reports.

Stock firmware generates similar logs and QCA could write their own decode logic
for their firmware versions.

Thanks,
Ben


> > +void ath10k_dbg_print_fw_dbg_buffer(struct ath10k *ar, __le32 *ibuf, int len,
> > +				    const char* lvl)
> > +{
> > +	/* Print out raw hex, external tools can decode if
> > +	 * they care.
> > +	 * TODO:  Add ar identifier to messages.
> > +	 */
> > +	int q = 0;
> > +
> > +	dev_printk(lvl, ar->dev, "ath10k_pci ATH10K_DBG_BUFFER:\n");
> > +	while (q < len) {
> > +		if (q + 8 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X %08X %08X %08X %08X %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1], ibuf[q+2], ibuf[q+3],
> > +			       ibuf[q+4], ibuf[q+5], ibuf[q+6], ibuf[q+7]);
> > +			q += 8;
> > +		}
> > +		else if (q + 7 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X %08X %08X %08X %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1], ibuf[q+2], ibuf[q+3],
> > +			       ibuf[q+4], ibuf[q+5], ibuf[q+6]);
> > +			q += 7;
> > +		}
> > +		else if (q + 6 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X %08X %08X %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1], ibuf[q+2], ibuf[q+3],
> > +			       ibuf[q+4], ibuf[q+5]);
> > +			q += 6;
> > +		}
> > +		else if (q + 5 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X %08X %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1], ibuf[q+2], ibuf[q+3],
> > +			       ibuf[q+4]);
> > +			q += 5;
> > +		}
> > +		else if (q + 4 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1], ibuf[q+2], ibuf[q+3]);
> > +			q += 4;
> > +		}
> > +		else if (q + 3 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1], ibuf[q+2]);
> > +			q += 3;
> > +		}
> > +		else if (q + 2 <= len) {
> > +			printk("%sath10k: [%04d]: %08X %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q], ibuf[q+1]);
> > +			q += 2;
> > +		}
> > +		else if (q + 1 <= len) {
> > +			printk("%sath10k: [%04d]: %08X\n",
> > +			       lvl, q,
> > +			       ibuf[q]);
> > +			q += 1;
> > +		}
> > +		else {
> > +			break;
> > +		}
> > +	}/* while */
> > +
> > +	dev_printk(lvl, ar->dev, "ATH10K_END\n");
> > +}
>
> Isn't this almost the same as what ath10k_dbg_dump() does?

--
Ben Greear <greearb@xxxxxxxxxxxxxxx>
Candela Technologies Inc  http://www.candelatech.com