Re: [PATCH] PCI: tegra: Update comment about config space
From: Pali Rohár
Date: Wed Oct 05 2022 - 15:43:48 EST
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@xxxxxxxxxx>
> > ---
> > 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).