Ben Greear <greearb@xxxxxxxxxxxxxxx> writes: > On 08/20/2014 12:29 AM, Kalle Valo wrote: >> Ben Greear <greearb@xxxxxxxxxxxxxxx> writes: >> >>>>> + /* 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 really don't like the idea of having untested code in ath10k. >> Buggy code is okay (preferably document the known bugs when >> submitting code), but untested code is not. > > It would be quite a waste of time to keep re-writing a user-space dump > tool while the patch set is in this much churn, I think. > It's not like ath10k is going to blow up, but maybe the decode is less > useful in that we cannot actually get a useful debuglog decode until > the problem is resolved. We get zero firmware dbglog messages > currently, so nothing is lost. Of course ath10k won't blow up, but every patch and every line of code increases the maintenance cost. For example, if I had known that the dbglog dump in your patchset doesn't work at all, I would have dropped it immediately and saved a lot of time of everyone's time, especially mine. So next time you send me untested code PLEASE clearly mention that. I assume that the developer has tested the submitted code well enough that it really works. Hoping it to work is not enough! Unless you are Johannes, Linus or Davem :) -- Kalle Valo -- 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
