linux-kernel - Re: [PATCH] PCI: tegra: Update comment about config space

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: <20221005194336.gdnu4vfzkudedjw5@pali>
Date:   Wed, 5 Oct 2022 21:43:36 +0200
From:   Pali Rohár <pali@...nel.org>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Lorenzo Pieralisi <lpieralisi@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Jonathan Hunter <jonathanh@...dia.com>,
        linux-tegra@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: tegra: Update comment about config space

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