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: <20180712062714.GI20905@kroah.com> Date: Thu, 12 Jul 2018 08:27:14 +0200 To: Pawel Laszczak Cc: linux-usb@vger.kernel.org, Felipe Balbi , linux-kernel@vger.kernel.org, ltyrala@cadence.com, adouglas@cadence.com List-ID: T24gVGh1LCBKdWwgMTIsIDIwMTggYXQgMDY6NDc6MDJBTSArMDEwMCwgUGF3ZWwgTGFzemN6YWsg d3JvdGU6Cj4gKy8qIFVTQiAyLjAgaGFyZHdhcmUgTE1QIGNhcGFiaWxpdHkqLwo+ICsjZGVmaW5l IFVTQlNTUF9ITEMJCQkoMSA8PCAxOSkKPiArI2RlZmluZSBVU0JTU1BfQkxDCQkJKDEgPDwgMjAp CgpBZ2FpbiwgQklUKCkgcGxlYXNlLgoKPiAraW50IHVzYnNzcF9oYW5kc2hha2Uodm9pZCBfX2lv bWVtICpwdHIsIHUzMiBtYXNrLCB1MzIgZG9uZSwgaW50IHVzZWMpCj4gK3sKPiArCXUzMglyZXN1 bHQ7CgpTb21lIHBsYWNlcyB5b3UgdXNlIHRhYnMgZm9yIHRoZSB2YXJpYWJsZSBkZWNsYXJhdGlv bnMsIGFuZCBzb21lIHlvdSBkbwpub3QuICBQaWNrIGEgc2luZ2xlIHN0eWxlIGFuZCBzdGljayB0 byBpdCBwbGVhc2UuCgo+ICsKPiArCWRvIHsKPiArCQlyZXN1bHQgPSByZWFkbChwdHIpOwo+ICsJ CWlmIChyZXN1bHQgPT0gfih1MzIpMCkJLyogY2FyZCByZW1vdmVkICovCj4gKwkJCXJldHVybiAt RU5PREVWOwo+ICsJCXJlc3VsdCAmPSBtYXNrOwo+ICsJCWlmIChyZXN1bHQgPT0gZG9uZSkKPiAr CQkJcmV0dXJuIDA7Cj4gKwkJdWRlbGF5KDEpOwo+ICsJCXVzZWMtLTsKPiArCX0gd2hpbGUgKHVz ZWMgPiAwKTsKPiArCXJldHVybiAtRVRJTUVET1VUOwoKV2UgZG9uJ3QgaGF2ZSBhIGJ1aWx0LWlu IGtlcm5lbCBmdW5jdGlvbiB0byBkbyB0aGlzIHR5cGUgb2YgdGhpbmcKYWxyZWFkeT8gIFRoYXQn cyBzYWQuICBPaCB3ZWxsLi4uCgo+ICtpbnQgdXNic3NwX2luaXQoc3RydWN0IHVzYnNzcF91ZGMg KnVzYnNzcF9kYXRhKQo+ICt7Cj4gKwlpbnQgcmV0dmFsID0gMDsKPiArCj4gKwl1c2Jzc3BfZGJn X3RyYWNlKHVzYnNzcF9kYXRhLCB0cmFjZV91c2Jzc3BfZGJnX2luaXQsICJ1c2Jzc3BfaW5pdCIp Owo+ICsKPiArCXNwaW5fbG9ja19pbml0KCZ1c2Jzc3BfZGF0YS0+bG9jayk7Cj4gKwlzcGluX2xv Y2tfaW5pdCgmdXNic3NwX2RhdGEtPmlycV90aHJlYWRfbG9jayk7Cj4gKwo+ICsJLy9UT0RPOiBt ZW1vcnkgaW5pdGlhbGl6YXRpb24KPiArCS8vcmV0dmFsID0gdXNic3NwX21lbV9pbml0KHVzYnNz cF9kYXRhLCBHRlBfS0VSTkVMKTsKPiArCj4gKwl1c2Jzc3BfZGJnX3RyYWNlKHVzYnNzcF9kYXRh LCB0cmFjZV91c2Jzc3BfZGJnX2luaXQsCj4gKwkJCSJGaW5pc2hlZCB1c2Jzc3BfaW5pdCIpOwoK V2hlbiB5b3VyIHRyYWNlIGZ1bmN0aW9ucyBkbyBub3RoaW5nIGJ1dCBzYXkgImVudGVyZWQgYSBm dW5jdGlvbiIsIGFuZAoiZXhpdGVkIGEgZnVuY3Rpb24iLCB3aHkgZXZlbiBoYXZlIHRoZW0/ICBm dHJhY2UgY2FuIHByb3ZpZGUgdGhhdCBmb3IKeW91IGFscmVhZHksIG5vIG5lZWQgdG8gb3Zlcmxv YWQgdGhhdCBvbiB0aGUgdHJhY2luZyBmcmFtZXdvcmssIHJpZ2h0PwoKPiArLyoKPiArICogZ2Fk Z2V0LWlmLmMgZmlsZSBpcyBwYXJ0IG9mIGdhZGdldC5jIGZpbGUgYW5kIGltcGxlbWVudHMgaW50 ZXJmYWNlCj4gKyAqIGZvciBnYWRnZXQgZHJpdmVyCj4gKyAqLwo+ICsjaW5jbHVkZSAiZ2FkZ2V0 LWlmLmMiCgpVZ2gsIEkga25vdyBVU0IgaGNkIGRyaXZlcnMgbG92ZSB0byBpbmNsdWRlIC5jIGZp bGVzIGluIHRoZSBtaWRkbGUgb2YKdGhlbSwgYnV0IGRvIHdlIGhhdmUgdG8gY29udGludWUgdGhh dCBjcmF6eSBwcmFjdGljZSBpbiBuZXdlciBkcml2ZXJzPwpJcyBpdCByZWFsbHkgbmVjZXNzYXJ5 PwoKPiArCS8qCj4gKwkgKiBDaGVjayB0aGUgY29tcGlsZXIgZ2VuZXJhdGVkIHNpemVzIG9mIHN0 cnVjdHVyZXMgdGhhdCBtdXN0IGJlIGxhaWQKPiArCSAqIG91dCBpbiBzcGVjaWZpYyB3YXlzIGZv ciBoYXJkd2FyZSBhY2Nlc3MuCj4gKwkgKi8KPiArCUJVSUxEX0JVR19PTihzaXplb2Yoc3RydWN0 IHVzYnNzcF9kb29yYmVsbF9hcnJheSkgIT0gMiozMi84KTsKPiArCUJVSUxEX0JVR19PTihzaXpl b2Yoc3RydWN0IHVzYnNzcF9zbG90X2N0eCkgIT0gOCozMi84KTsKPiArCUJVSUxEX0JVR19PTihz aXplb2Yoc3RydWN0IHVzYnNzcF9lcF9jdHgpICE9IDgqMzIvOCk7Cj4gKwkvKiB1c2Jzc3BfZGV2 aWNlIGhhcyBlaWdodCBmaWVsZHMsIGFuZCBhbHNvCj4gKwkgKiBlbWJlZHMgb25lIHVzYnNzcF9z bG90X2N0eCBhbmQgMzEgdXNic3NwX2VwX2N0eAo+ICsJICovCj4gKwlCVUlMRF9CVUdfT04oc2l6 ZW9mKHN0cnVjdCB1c2Jzc3Bfc3RyZWFtX2N0eCkgIT0gNCozMi84KTsKPiArCUJVSUxEX0JVR19P TihzaXplb2YodW5pb24gdXNic3NwX3RyYikgIT0gNCozMi84KTsKPiArCUJVSUxEX0JVR19PTihz aXplb2Yoc3RydWN0IHVzYnNzcF9lcnN0X2VudHJ5KSAhPSA0KjMyLzgpOwo+ICsJQlVJTERfQlVH X09OKHNpemVvZihzdHJ1Y3QgdXNic3NwX2NhcF9yZWdzKSAhPSA4KjMyLzgpOwo+ICsJQlVJTERf QlVHX09OKHNpemVvZihzdHJ1Y3QgdXNic3NwX2ludHJfcmVnKSAhPSA4KjMyLzgpOwo+ICsJLyog dXNic3NwX3J1bl9yZWdzIGhhcyBlaWdodCBmaWVsZHMgYW5kIGVtYmVkcyAxMjggdXNic3NwX2lu dHJfcmVncyAqLwo+ICsJQlVJTERfQlVHX09OKHNpemVvZihzdHJ1Y3QgdXNic3NwX3J1bl9yZWdz KSAhPSAoOCs4KjEyOCkqMzIvOCk7CgpJIGxvdmUgaGFyZC1jb2RlZCBudW1iZXJzIGFzIG11Y2gg YXMgdGhlIG5leHQgcGVyc29uLCBidXQgcmVhbGx5PyAgSXMKdGhpcyBuZWNlc3Nhcnkgbm93IHRo YXQgeW91IGhhdmUgdGhlIGNvZGUgdXAgYW5kIHdvcmtpbmcgcHJvcGVybHk/CgoKSSd2ZSBzdG9w cGVkIHJldmlld2luZyB0aGlzIHNlcmllcyBoZXJlLgoKdGhhbmtzLAoKZ3JlZyBrLWgKLS0tClRv IHVuc3Vic2NyaWJlIGZyb20gdGhpcyBsaXN0OiBzZW5kIHRoZSBsaW5lICJ1bnN1YnNjcmliZSBs aW51eC11c2IiIGluCnRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW9Admdlci5rZXJu ZWwub3JnCk1vcmUgbWFqb3Jkb21vIGluZm8gYXQgIGh0dHA6Ly92Z2VyLmtlcm5lbC5vcmcvbWFq b3Jkb21vLWluZm8uaHRtbAo= 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 A2AFDC43A1D for ; Thu, 12 Jul 2018 06:27:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 6130E20BF2 for ; Thu, 12 Jul 2018 06:27:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6130E20BF2 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 S1727092AbeGLGfX (ORCPT ); Thu, 12 Jul 2018 02:35:23 -0400 Received: from mail.linuxfoundation.org ([140.211.169.12]:37298 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726693AbeGLGfX (ORCPT ); Thu, 12 Jul 2018 02:35:23 -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 0A7FBC75; Thu, 12 Jul 2018 06:27:16 +0000 (UTC) Date: Thu, 12 Jul 2018 08:27:14 +0200 From: Greg Kroah-Hartman To: Pawel Laszczak Cc: linux-usb@vger.kernel.org, Felipe Balbi , linux-kernel@vger.kernel.org, ltyrala@cadence.com, adouglas@cadence.com Subject: Re: [PATCH 05/31] usb: usbssp: Added first part of initialization sequence. Message-ID: <20180712062714.GI20905@kroah.com> References: <1531374448-26532-1-git-send-email-pawell@cadence.com> <1531374448-26532-6-git-send-email-pawell@cadence.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1531374448-26532-6-git-send-email-pawell@cadence.com> 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 06:47:02AM +0100, 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? > +/* > + * gadget-if.c file is part of gadget.c file and implements interface > + * for gadget driver > + */ > +#include "gadget-if.c" Ugh, I know USB hcd drivers love to include .c files in the middle of them, but do we have to continue that crazy practice in newer drivers? Is it really necessary? > + /* > + * Check the compiler generated sizes of structures that must be laid > + * out in specific ways for hardware access. > + */ > + BUILD_BUG_ON(sizeof(struct usbssp_doorbell_array) != 2*32/8); > + BUILD_BUG_ON(sizeof(struct usbssp_slot_ctx) != 8*32/8); > + BUILD_BUG_ON(sizeof(struct usbssp_ep_ctx) != 8*32/8); > + /* usbssp_device has eight fields, and also > + * embeds one usbssp_slot_ctx and 31 usbssp_ep_ctx > + */ > + BUILD_BUG_ON(sizeof(struct usbssp_stream_ctx) != 4*32/8); > + BUILD_BUG_ON(sizeof(union usbssp_trb) != 4*32/8); > + BUILD_BUG_ON(sizeof(struct usbssp_erst_entry) != 4*32/8); > + BUILD_BUG_ON(sizeof(struct usbssp_cap_regs) != 8*32/8); > + BUILD_BUG_ON(sizeof(struct usbssp_intr_reg) != 8*32/8); > + /* usbssp_run_regs has eight fields and embeds 128 usbssp_intr_regs */ > + BUILD_BUG_ON(sizeof(struct usbssp_run_regs) != (8+8*128)*32/8); I love hard-coded numbers as much as the next person, but really? Is this necessary now that you have the code up and working properly? I've stopped reviewing this series here. thanks, greg k-h