Linux Tegra architecture development
 help / color / mirror / Atom feed
From: "Pali Rohár" <pali@kernel.org>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: "Lorenzo Pieralisi" <lpieralisi@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Wilczyński" <kw@linux.com>,
	"Bjorn Helgaas" <bhelgaas@google.com>,
	"Jonathan Hunter" <jonathanh@nvidia.com>,
	linux-tegra@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI: tegra: Update comment about config space
Date: Wed, 5 Oct 2022 21:43:36 +0200	[thread overview]
Message-ID: <20221005194336.gdnu4vfzkudedjw5@pali> (raw)
In-Reply-To: <YzRcYwQYlawV10QS@orome>

On Wednesday 28 September 2022 16:38:27 Thierry Reding wrote:
> On Sun, Sep 11, 2022 at 01:32:16PM +0200, Pali Rohár wrote:
> > Like many other ARM PCIe controllers, it uses old PCI Configuration
> > Mechanism #1 from PCI Local Bus for accessing PCI config space.
> > It is not PCIe ECAM in any case.
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  drivers/pci/controller/pci-tegra.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> Perhaps this should be rolled into the PCI_CONF1_EXT_ADDRESS patch?

Well, I split documentation change and PCI_CONF1_EXT_ADDRESS usage into
two patches as those are two different / separate things. Documentation
change is a fix (because documentation is wrong) and PCI_CONF1_EXT_ADDRESS
is an improvement - code cleanup. And in case if there is a issue with
"cleanup" patch it can be reverted without need to revert also "fix"
part. This is just information how I looked at these changes and why I
decided to split them.

> On
> the other hand there's really no use in keeping this comment around
> after that other patch because the documentation for the new macro lays
> out the details already.
> 
> Thierry

Ok, whether documentation is needed or not - it is your maintainer
decision. Maybe really obvious things do not have to be documented.
Also another look at this problem can be that if somebody wrote wrong
documentation for it, maybe it is not too obvious? I do not have opinion
on this, so choose what is better :-)

In any case, wrong documentation (which is the current state) should be
fixed (and removal in most case is also proper fix).

  reply	other threads:[~2022-10-05 19:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-11 11:32 [PATCH] PCI: tegra: Update comment about config space Pali Rohár
2022-09-28 14:38 ` Thierry Reding
2022-10-05 19:43   ` Pali Rohár [this message]
2022-10-06 12:31     ` Lorenzo Pieralisi
2022-10-06 12:50     ` Thierry Reding
2022-11-01 23:29       ` Pali Rohár

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=20221005194336.gdnu4vfzkudedjw5@pali \
    --to=pali@kernel.org \
    --cc=bhelgaas@google.com \
    --cc=jonathanh@nvidia.com \
    --cc=kw@linux.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=lpieralisi@kernel.org \
    --cc=robh@kernel.org \
    --cc=thierry.reding@gmail.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