Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host
 controller driver
To:     Bjorn Helgaas <helgaas@kernel.org>
CC:     <linux-pci@vger.kernel.org>, <lorenzo.pieralisi@arm.com>,
        <linux-kernel@vger.kernel.org>, <vaerov@amazon.com>,
        <dwmw@amazon.co.uk>, <benh@kernel.crashing.org>,
        <alisaidi@amazon.com>, <zeev@amazon.com>, <ronenk@amazon.com>,
        <barakw@amazon.com>
References: <1553512040-4453-1-git-send-email-jonnyc@amazon.com>
 <1553594455-30436-1-git-send-email-jonnyc@amazon.com>
 <20190326125508.GG24180@google.com>
From:   Jonathan Chocron <jonnyc@amazon.com>
Message-ID: <7a1c1611-bd88-de5e-c4ae-bc85113191b6@amazon.com>
Date:   Thu, 28 Mar 2019 12:55:49 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.1
MIME-Version: 1.0
In-Reply-To: <20190326125508.GG24180@google.com>
Content-Type: text/plain; charset="utf-8"; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 3/26/19 14:55, Bjorn Helgaas wrote:
> Nits, probably Lorenzo will fix them up unless he sees more substantive
> things.
>
> On Tue, Mar 26, 2019 at 12:00:55PM +0200, Jonathan Chocron wrote:
>> Adding support for Amazon's Annapurna Labs PCIe driver.
> Ideally, use "imperative mood", i.e., write it as a command:
>
>    Add support for Amazon's Annapurna Labs PCIe driver.
Done.
>> The HW controller is based on DesignWare's IP.
>>
>> The HW doesn't support accessing the Root Port's config space via
>> ECAM, so we obtain its base address via an AMZN0001 device.
>>
>> Furthermore, the DesignWare PCIe controller doesn't filter out
>> config transactions sent to devices 1 and up on its bus, so they
>> are filtered by the driver.
>> All subordinate buses do support ECAM access.
> I didn't communicate my point very clearly.  The above four lines
> should either be (1) a single paragraph, wrapped to fill the entire
> width, or (2) two paragraphs, with a blank line before "All
> subordinate buses ..."
>
> The fact that "... by the driver" ends in the middle of the line
> suggests that it's the end of the paragraph, but the fact that there's
> no blank line following suggests that it's not.  So it creates an
> unnecessary hiccup for the reader.
Added a blank line.
>> Implementing specific PCI config access functions involves:
>>   - Adding an init function to obtain the Root Port's base address
>>     from an AMZN0001 device.
>>   - Adding a new entry in the mcfg quirk array
> s/mcfg/MCFG/ since "MCFG" is an ACPI table ID, not a word.
Done.
> Bjorn