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=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS,URIBL_BLOCKED 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 019D1C04EB9 for ; Mon, 3 Dec 2018 08:30:46 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C2B2420851 for ; Mon, 3 Dec 2018 08:30:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C2B2420851 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=huawei.com 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 S1725888AbeLCIas (ORCPT ); Mon, 3 Dec 2018 03:30:48 -0500 Received: from szxga06-in.huawei.com ([45.249.212.32]:50423 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725838AbeLCIar (ORCPT ); Mon, 3 Dec 2018 03:30:47 -0500 Received: from DGGEMS413-HUB.china.huawei.com (unknown [172.30.72.58]) by Forcepoint Email with ESMTP id DF67723CD3B2F; Mon, 3 Dec 2018 16:30:41 +0800 (CST) Received: from [127.0.0.1] (10.142.63.192) by DGGEMS413-HUB.china.huawei.com (10.3.19.213) with Microsoft SMTP Server id 14.3.408.0; Mon, 3 Dec 2018 16:30:33 +0800 CC: , USB , devicetree , Linux Kernel Mailing List , Suzhuangluan , Kongfei , Felipe Balbi , "Greg Kroah-Hartman" , "David S. Miller" , Mauro Carvalho Chehab , Andrew Morton , Arnd Bergmann , John Stultz Subject: Re: [PATCH v1 04/12] usb: dwc3: dwc3-hisi: Add code for dwc3 of Hisilicon Soc Platform To: Andy Shevchenko References: <20181203034515.91412-1-chenyu56@huawei.com> <20181203034515.91412-5-chenyu56@huawei.com> From: Chen Yu Message-ID: <2b46d6fc-a132-40fa-6f15-78e84a4d3b01@huawei.com> Date: Mon, 3 Dec 2018 16:30:33 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.142.63.192] X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On 2018/12/3 16:12, Andy Shevchenko wrote: > On Mon, Dec 3, 2018 at 5:48 AM Yu Chen wrote: >> >> This driver handles the poweron and shutdown of dwc3 core >> on Hisilicon Soc Platform. > >> +config USB_DWC3_HISI >> + tristate "HiSilicon Kirin Platforms" >> + depends on ((ARCH_HISI && ARM64) || COMPILE_TEST) && OF > > It would be easy to read if > depends on OF > would be on a separate line > >> + default USB_DWC3 >> + help >> + Support USB2/3 functionality in HiSilicon Kirin platforms. >> + Say 'Y' or 'M' if you have one such device. > >> +++ b/drivers/usb/dwc3/dwc3-hisi.c >> @@ -0,0 +1,335 @@ >> +// SPDX-License-Identifier: GPL-2.0+ > > You need to talk to your manager and fix License correspondingly. > Currently there is an ambigous reading. > >> +/** > > Why /** ? > >> + * dwc3-hisi.c - Support for dwc3 platform devices on HiSilicon platforms > > Usually put filename in the file is not the best idea. > >> + * >> + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd. >> + * http://www.huawei.com >> + * >> + * Authors: Yu Chen > >> + * >> + * This program is free software: you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 of >> + * the License as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. > > The idea of SPDX is exactly to remove the above boilerplate text. > >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > > It would be easy to maintain ordered list in the future. > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > >> +static int dwc3_hisi_probe(struct platform_device *pdev) >> +{ >> + struct dwc3_hisi *dwc3_hisi; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; > >> + > > Redundant blank line > >> + int ret; >> + int i; > >> +} > >> +#ifdef CONFIG_PM_SLEEP >> +static int dwc3_hisi_suspend(struct device *dev) > > Don't know the usual practice here, but you can just use > __maybe_unused and remove this #ifdef. > >> +{ >> + struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev); >> + int ret; >> + > >> + dev_info(dev, "%s\n", __func__); > > Noise > >> +} > >> +static int dwc3_hisi_resume(struct device *dev) > > __maybe_unused ? > >> +{ > >> + dev_info(dev, "%s\n", __func__); > > Noise. > >> + /* Wait for clock stable */ >> + msleep(100); > > Don't you have any hardware notification that clocks are stable? > (Something like PLL locked?) > >> + >> + return 0; >> +} >> +#endif /* CONFIG_PM_SLEEP */ > >> +static const struct of_device_id dwc3_hisi_match[] = { >> + { .compatible = "hisilicon,hi3660-dwc3" }, >> + { /* sentinel */ }, > > No need comma for terminator line. > >> +}; > Thanks a lot for your advice! I will check and fix the patch. Chen Yu From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chen Yu Subject: Re: [PATCH v1 04/12] usb: dwc3: dwc3-hisi: Add code for dwc3 of Hisilicon Soc Platform Date: Mon, 3 Dec 2018 16:30:33 +0800 Message-ID: <2b46d6fc-a132-40fa-6f15-78e84a4d3b01@huawei.com> References: <20181203034515.91412-1-chenyu56@huawei.com> <20181203034515.91412-5-chenyu56@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US Sender: linux-kernel-owner@vger.kernel.org To: Andy Shevchenko Cc: wangbinghui@hisilicon.com, USB , devicetree , Linux Kernel Mailing List , Suzhuangluan , Kongfei , Felipe Balbi , Greg Kroah-Hartman , "David S. Miller" , Mauro Carvalho Chehab , Andrew Morton , Arnd Bergmann , John Stultz List-Id: devicetree@vger.kernel.org Hi, On 2018/12/3 16:12, Andy Shevchenko wrote: > On Mon, Dec 3, 2018 at 5:48 AM Yu Chen wrote: >> >> This driver handles the poweron and shutdown of dwc3 core >> on Hisilicon Soc Platform. > >> +config USB_DWC3_HISI >> + tristate "HiSilicon Kirin Platforms" >> + depends on ((ARCH_HISI && ARM64) || COMPILE_TEST) && OF > > It would be easy to read if > depends on OF > would be on a separate line > >> + default USB_DWC3 >> + help >> + Support USB2/3 functionality in HiSilicon Kirin platforms. >> + Say 'Y' or 'M' if you have one such device. > >> +++ b/drivers/usb/dwc3/dwc3-hisi.c >> @@ -0,0 +1,335 @@ >> +// SPDX-License-Identifier: GPL-2.0+ > > You need to talk to your manager and fix License correspondingly. > Currently there is an ambigous reading. > >> +/** > > Why /** ? > >> + * dwc3-hisi.c - Support for dwc3 platform devices on HiSilicon platforms > > Usually put filename in the file is not the best idea. > >> + * >> + * Copyright (C) 2017-2018 Hilisicon Electronics Co., Ltd. >> + * http://www.huawei.com >> + * >> + * Authors: Yu Chen > >> + * >> + * This program is free software: you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 of >> + * the License as published by the Free Software Foundation. >> + * >> + * This program is distributed in the hope that it will be useful, >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the >> + * GNU General Public License for more details. > > The idea of SPDX is exactly to remove the above boilerplate text. > >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include > > It would be easy to maintain ordered list in the future. > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > > while (i--) > >> + while (--i >= 0) > >> +static int dwc3_hisi_probe(struct platform_device *pdev) >> +{ >> + struct dwc3_hisi *dwc3_hisi; >> + struct device *dev = &pdev->dev; >> + struct device_node *np = dev->of_node; > >> + > > Redundant blank line > >> + int ret; >> + int i; > >> +} > >> +#ifdef CONFIG_PM_SLEEP >> +static int dwc3_hisi_suspend(struct device *dev) > > Don't know the usual practice here, but you can just use > __maybe_unused and remove this #ifdef. > >> +{ >> + struct dwc3_hisi *dwc3_hisi = dev_get_drvdata(dev); >> + int ret; >> + > >> + dev_info(dev, "%s\n", __func__); > > Noise > >> +} > >> +static int dwc3_hisi_resume(struct device *dev) > > __maybe_unused ? > >> +{ > >> + dev_info(dev, "%s\n", __func__); > > Noise. > >> + /* Wait for clock stable */ >> + msleep(100); > > Don't you have any hardware notification that clocks are stable? > (Something like PLL locked?) > >> + >> + return 0; >> +} >> +#endif /* CONFIG_PM_SLEEP */ > >> +static const struct of_device_id dwc3_hisi_match[] = { >> + { .compatible = "hisilicon,hi3660-dwc3" }, >> + { /* sentinel */ }, > > No need comma for terminator line. > >> +}; > Thanks a lot for your advice! I will check and fix the patch. Chen Yu 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: [v1,04/12] usb: dwc3: dwc3-hisi: Add code for dwc3 of Hisilicon Soc Platform From: Yu Chen Message-Id: <2b46d6fc-a132-40fa-6f15-78e84a4d3b01@huawei.com> Date: Mon, 3 Dec 2018 16:30:33 +0800 To: Andy Shevchenko Cc: wangbinghui@hisilicon.com, USB , devicetree , Linux Kernel Mailing List , Suzhuangluan , Kongfei , Felipe Balbi , Greg Kroah-Hartman , "David S. Miller" , Mauro Carvalho Chehab , Andrew Morton , Arnd Bergmann , John Stultz List-ID: SGksCgpPbiAyMDE4LzEyLzMgMTY6MTIsIEFuZHkgU2hldmNoZW5rbyB3cm90ZToKPiBPbiBNb24s IERlYyAzLCAyMDE4IGF0IDU6NDggQU0gWXUgQ2hlbiA8Y2hlbnl1NTZAaHVhd2VpLmNvbT4gd3Jv dGU6Cj4+Cj4+IFRoaXMgZHJpdmVyIGhhbmRsZXMgdGhlIHBvd2Vyb24gYW5kIHNodXRkb3duIG9m IGR3YzMgY29yZQo+PiBvbiBIaXNpbGljb24gU29jIFBsYXRmb3JtLgo+IAo+PiArY29uZmlnIFVT Ql9EV0MzX0hJU0kKPj4gKyAgICAgICB0cmlzdGF0ZSAiSGlTaWxpY29uIEtpcmluIFBsYXRmb3Jt cyIKPj4gKyAgICAgICBkZXBlbmRzIG9uICgoQVJDSF9ISVNJICYmIEFSTTY0KSB8fCBDT01QSUxF X1RFU1QpICYmIE9GCj4gCj4gSXQgd291bGQgYmUgZWFzeSB0byByZWFkIGlmCj4gZGVwZW5kcyBv biBPRgo+IHdvdWxkIGJlIG9uIGEgc2VwYXJhdGUgbGluZQo+IAo+PiArICAgICAgIGRlZmF1bHQg VVNCX0RXQzMKPj4gKyAgICAgICBoZWxwCj4+ICsgICAgICAgICBTdXBwb3J0IFVTQjIvMyBmdW5j dGlvbmFsaXR5IGluIEhpU2lsaWNvbiBLaXJpbiBwbGF0Zm9ybXMuCj4+ICsgICAgICAgICBTYXkg J1knIG9yICdNJyBpZiB5b3UgaGF2ZSBvbmUgc3VjaCBkZXZpY2UuCj4gCj4+ICsrKyBiL2RyaXZl cnMvdXNiL2R3YzMvZHdjMy1oaXNpLmMKPj4gQEAgLTAsMCArMSwzMzUgQEAKPj4gKy8vIFNQRFgt TGljZW5zZS1JZGVudGlmaWVyOiBHUEwtMi4wKwo+IAo+IFlvdSBuZWVkIHRvIHRhbGsgdG8geW91 ciBtYW5hZ2VyIGFuZCBmaXggTGljZW5zZSBjb3JyZXNwb25kaW5nbHkuCj4gQ3VycmVudGx5IHRo ZXJlIGlzIGFuIGFtYmlnb3VzIHJlYWRpbmcuCj4gCj4+ICsvKioKPiAKPiBXaHkgLyoqID8KPiAK Pj4gKyAqIGR3YzMtaGlzaS5jIC0gU3VwcG9ydCBmb3IgZHdjMyBwbGF0Zm9ybSBkZXZpY2VzIG9u IEhpU2lsaWNvbiBwbGF0Zm9ybXMKPiAKPiBVc3VhbGx5IHB1dCBmaWxlbmFtZSBpbiB0aGUgZmls ZSBpcyBub3QgdGhlIGJlc3QgaWRlYS4KPiAKPj4gKyAqCj4+ICsgKiBDb3B5cmlnaHQgKEMpIDIw MTctMjAxOCBIaWxpc2ljb24gRWxlY3Ryb25pY3MgQ28uLCBMdGQuCj4+ICsgKiAgICAgICAgICAg ICBodHRwOi8vd3d3Lmh1YXdlaS5jb20KPj4gKyAqCj4+ICsgKiBBdXRob3JzOiBZdSBDaGVuIDxj aGVueXU1NkBodWF3ZWkuY29tPgo+IAo+PiArICoKPj4gKyAqIFRoaXMgcHJvZ3JhbSBpcyBmcmVl IHNvZnR3YXJlOiB5b3UgY2FuIHJlZGlzdHJpYnV0ZSBpdCBhbmQvb3IgbW9kaWZ5Cj4+ICsgKiBp dCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBHZW5lcmFsIFB1YmxpYyBMaWNlbnNlIHZlcnNp b24gMiAgb2YKPj4gKyAqIHRoZSBMaWNlbnNlIGFzIHB1Ymxpc2hlZCBieSB0aGUgRnJlZSBTb2Z0 d2FyZSBGb3VuZGF0aW9uLgo+PiArICoKPj4gKyAqIFRoaXMgcHJvZ3JhbSBpcyBkaXN0cmlidXRl ZCBpbiB0aGUgaG9wZSB0aGF0IGl0IHdpbGwgYmUgdXNlZnVsLAo+PiArICogYnV0IFdJVEhPVVQg QU5ZIFdBUlJBTlRZOyB3aXRob3V0IGV2ZW4gdGhlIGltcGxpZWQgd2FycmFudHkgb2YKPj4gKyAq IE1FUkNIQU5UQUJJTElUWSBvciBGSVRORVNTIEZPUiBBIFBBUlRJQ1VMQVIgUFVSUE9TRS4gIFNl ZSB0aGUKPj4gKyAqIEdOVSBHZW5lcmFsIFB1YmxpYyBMaWNlbnNlIGZvciBtb3JlIGRldGFpbHMu Cj4gCj4gVGhlIGlkZWEgb2YgU1BEWCBpcyBleGFjdGx5IHRvIHJlbW92ZSB0aGUgYWJvdmUgYm9p bGVycGxhdGUgdGV4dC4KPiAKPj4gKyAqLwo+PiArCj4+ICsjaW5jbHVkZSA8bGludXgvbW9kdWxl Lmg+Cj4+ICsjaW5jbHVkZSA8bGludXgva2VybmVsLmg+Cj4+ICsjaW5jbHVkZSA8bGludXgvc2xh Yi5oPgo+PiArI2luY2x1ZGUgPGxpbnV4L29mLmg+Cj4+ICsjaW5jbHVkZSA8bGludXgvb2ZfcGxh dGZvcm0uaD4KPj4gKyNpbmNsdWRlIDxsaW51eC9wbV9ydW50aW1lLmg+Cj4+ICsjaW5jbHVkZSA8 bGludXgvY2xrLmg+Cj4+ICsjaW5jbHVkZSA8bGludXgvY2xrLXByb3ZpZGVyLmg+Cj4+ICsjaW5j bHVkZSA8bGludXgvcmVnbWFwLmg+Cj4+ICsjaW5jbHVkZSA8bGludXgvcmVzZXQuaD4KPj4gKyNp bmNsdWRlIDxsaW51eC9leHRjb24uaD4KPj4gKyNpbmNsdWRlIDxsaW51eC9yZWd1bGF0b3IvY29u c3VtZXIuaD4KPj4gKyNpbmNsdWRlIDxsaW51eC9waW5jdHJsL2NvbnN1bWVyLmg+Cj4gCj4gSXQg d291bGQgYmUgZWFzeSB0byBtYWludGFpbiBvcmRlcmVkIGxpc3QgaW4gdGhlIGZ1dHVyZS4KPiAK Pj4gKyAgICAgICAgICAgICAgICAgICAgICAgd2hpbGUgKC0taSA+PSAwKQo+IAo+IHdoaWxlIChp LS0pCj4gCj4+ICsgICAgICAgICAgICAgICAgICAgICAgIHdoaWxlICgtLWkgPj0gMCkKPiAKPiB3 aGlsZSAoaS0tKQo+IAo+PiArICAgICAgICAgICAgICAgICAgICAgICB3aGlsZSAoLS1pID49IDAp Cj4gCj4gd2hpbGUgKGktLSkKPiAKPj4gKyAgICAgICAgICAgICAgICAgICAgICAgd2hpbGUgKC0t aSA+PSAwKQo+IAo+IHdoaWxlIChpLS0pCj4gCj4+ICsgICAgICAgICAgICAgICAgICAgICAgIHdo aWxlICgtLWkgPj0gMCkKPiAKPj4gK3N0YXRpYyBpbnQgZHdjM19oaXNpX3Byb2JlKHN0cnVjdCBw bGF0Zm9ybV9kZXZpY2UgKnBkZXYpCj4+ICt7Cj4+ICsgICAgICAgc3RydWN0IGR3YzNfaGlzaSAg ICAgICAgKmR3YzNfaGlzaTsKPj4gKyAgICAgICBzdHJ1Y3QgZGV2aWNlICAgICAgICAgICAqZGV2 ID0gJnBkZXYtPmRldjsKPj4gKyAgICAgICBzdHJ1Y3QgZGV2aWNlX25vZGUgICAgICAqbnAgPSBk ZXYtPm9mX25vZGU7Cj4gCj4+ICsKPiAKPiBSZWR1bmRhbnQgYmxhbmsgbGluZQo+IAo+PiArICAg ICAgIGludCAgICAgICAgICAgICAgICAgICAgIHJldDsKPj4gKyAgICAgICBpbnQgICAgICAgICAg ICAgICAgICAgICBpOwo+IAo+PiArfQo+IAo+PiArI2lmZGVmIENPTkZJR19QTV9TTEVFUAo+PiAr c3RhdGljIGludCBkd2MzX2hpc2lfc3VzcGVuZChzdHJ1Y3QgZGV2aWNlICpkZXYpCj4gCj4gRG9u J3Qga25vdyB0aGUgdXN1YWwgcHJhY3RpY2UgaGVyZSwgYnV0IHlvdSBjYW4ganVzdCB1c2UKPiBf X21heWJlX3VudXNlZCBhbmQgcmVtb3ZlIHRoaXMgI2lmZGVmLgo+IAo+PiArewo+PiArICAgICAg IHN0cnVjdCBkd2MzX2hpc2kgKmR3YzNfaGlzaSA9IGRldl9nZXRfZHJ2ZGF0YShkZXYpOwo+PiAr ICAgICAgIGludCByZXQ7Cj4+ICsKPiAKPj4gKyAgICAgICBkZXZfaW5mbyhkZXYsICIlc1xuIiwg X19mdW5jX18pOwo+IAo+IE5vaXNlCj4gCj4+ICt9Cj4gCj4+ICtzdGF0aWMgaW50IGR3YzNfaGlz aV9yZXN1bWUoc3RydWN0IGRldmljZSAqZGV2KQo+IAo+IF9fbWF5YmVfdW51c2VkID8KPiAKPj4g K3sKPiAKPj4gKyAgICAgICBkZXZfaW5mbyhkZXYsICIlc1xuIiwgX19mdW5jX18pOwo+IAo+IE5v aXNlLgo+IAo+PiArICAgICAgIC8qIFdhaXQgZm9yIGNsb2NrIHN0YWJsZSAqLwo+PiArICAgICAg IG1zbGVlcCgxMDApOwo+IAo+IERvbid0IHlvdSBoYXZlIGFueSBoYXJkd2FyZSBub3RpZmljYXRp b24gdGhhdCBjbG9ja3MgYXJlIHN0YWJsZT8KPiAoU29tZXRoaW5nIGxpa2UgUExMIGxvY2tlZD8p Cj4gCj4+ICsKPj4gKyAgICAgICByZXR1cm4gMDsKPj4gK30KPj4gKyNlbmRpZiAvKiBDT05GSUdf UE1fU0xFRVAgKi8KPiAKPj4gK3N0YXRpYyBjb25zdCBzdHJ1Y3Qgb2ZfZGV2aWNlX2lkIGR3YzNf aGlzaV9tYXRjaFtdID0gewo+PiArICAgICAgIHsgLmNvbXBhdGlibGUgPSAiaGlzaWxpY29uLGhp MzY2MC1kd2MzIiB9LAo+PiArICAgICAgIHsgLyogc2VudGluZWwgKi8gfSwKPiAKPiBObyBuZWVk IGNvbW1hIGZvciB0ZXJtaW5hdG9yIGxpbmUuCj4gCj4+ICt9Owo+IAoKVGhhbmtzIGEgbG90IGZv ciB5b3VyIGFkdmljZSEgSSB3aWxsIGNoZWNrIGFuZCBmaXggdGhlIHBhdGNoLgoKQ2hlbiBZdQo=