Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 5 Oct 2022 21:43:36 +0200
From:   Pali =?utf-8?B?Um9ow6Fy?= <pali@kernel.org>
To:     Thierry Reding <thierry.reding@gmail.com>
Cc:     Lorenzo Pieralisi <lpieralisi@kernel.org>,
        Rob Herring <robh@kernel.org>,
        Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= <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
Message-ID: <20221005194336.gdnu4vfzkudedjw5@pali>
References: <20220911113216.14892-1-pali@kernel.org>
 <YzRcYwQYlawV10QS@orome>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YzRcYwQYlawV10QS@orome>
User-Agent: NeoMutt/20180716
Precedence: bulk

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