linux-kernel - Re: 答复: [PATCH 1/2] PCI: dwc: kirin: add PCIe Driver for HiSilicon Kirin SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47c5d676-44ae-e08c-3002-802ca31ff6b7@axis.com>
Date:   Thu, 18 May 2017 10:56:31 +0200
From:   Niklas Cassel <niklas.cassel@...s.com>
To:     Guodong Xu <guodong.xu@...aro.org>,
        songxiaowei <songxiaowei@...ilicon.com>
CC:     Arnd Bergmann <arnd@...db.de>, Bjorn Helgaas <bhelgaas@...gle.com>,
        Kishon <kishon@...com>, Jingoo Han <jingoohan1@...il.com>,
        Tomasz Nowicki <tn@...ihalf.com>,
        Keith Busch <keith.busch@...el.com>, Duc Dang <dhdang@....com>,
        "liudongdong (C)" <liudongdong3@...wei.com>,
        "Chenfeng (puck)" <puck.chen@...ilicon.com>,
        Wangbinghui <wangbinghui@...ilicon.com>,
        Suzhuangluan <suzhuangluan@...ilicon.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-pci <linux-pci@...r.kernel.org>
Subject: Re: 答复: [PATCH 1/2] PCI: dwc: kirin: add PCIe Driver for HiSilicon Kirin SoC



On 05/18/2017 08:23 AM, Guodong Xu wrote:
> On Thu, May 18, 2017 at 9:50 AM, songxiaowei <songxiaowei@...ilicon.com> wrote:
>> Hi Arnd,
>>
>>
>> On Fri, May 12, 2017 at 3:51 AM, Song Xiaowei <songxiaowei@...ilicon.com> wrote:
>>> From: songxiaowei <songxiaowei@...ilicon.com>
>>
>> Looks good overall, just a few details:
>>
>> Please fix your ~/.gitconfig to contain the same real name ("Song Xiaowei"
>> instead of "songxiaowei") that you use for sending the emails.
>>
>> [Xiaowei Song]  I'll fix this issue and update the patch.
>>
>>> +
>>> +static int kirin_pcie_phy_init(struct kirin_pcie *kirin_pcie) {
>> ...
>>> +       reg_val = kirin_apb_phy_readl(kirin_pcie, 0x400);
>>> +       while (reg_val & pipe_clk_stable) {
>>> +               udelay(100);
>>> +               if (time == 0) {
>>> +                       dev_err(kirin_pcie->pci->dev, "PIPE clk is not stable\n");
>>> +                       return -EINVAL;
>>> +               }
>>> +               time--;
>>> +               reg_val = kirin_apb_phy_readl(kirin_pcie, 0x400);
>>> +       }
>>
>> If this is not called with interrupts disabled, please use a sleeping function (e.g. msleep(1)) as the delay and compare against ktime_before() to see how much total time has expired when waiting for a timeout, instead of using a counter.
>>
>> [Xiaowei Song] This issue was fixed in the patch sent at 2017-5-15, and I'll send it again.
> 
> Xiaowei,
> 
> Please make sure you add [PATCH v..] version information when you
> making future updates. Eg.
> 
> $ git format-patch --subject-prefix="PATCH v3"

I actually prefer

git format-patch -v 3

that way you don't accidentally get something
other than "PATCH".



Another very, very minor remark.
It's usually a good idea to have a comma after
each initializer, that way you don't have to
change multiple lines when adding/removing something.

+struct platform_driver kirin_pcie_driver = {
+	.probe			= kirin_pcie_probe,
+	.driver			= {
+		.name			= "Kirin-pcie",
+		.of_match_table = kirin_pcie_match,
+		.suppress_bind_attrs = true
+	},
+};


Best regards,
Niklas