linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Helgaas <helgaas@kernel.org>
To: Jonathan Chocron <jonnyc@amazon.com>
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
Subject: Re: [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver
Date: Tue, 26 Mar 2019 07:55:08 -0500	[thread overview]
Message-ID: <20190326125508.GG24180@google.com> (raw)
In-Reply-To: <1553594455-30436-1-git-send-email-jonnyc@amazon.com>

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.

> 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.

> 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.

Bjorn

  parent reply	other threads:[~2019-03-26 12:55 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-03-25 11:07 [PATCH] PCI: al: add pcie-al.c jonnyc
2019-03-25 12:58 ` Bjorn Helgaas
2019-03-25 15:56   ` Jonathan Chocron
2019-03-25 17:36     ` Bjorn Helgaas
2019-03-26 10:00 ` [PATCH v2] PCI: al: Add Amazon Annapurna Labs PCIe host controller driver Jonathan Chocron
2019-03-26 12:17   ` Lorenzo Pieralisi
2019-03-26 13:24     ` David Woodhouse
2019-03-26 15:58       ` Lorenzo Pieralisi
2019-03-27  9:52         ` David Woodhouse
2019-03-27 11:20           ` Lorenzo Pieralisi
2019-03-27 11:40             ` David Woodhouse
2019-03-27  9:43     ` David Woodhouse
2019-03-27 11:39       ` Lorenzo Pieralisi
2019-03-27 12:01         ` David Woodhouse
2019-03-26 12:55   ` Bjorn Helgaas [this message]
2019-03-28 10:55     ` Jonathan Chocron
2019-03-28 11:57     ` [PATCH v3] " Jonathan Chocron
2019-04-08 22:57       ` Benjamin Herrenschmidt
2019-04-16 13:13         ` David Woodhouse
2019-04-16 14:01       ` Lorenzo Pieralisi
2019-04-25 14:08       ` Bjorn Helgaas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190326125508.GG24180@google.com \
    --to=helgaas@kernel.org \
    --cc=alisaidi@amazon.com \
    --cc=barakw@amazon.com \
    --cc=benh@kernel.crashing.org \
    --cc=dwmw@amazon.co.uk \
    --cc=jonnyc@amazon.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=lorenzo.pieralisi@arm.com \
    --cc=ronenk@amazon.com \
    --cc=vaerov@amazon.com \
    --cc=zeev@amazon.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).