From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: [05/31] usb: usbssp: Added first part of initialization sequence. From: Greg Kroah-Hartman Message-Id: <20180712090924.GA8255@kroah.com> Date: Thu, 12 Jul 2018 11:09:24 +0200 To: Pawel Laszczak Cc: "linux-usb@vger.kernel.org" , Felipe Balbi , "linux-kernel@vger.kernel.org" , Lukasz Tyrala , Alan Douglas List-ID: T24gVGh1LCBKdWwgMTIsIDIwMTggYXQgMDk6MDM6MzBBTSArMDAwMCwgUGF3ZWwgTGFzemN6YWsg d3JvdGU6Cj4gPiA+ICsvKiBVU0IgMi4wIGhhcmR3YXJlIExNUCBjYXBhYmlsaXR5Ki8KPiA+ID4g KyNkZWZpbmUgVVNCU1NQX0hMQwkJCSgxIDw8IDE5KQo+ID4gPiArI2RlZmluZSBVU0JTU1BfQkxD CQkJKDEgPDwgMjApCj4gPiAKPiA+IEFnYWluLCBCSVQoKSBwbGVhc2UuCj4gPiAKPiA+ID4gK2lu dCB1c2Jzc3BfaGFuZHNoYWtlKHZvaWQgX19pb21lbSAqcHRyLCB1MzIgbWFzaywgdTMyIGRvbmUs IGludCB1c2VjKQo+ID4gPiArewo+ID4gPiArCXUzMglyZXN1bHQ7Cj4gPiAKPiA+IFNvbWUgcGxh Y2VzIHlvdSB1c2UgdGFicyBmb3IgdGhlIHZhcmlhYmxlIGRlY2xhcmF0aW9ucywgYW5kIHNvbWUg eW91IGRvCj4gPiBub3QuICBQaWNrIGEgc2luZ2xlIHN0eWxlIGFuZCBzdGljayB0byBpdCBwbGVh c2UuCj4gPiAKPiA+ID4gKwo+ID4gPiArCWRvIHsKPiA+ID4gKwkJcmVzdWx0ID0gcmVhZGwocHRy KTsKPiA+ID4gKwkJaWYgKHJlc3VsdCA9PSB+KHUzMikwKQkvKiBjYXJkIHJlbW92ZWQgKi8KPiA+ ID4gKwkJCXJldHVybiAtRU5PREVWOwo+ID4gPiArCQlyZXN1bHQgJj0gbWFzazsKPiA+ID4gKwkJ aWYgKHJlc3VsdCA9PSBkb25lKQo+ID4gPiArCQkJcmV0dXJuIDA7Cj4gPiA+ICsJCXVkZWxheSgx KTsKPiA+ID4gKwkJdXNlYy0tOwo+ID4gPiArCX0gd2hpbGUgKHVzZWMgPiAwKTsKPiA+ID4gKwly ZXR1cm4gLUVUSU1FRE9VVDsKPiA+IAo+ID4gV2UgZG9uJ3QgaGF2ZSBhIGJ1aWx0LWluIGtlcm5l bCBmdW5jdGlvbiB0byBkbyB0aGlzIHR5cGUgb2YgdGhpbmcgYWxyZWFkeT8KPiA+IFRoYXQncyBz YWQuICBPaCB3ZWxsLi4uCj4gPiAKPiA+ID4gK2ludCB1c2Jzc3BfaW5pdChzdHJ1Y3QgdXNic3Nw X3VkYyAqdXNic3NwX2RhdGEpIHsKPiA+ID4gKwlpbnQgcmV0dmFsID0gMDsKPiA+ID4gKwo+ID4g PiArCXVzYnNzcF9kYmdfdHJhY2UodXNic3NwX2RhdGEsIHRyYWNlX3VzYnNzcF9kYmdfaW5pdCwK PiA+ICJ1c2Jzc3BfaW5pdCIpOwo+ID4gPiArCj4gPiA+ICsJc3Bpbl9sb2NrX2luaXQoJnVzYnNz cF9kYXRhLT5sb2NrKTsKPiA+ID4gKwlzcGluX2xvY2tfaW5pdCgmdXNic3NwX2RhdGEtPmlycV90 aHJlYWRfbG9jayk7Cj4gPiA+ICsKPiA+ID4gKwkvL1RPRE86IG1lbW9yeSBpbml0aWFsaXphdGlv bgo+ID4gPiArCS8vcmV0dmFsID0gdXNic3NwX21lbV9pbml0KHVzYnNzcF9kYXRhLCBHRlBfS0VS TkVMKTsKPiA+ID4gKwo+ID4gPiArCXVzYnNzcF9kYmdfdHJhY2UodXNic3NwX2RhdGEsIHRyYWNl X3VzYnNzcF9kYmdfaW5pdCwKPiA+ID4gKwkJCSJGaW5pc2hlZCB1c2Jzc3BfaW5pdCIpOwo+ID4g Cj4gPiBXaGVuIHlvdXIgdHJhY2UgZnVuY3Rpb25zIGRvIG5vdGhpbmcgYnV0IHNheSAiZW50ZXJl ZCBhIGZ1bmN0aW9uIiwgYW5kCj4gPiAiZXhpdGVkIGEgZnVuY3Rpb24iLCB3aHkgZXZlbiBoYXZl IHRoZW0/ICBmdHJhY2UgY2FuIHByb3ZpZGUgdGhhdCBmb3IgeW91Cj4gPiBhbHJlYWR5LCBubyBu ZWVkIHRvIG92ZXJsb2FkIHRoYXQgb24gdGhlIHRyYWNpbmcgZnJhbWV3b3JrLCByaWdodD8KPiAK PiBEbyB5b3Ugc3VnZ2VzdCB0byB1c2Ugb25seTogCj4gCXRyYWNlX3VzYnNzcF9kYmdfaW5pdCgi RmluaXNoZWQgdXNic3NwX2luaXQiKTsgCj4gaW5zdGVhZDogCj4gCXVzYnNzcF9kYmcodXNic3Nw X2RhdGEsICIlcFZcbiIsICJGaW5pc2hlZCB1c2Jzc3BfaW5pdCIpOwo+IAl0cmFjZV91c2Jzc3Bf ZGJnX2luaXQoIkZpbmlzaGVkIHVzYnNzcF9pbml0Iik7Cj4gPwo+IAo+IEknbSBzaW1wbGUgcmUt dXNlZCB0aGUgY29kZSBmcm9tIFhIQ0kgZHJpdmVyLiBJdCdzIHJlYWxseSByZWR1bmRhbnQsIAo+ IGJ1dCBJIGRvbid0IGtub3cgdGhlIGludGVudGlvbiBvZiBhdXRob3IK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6472AC43A1D for ; Thu, 12 Jul 2018 09:09:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 1682420BF2 for ; Thu, 12 Jul 2018 09:09:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1682420BF2 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732421AbeGLJSI (ORCPT ); Thu, 12 Jul 2018 05:18:08 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:57558 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726582AbeGLJSI (ORCPT ); Thu, 12 Jul 2018 05:18:08 -0400 Received: from localhost (LFbn-1-12247-202.w90-92.abo.wanadoo.fr [90.92.61.202]) by mail.linuxfoundation.org (Postfix) with ESMTPSA id 09F7986A; Thu, 12 Jul 2018 09:09:26 +0000 (UTC) Date: Thu, 12 Jul 2018 11:09:24 +0200 From: Greg Kroah-Hartman To: Pawel Laszczak Cc: "linux-usb@vger.kernel.org" , Felipe Balbi , "linux-kernel@vger.kernel.org" , Lukasz Tyrala , Alan Douglas Subject: Re: [PATCH 05/31] usb: usbssp: Added first part of initialization sequence. Message-ID: <20180712090924.GA8255@kroah.com> References: <1531374448-26532-1-git-send-email-pawell@cadence.com> <1531374448-26532-6-git-send-email-pawell@cadence.com> <20180712062714.GI20905@kroah.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.10.0 (2018-05-17) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 12, 2018 at 09:03:30AM +0000, Pawel Laszczak wrote: > > > +/* USB 2.0 hardware LMP capability*/ > > > +#define USBSSP_HLC (1 << 19) > > > +#define USBSSP_BLC (1 << 20) > > > > Again, BIT() please. > > > > > +int usbssp_handshake(void __iomem *ptr, u32 mask, u32 done, int usec) > > > +{ > > > + u32 result; > > > > Some places you use tabs for the variable declarations, and some you do > > not. Pick a single style and stick to it please. > > > > > + > > > + do { > > > + result = readl(ptr); > > > + if (result == ~(u32)0) /* card removed */ > > > + return -ENODEV; > > > + result &= mask; > > > + if (result == done) > > > + return 0; > > > + udelay(1); > > > + usec--; > > > + } while (usec > 0); > > > + return -ETIMEDOUT; > > > > We don't have a built-in kernel function to do this type of thing already? > > That's sad. Oh well... > > > > > +int usbssp_init(struct usbssp_udc *usbssp_data) { > > > + int retval = 0; > > > + > > > + usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init, > > "usbssp_init"); > > > + > > > + spin_lock_init(&usbssp_data->lock); > > > + spin_lock_init(&usbssp_data->irq_thread_lock); > > > + > > > + //TODO: memory initialization > > > + //retval = usbssp_mem_init(usbssp_data, GFP_KERNEL); > > > + > > > + usbssp_dbg_trace(usbssp_data, trace_usbssp_dbg_init, > > > + "Finished usbssp_init"); > > > > When your trace functions do nothing but say "entered a function", and > > "exited a function", why even have them? ftrace can provide that for you > > already, no need to overload that on the tracing framework, right? > > Do you suggest to use only: > trace_usbssp_dbg_init("Finished usbssp_init"); > instead: > usbssp_dbg(usbssp_data, "%pV\n", "Finished usbssp_init"); > trace_usbssp_dbg_init("Finished usbssp_init"); > ? > > I'm simple re-used the code from XHCI driver. It's really redundant, > but I don't know the intention of author 😊. Why are any of those lines needed? Doesn't ftrace work properly for you? And yeah, if xhci has this it should be removed from there as well. thanks, greg k-h