On 08/19/2014 08:25 AM, Ben Greear wrote: >>> + dump_data->kernel_ver_code = cpu_to_le32(LINUX_VERSION_CODE); >>> + strlcpy(dump_data->kernel_ver, VERMAGIC_STRING, >>> + sizeof(dump_data->kernel_ver)); >>> + >>> + dump_data->tv_sec = cpu_to_le64(crash_data->timestamp.tv_sec); >>> + dump_data->tv_nsec = cpu_to_le64(crash_data->timestamp.tv_nsec); >>> + >>> + /* Gather dbg-log */ >>> + dump_tlv = (struct ath10k_tlv_dump_data *)(buf + sofar); >>> + dump_tlv->type = cpu_to_le32(ATH10K_FW_CRASH_DUMP_DBGLOG); >>> + dump_tlv->tlv_len = cpu_to_le32(sizeof(crash_data->dbglog_entry_data)); >> >> Hmm should this really be sizeof()? Not next_idx (which effectively >> defines number of bytes of the dbglog)? > > I haven't tried decoding this yet, but we may need to know the next_idx > to decode this properly. I thought some more on this on the way to work..and I think it will not be easy to decode this if we do not know the starting point for the messages. Each report from firmware will start at the beginning of a message, so we either need to do a lot of memory copying, or play tricks with a circular buffer storage. A long time ago, Kalle mentioned he didn't want any ability to decode the messages in the kernel, so the 'start' boundary will need to be the start of the message(s) received from firmware. This means that the ath10k_debug_dbglog_add will need to be improved to handle wraps more intelligently, somehow. This series has taken a long time already, so it would be fine with me to deal with the dbglog cleanup in subsequent patches, and just assume for now that the dbglog messages start at index 0 in the dbglog-entry-data. Thanks, Ben -- Ben Greear <greearb@xxxxxxxxxxxxxxx> Candela Technologies Inc http://www.candelatech.com -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html