From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga14.intel.com ([192.55.52.115]:37365 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752768AbcIORef (ORCPT ); Thu, 15 Sep 2016 13:34:35 -0400 From: "Grumbach, Emmanuel" To: "greearb@candelatech.com" , "kvalo@qca.qualcomm.com" CC: "linux-wireless@vger.kernel.org" , "ath10k@lists.infradead.org" Subject: Re: [PATCH v2 09/21] ath10k: print fw debug messages in hex. Date: Thu, 15 Sep 2016 17:34:29 +0000 Message-ID: <1473960869.31073.15.camel@intel.com> (sfid-20160915_193446_472375_E3B738A8) References: <1462986153-16318-1-git-send-email-greearb@candelatech.com> <1462986153-16318-10-git-send-email-greearb@candelatech.com> <87oa3qefac.fsf@kamboji.qca.qualcomm.com> <57D96910.2010608@candelatech.com> <87k2edb6kp.fsf@kamboji.qca.qualcomm.com> In-Reply-To: Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: T24gVGh1LCAyMDE2LTA5LTE1IGF0IDA4OjE0IC0wNzAwLCBCZW4gR3JlZWFyIHdyb3RlOg0KPiBP biAwOS8xNS8yMDE2IDA3OjA2IEFNLCBWYWxvLCBLYWxsZSB3cm90ZToNCj4gPiBCZW4gR3JlZWFy IDxncmVlYXJiQGNhbmRlbGF0ZWNoLmNvbT4gd3JpdGVzOg0KPiA+IA0KPiA+ID4gT24gMDkvMTQv MjAxNiAwNzoxOCBBTSwgVmFsbywgS2FsbGUgd3JvdGU6DQo+ID4gPiA+IGdyZWVhcmJAY2FuZGVs YXRlY2guY29tIHdyaXRlczoNCj4gPiA+ID4gDQo+ID4gPiA+ID4gRnJvbTogQmVuIEdyZWVhciA8 Z3JlZWFyYkBjYW5kZWxhdGVjaC5jb20+DQo+ID4gPiA+ID4gDQo+ID4gPiA+ID4gVGhpcyBhbGxv d3MgdXNlci1zcGFjZSB0b29scyB0byBkZWNvZGUgZGVidWctbG9nDQo+ID4gPiA+ID4gbWVzc2Fn ZXMgYnkgcGFyc2luZyBkbWVzZyBvciAvdmFyL2xvZy9tZXNzYWdlcy4NCj4gPiA+ID4gPiANCj4g PiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBCZW4gR3JlZWFyIDxncmVlYXJiQGNhbmRlbGF0ZWNoLmNv bT4NCj4gPiA+ID4gDQo+ID4gPiA+IERvbid0IHRyYWNpbmcgcG9pbnRzIGFscmVhZHkgcHJvdmlk ZSB0aGUgc2FtZSBpbmZvcm1hdGlvbj8NCj4gPiA+IA0KPiA+ID4gVHJhY2luZyB0b29scyBhcmUg ZGlmZmljdWx0IHRvIHNldCB1cCBhbmQgbWF5IG5vdCBiZSBhdmFpbGFibGUgb24NCj4gPiA+IHJh bmRvbSBlbWJlZGRlZCBkZXZpY2VzLiAgQW5kIGlmIHdlIGFyZSBkZWFsaW5nIHdpdGggYnVnIHJl cG9ydHMNCj4gPiA+IGZyb20NCj4gPiA+IHRoZSBmaWVsZCwgbW9zdCB1c2VycyB3aWxsIG5vdCBi ZSBhYmxlIHRvIHNldCBpdCB1cCByZWdhcmRsZXNzLg0KPiA+ID4gDQo+ID4gPiBUaGVyZSBhcmUg c2ltaWxhciB3YXlzIHRvIHByaW50IG91dCBoZXgsIGJ1dCB0aGUgbG9naWMgYmVsb3cNCj4gPiA+ IGNyZWF0ZXMNCj4gPiA+IHNwZWNpZmljIGFuZCBwYXJzZWFibGUgbG9ncyBpbiB0aGUgJ2RtZXNn JyBvdXRwdXQgYW5kIHNpbWlsYXIuDQo+ID4gPiANCj4gPiA+IEkgaGF2ZSB3cml0dGVuIGEgdG9v bCB0aGF0IGNhbiBkZWNvZGUgdGhlc2UgbWVzc2FnZXMgaW50byB1c2VmdWwNCj4gPiA+IGh1bWFu LXJlYWRhYmxlDQo+ID4gPiB0ZXh0IHNvIHRoYXQgSSBjYW4gZGVidWcgZmlybXdhcmUgaXNzdWVz IGJvdGggbG9jYWxseSBhbmQgZnJvbQ0KPiA+ID4gZmllbGQgcmVwb3J0cy4NCj4gPiA+IA0KPiA+ ID4gU3RvY2sgZmlybXdhcmUgZ2VuZXJhdGVzIHNpbWlsYXIgbG9ncyBhbmQgUUNBIGNvdWxkIHdy aXRlIHRoZWlyDQo+ID4gPiBvd24gZGVjb2RlIGxvZ2ljDQo+ID4gPiBmb3IgdGhlaXIgZmlybXdh cmUgdmVyc2lvbnMuDQo+ID4gDQo+ID4gUmVpbnZlbnRpbmcgdGhlIHdoZWVsIGJ5IHVzaW5nIHBy aW50ayBhcyB0aGUgZGVsaXZlcnkgbWVjaGFuaXNtDQo+ID4gZG9lc24ndA0KPiA+IHNvdW5kIGxp a2UgYSBnb29kIGlkZWEuIElJUkMgRW1tYW51ZWwgdGFsa2VkIGFib3V0IHNvbWUga2luZCBvZg0K PiA+IGZpcm13YXJlDQo+ID4gZGVidWdnaW5nIGZyYW1ld29yaywgaGUgbWlnaHQgaGF2ZSBzb21l IGlkZWFzLg0KPiANCj4gV2FpdGluZyBmb3IgbWFnaWNhbCBmcmFtZXdvcmtzIHRvIGZpeCBwcm9i bGVtcyBpcyBldmVuIHdvcnNlLg0KPiANCkl0IGhhcyBiZWVuIHllYXJzIHNpbmNlIGF0aDEwayBo YXMgYmVlbiBpbiB0aGUga2VybmVsLiAgVGhlcmUgaXMNCj4gYmFzaWNhbGx5DQo+IHN0aWxsIG5v IHdheSB0byBkZWJ1ZyB3aGF0IHRoZSBmaXJtd2FyZSBpcyBkb2luZy4NCj4gDQoNCkkga25vdyB0 aGUgZmVlbGluZyA6KSBJIHdhcyBpbiB0aGUgc2FtZSBzaXR1YXRpb24gYmVmb3JlIEkgYWRkZWQg c3R1ZmYNCmZvciBpd2x3aWZpLg0KDQo+IE15IHBhdGNoIGdpdmVzIHlvdSBzb21ldGhpbmcgdGhh dCBjYW4gd29yayByaWdodCBub3csIHdpdGggdGhlDQo+IHN0YW5kYXJkICdkbWVzZycNCj4gZnJh bWV3b3JrIGZvdW5kIGluIHZpcnR1YWxseSBhbGwga2VybmVscyBuZXcgYW5kIG9sZCwgYW5kIGl0 IGhhcyBiZWVuDQo+IHByb3Zlbg0KPiB0byBiZSB1c2VmdWwgaW4gdGhlIGZpZWxkLiAgVGhlIG1l c3NhZ2VzIGFyZSBhbHNvIG5pY2VseSBpbnRlcmxlYXZlZA0KPiB3aXRoIHRoZQ0KPiByZXN0IG9m IHRoZSBtYWM4MDIxMSBzdGFjayBtZXNzYWdlcyBhbmQgYW55IG90aGVyIGRyaXZlciBtZXNzYWdl cywgc28NCj4geW91IGhhdmUNCj4gY29udGV4dC4NCj4gDQo+IElmIHNvbWVvbmUgd2FudHMgdG8g YWRkIHN1cHBvcnQgZm9yIGEgZnJhbWV3b3JrIGxhdGVyLCB0aGVuIGJ5IGFsbA0KPiBtZWFucywg cG9zdA0KPiB0aGUgcGF0Y2hlcyB3aGVuIGl0IGlzIHJlYWR5Lg0KDQpGcm9tIG15IGV4cGVyaWVu Y2UsIGEgc3Ryb25nIGFuZCBlYXN5LXRvLXVzZSBmaXJtd2FyZSBkZWJ1Zw0KaW5mcmFzdHJ1Y3R1 cmUgaXMgaW1wb3J0YW50IGJlY2F1c2UgdHlwaWNhbGx5LCB0aGUgZmlybXdhcmUgaXMgd3JpdHRl bg0KYnkgb3RoZXIgcGVvcGxlIHdobyBoYXZlIGRpZmZlcmVudCBwcmlvcml0aWVzIChhbmQgYXJl IG5vdCBhbHdheXMgTGludXgNCndpemFyZHMpIGV0Yy4uLiBCZWluZyBhYmxlIHRvIGdpdmUgdGhl bSBnb29kIGRhdGEgaXMgdGhlIG9ubHkgd2F5IHRvDQpoYXZlIHRoZW0gZml4IHRoZWlyIGJ1Z3Mg OikgRm9yIHVzLCBpdCB3YXMgcmVhbGx5IGEgZ2FtZSBjaGFuZ2VyLiBXaGVuDQp5b3Ugd29yayBm b3IgYSBiaWcgY29ycG9yYXRlLCBoYXZpbmcgMiBncm91cHMgd29yayBiZXR0ZXIgdG9nZXRoZXIN CmFsd2F5cyBoYXMgYSBiaWcgaW1wYWN0LiBUaGF0J3MgZm9yIHRoZSBwaGlsb3NvcGhpY2FsIHBh cnQgOikNCg0KRldJVzogd2hhdCBJIGRpZCBoYXMgbm90aGluZyB0byBkbyB3aXRoIEZXICdsaXZl IHRyYWNpbmcnLCBidXQgd2l0aA0KZmlybXdhcmUgZHVtcHMuIE9uZSBwYXJ0IG9mIG91ciBmaXJt d2FyZSBkdW1wcyBpbmNsdWRlIHRyYWNpbmcuIFdlIGFsc28NCmhhdmUgImZpcm13YXJlIHByaW50 cyIsIGJ1dCB3ZSBkb24ndCBwcmludCB0aGVtIGluIHRoZSBrZXJuZWwgbG9nIGFuZA0KdGhleSBh cmUgbm90IHBhcnQgb2YgdGhlIGZpcm13YXJlIGR1bXAgdGhpbmcuIFdlIHJhdGhlciByZWNvcmQg dGhlbSBpbg0KdHJhY2Vwb2ludHMganVzdCBsaWtlIHJlYWxseSAqYW55dGhpbmcqIHRoYXQgY29t ZXMgZnJvbSB0aGUgZmlybXdhcmUuDQpCYXNpY2FsbHksIHdlIGhhdmUgMiBsYXllcnMsIHRoZSB0 cmFuc3BvcnQgbGF5ZXIgKFBDSWUpIGFuZCB0aGUNCm9wZXJhdGlvbl9tb2RlIGxheWVyLiBUaGUg Zmlyc3QganVzdCBicmluZ3MgdGhlIGRhdGEgZnJvbSB0aGUgZmlybXdhcmUNCmFuZCBpbiB0aGF0 IGxheWVyIHdlICpibGluZGx5KiByZWNvcmQgZXZlcnl0aGluZyBpbiB0cmFjZXBvaW50cy4gSW4g dGhlDQpvcGVyYXRpb25fbW9kZSBsYXllciwgd2UgbG9vayBhdCB0aGUgZGF0YSBpdHNlbGYuIElu IGNhc2Ugb2YgZGVidWcNCnByaW50cyBmcm9tIHRoZSBmaXJtd2FyZSwgd2Ugc2ltcGx5IGRpc2Nh cmQgdGhlbSwgYmVjYXVzZSB3ZSBkb24ndA0KcmVhbGx5IGNhcmUgb2YgdGhlIG1lYW5pbmcuIEFs bCB3ZSB3YW50IGlzIHRvIGhhdmUgdGhlbSBnbyB0aHJvdWdoIHRoZQ0KUENJZSBsYXllciBzbyB0 aGF0IHRoZXkgYXJlIHJlY29yZGVkIGluIHRoZSB0cmFjZXBvaW50cy4NCldoZW4gd2UgZmluaXNo IHJlY29yZGluZyB0aGUgc2VxdWVuY2Ugd2Ugd2FudGVkIHdpdGggdHJhY2luZyAodHJhY2UNCi1j bWQpLCB3ZSBwYXJzZSB0aGUgb3V0cHV0IGFuZCB0aGVuLCB3ZSBwYXJzZSB0aGUgZmlybXdhcmUg cHJpbnRzLg0KSU1ITywgdGhpcyBpcyBtb3JlIHJlbGlhYmxlIHRoYW4ga2VybmVsIGxvZ3MgYW5k IHlvdSBkb24ndCBsb3NlIHRoZQ0KYWxpZ25tZW50IHdpdGggdGhlIGRyaXZlciB0cmFjZXMgYXMg bG9uZyBhcyB5b3UgaGF2ZSBkcml2ZXIgZGF0YSBpbg0KdHJhY2Vwb2ludHMgYXMgd2VsbC4NCg0K PiANCj4gVGhhbmtzLA0KPiBCZW4NCj4gDQo+IA== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mga04.intel.com ([192.55.52.120]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bkaYo-0003k8-VS for ath10k@lists.infradead.org; Thu, 15 Sep 2016 17:34:56 +0000 From: "Grumbach, Emmanuel" Subject: Re: [PATCH v2 09/21] ath10k: print fw debug messages in hex. Date: Thu, 15 Sep 2016 17:34:29 +0000 Message-ID: <1473960869.31073.15.camel@intel.com> References: <1462986153-16318-1-git-send-email-greearb@candelatech.com> <1462986153-16318-10-git-send-email-greearb@candelatech.com> <87oa3qefac.fsf@kamboji.qca.qualcomm.com> <57D96910.2010608@candelatech.com> <87k2edb6kp.fsf@kamboji.qca.qualcomm.com> In-Reply-To: Content-Language: en-US Content-ID: MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: "greearb@candelatech.com" , "kvalo@qca.qualcomm.com" Cc: "linux-wireless@vger.kernel.org" , "ath10k@lists.infradead.org" On Thu, 2016-09-15 at 08:14 -0700, Ben Greear wrote: > On 09/15/2016 07:06 AM, Valo, Kalle wrote: > > Ben Greear writes: > > > > > On 09/14/2016 07:18 AM, Valo, Kalle wrote: > > > > greearb@candelatech.com writes: > > > > > > > > > From: Ben Greear > > > > > > > > > > This allows user-space tools to decode debug-log > > > > > messages by parsing dmesg or /var/log/messages. > > > > > > > > > > Signed-off-by: Ben Greear > > > > > > > > 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. > > > > Reinventing the wheel by using printk as the delivery mechanism > > doesn't > > sound like a good idea. IIRC Emmanuel talked about some kind of > > firmware > > debugging framework, he might have some ideas. > > Waiting for magical frameworks to fix problems is even worse. > It has been years since ath10k has been in the kernel. There is > basically > still no way to debug what the firmware is doing. > I know the feeling :) I was in the same situation before I added stuff for iwlwifi. > My patch gives you something that can work right now, with the > standard 'dmesg' > framework found in virtually all kernels new and old, and it has been > proven > to be useful in the field. The messages are also nicely interleaved > with the > rest of the mac80211 stack messages and any other driver messages, so > you have > context. > > If someone wants to add support for a framework later, then by all > means, post > the patches when it is ready. >From my experience, a strong and easy-to-use firmware debug infrastructure is important because typically, the firmware is written by other people who have different priorities (and are not always Linux wizards) etc... Being able to give them good data is the only way to have them fix their bugs :) For us, it was really a game changer. When you work for a big corporate, having 2 groups work better together always has a big impact. That's for the philosophical part :) FWIW: what I did has nothing to do with FW 'live tracing', but with firmware dumps. One part of our firmware dumps include tracing. We also have "firmware prints", but we don't print them in the kernel log and they are not part of the firmware dump thing. We rather record them in tracepoints just like really *anything* that comes from the firmware. Basically, we have 2 layers, the transport layer (PCIe) and the operation_mode layer. The first just brings the data from the firmware and in that layer we *blindly* record everything in tracepoints. In the operation_mode layer, we look at the data itself. In case of debug prints from the firmware, we simply discard them, because we don't really care of the meaning. All we want is to have them go through the PCIe layer so that they are recorded in the tracepoints. When we finish recording the sequence we wanted with tracing (trace -cmd), we parse the output and then, we parse the firmware prints. IMHO, this is more reliable than kernel logs and you don't lose the alignment with the driver traces as long as you have driver data in tracepoints as well. > > Thanks, > Ben > > _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k