Hi Heitor. A few comments below on the formatting.
Thanks,
Brian
On 11/13/2015 01:33 PM, Heitor Ricardo Alves de Siqueira wrote:
> +
> +#define JIFF_TO_USEC(x) x * 1000 * 1000 / sysconf(_SC_CLK_TCK)
> +/**
> + * print_trace -
> + * @hdr: ipr entry header pointer
> + *
> + * Returns:
> + * nothing
> + **/
> +static void print_trace(struct ipr_dump_entry_header *hdr)
> +{
> + u32 i, min_pos, min_time, num_entries, offset;
> + struct ipr_trace_entry *raw_trace;
> + struct ipr_trace_entry t;
> + const char *cmnd;
> +
> + num_entries = hdr->len / sizeof(*raw_trace);
> + raw_trace = dump_map + hdr->offset;
> + min_pos = 0;
> + min_time = TARGET_TO_HOST32(raw_trace->time, hdr->eye_catcher);
> +
> + fprintf(out_fp, "\t entries found: %d\n\n", num_entries);
> + for (i = 1; i < num_entries; ++i) {
> + raw_trace = dump_map + hdr->offset + i*sizeof(*raw_trace);
> + if (TARGET_TO_HOST32(raw_trace->time, hdr->eye_catcher) <
> + min_time) {
> + min_time = TARGET_TO_HOST32(raw_trace->time,
> + hdr->eye_catcher);
> + min_pos = i;
> + }
> + }
> + for (i = 0; i < num_entries; ++i) {
> + offset = hdr->offset +
> + ((min_pos+i)%num_entries) * sizeof(*raw_trace);
> + t = read_trace_data(offset, hdr->eye_catcher);
> + fprintf(out_fp, "\t %08X %02X%02X%02X%02X %08X %08X",
> + t.time, t.op_code, t.ata_op_code, t.type, t.cmd_index,
> + t.res_handle, t.u.add_data);
> + if (verbose_trace) {
> + fprintf(out_fp, "\t | \t%lld\xC2\xB5s\t",
> + (long long unsigned)JIFF_TO_USEC(t.time));
For the time stamp here, I think it might be more useful if we used the
timestamp
of the oldest trace entry as time 0, and have all subsequent times be a delta of
that time.
> + switch(t.type) {
> + case IPR_TRACE_START:
> + fprintf(out_fp, "IPR_TRACE_START\t\t");
> + break;
> + case IPR_TRACE_FINISH:
> + fprintf(out_fp, "IPR_TRACE_FINISH\t");
Suggest we shorten these strings to just be START / FINISH
> + break;
> + default:
> + fprintf(out_fp, "\t\t\t");
> + }
> + cmnd = get_scsi_command_literal(t.op_code);
> + if (cmnd)
> + fprintf(out_fp, "%s", cmnd);
> +
> + }
> + fprintf(out_fp, "\n");
> + }
> +}
> +
> +/**
> + * print_sdt -
> + * @data_offset: offset to sdt data
> + *
> + * Returns:
> + * nothing
> + **/
> +static void print_sdt(u32 data_offset)
> +{
> + u32 i;
> + struct ipr_sdt_header *raw_hdr;
> + struct ipr_sdt_entry *raw_entry;
> +
> + raw_hdr = dump_map + data_offset;
> +
> + fprintf(out_fp, "\t state: 0x%X", htobe32(raw_hdr->state));
> + if (htobe32(raw_hdr->state) == IPR_FMT2_SDT_READY_TO_USE ||
> + htobe32(raw_hdr->state) == IPR_FMT3_SDT_READY_TO_USE)
> + fprintf(out_fp, " (READY_TO_USE)\n");
> + else
> + fprintf(out_fp, "\n");
> + fprintf(out_fp, "\t entries found: %d\n\n",
> + htobe32(raw_hdr->entries_used));
> +
> + for (i = 0; i < htobe32(raw_hdr->entries_used); ++i) {
> + raw_entry = dump_map + data_offset + sizeof(*raw_hdr)
> + + i * sizeof(*raw_entry);
> + fprintf(out_fp, "\t %08X %08X %02X%02X%02X%02X %04X\n",
> + htobe32(raw_entry->bar_str_offset),
> + htobe32(raw_entry->end_offset),
> + raw_entry->entry_byte, raw_entry->endian,
> + raw_entry->valid_entry, raw_entry->resv,
> + htobe16(raw_entry->priority));
> + }
> +}
> +
> +/**
> + * dump_entry_data -
> + * @hdr: ipr dump entry header pointer
> + *
> + * Returns:
> + * nothing
> + **/
> +static void dump_entry_data(struct ipr_dump_entry_header *hdr)
> +{
> + u32 i;
> + char *s;
> +
> + switch (hdr->id) {
> + case ID_IOA_DUMP:
> + fprintf(out_fp, "Smart Dump Table Data:\n");
> + print_sdt(hdr->offset);
We can probably skip printing this to the output. Its not really useful for
driver debug.
> + break;
> + case ID_IOA_LOC:
> + fprintf(out_fp, "IOA Location:\n");
> + s = dump_map+hdr->offset;
> + fprintf(out_fp, "\t %.*s\n\n", hdr->len, s);
> + break;
> + case ID_DRV_VER:
> + fprintf(out_fp, "Driver Version:\n");
> + s = dump_map+hdr->offset;
> + fprintf(out_fp, "\t %.*s\n\n", hdr->len, s);
> + break;
> + case ID_DRV_TRACE:
> + fprintf(out_fp, "Driver Trace Entries:\n");
> + print_trace(hdr);
> + fprintf(out_fp, "\n");
> + break;
> + case ID_DRV_TYPE:
> + fprintf(out_fp, "Driver Type:\n");
> + i = TARGET_TO_HOST32(*(int*)(dump_map+hdr->offset),
> + hdr->eye_catcher);
> + fprintf(out_fp, "\t CCIN: 0x%04X\n", i);
> + i = TARGET_TO_HOST32(*(int*)(dump_map+hdr->offset+sizeof(i)),
> + hdr->eye_catcher);
> + fprintf(out_fp,"\t Adapter firmware Version: "
> + " %02X:%02X:%02X:%02X\n\n",
Can you remove the ":" separator here? I think it will only cause confusion for
anyone
expecting a 4 byte hex value here.
> + i >> 24, /* major release */
> + (i & 0x00FF0000) >> 16, /* card type */
> + (i & 0x0000FF00) >> 8, /* minor release 0 */
> + i & 0x000000FF); /* minor release 1 */
> + break;
> + case ID_DRV_CTRL_BLK:
> + case ID_DRV_PEND_OPS:
> + /* not implemented yet */
> + fprintf(stderr,
> + "Warning: found IPR Control Block or IPR Pending Ops!\n"
> + "These are not implemented yet. Skipping...\n");
> + break;
> + default:
> + fprintf(stderr,
> + "Warning: did not recognize data identification "
> + "(id 0x%08X). Skipping...\n\n", hdr->id);
> + }
> +}
> +
--
Brian King
Power Linux I/O
IBM Linux Technology Center
------------------------------------------------------------------------------
_______________________________________________
Iprdd-devel mailing list
Iprdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iprdd-devel
