MIME-Version: 1.0
References: <20211207104924.21327-1-sergio.paracuellos@gmail.com> <20220112215203.GA286653@bhelgaas>
In-Reply-To: <20220112215203.GA286653@bhelgaas>
From:   Sergio Paracuellos <sergio.paracuellos@gmail.com>
Date:   Thu, 13 Jan 2022 06:52:41 +0100
Message-ID: <CAMhs-H-D_58dbYX1jetMMVEf4p+sSP0CV5GEEC7bBLs8Eo7wpQ@mail.gmail.com>
Subject: Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
To:     Bjorn Helgaas <helgaas@kernel.org>
Cc:     linux-pci <linux-pci@vger.kernel.org>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Guenter Roeck <linux@roeck-us.net>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Bjorn,

On Wed, Jan 12, 2022 at 10:52 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Tue, Dec 07, 2021 at 11:49:19AM +0100, Sergio Paracuellos wrote:
> > Hi all,
> >
> > MIPS specific code can be removed from driver and put into ralink mt762=
1
> > instead which is a more accurate place to do this. To make this possibl=
e
> > we need to have access to 'bridge->windows' in 'pcibios_root_bridge_pre=
pare()'
> > which has been implemented for ralink mt7621 platform (there is no real
> > need to implement this for any other platforms since those ones haven't=
 got
> > I/O coherency units). This also allow us to properly enable this driver=
 to
> > completely be enabled for COMPILE_TEST. This patchset appoarch:
> > - Move windows list splice in 'pci_register_host_bridge()' after functi=
on
> >   'pcibios_root_bridge_prepare()' is called.
> > - Implement 'pcibios_root_bridge_prepare()' for ralink mt7621.
> > - Avoid custom MIPs code in pcie-mt7621 driver.
> > - Add missing 'MODULE_LICENSE()' to pcie-mt7621 driver to avoid compile=
 test
> >   module compilation to complain (already sent patch from Yanteng Si th=
at
> >   I have rewrite commit message and long description a bit.
> > - Remove MIPS conditional code from Kconfig and mark driver as 'tristat=
e'.
> >
> > This patchset is a real fix for some errors reported by Kernel Test Rob=
ot about
> > implicit mips functions used in driver code and fix errors in driver wh=
en
> > is compiled as a module [1] (mips:allmodconfig).
> >
> > Changes in v3:
> >  - Rebase the series on the top of the temporal fix sent for v5.16[3] f=
or
> >    the module compilation problem.
> >  - Address review comments from Guenter in PATCH 2 (thanks Guenter!):
> >     - Address TODO in comment about the hardware does not allow zeros
> >       after 1s for the mask and WARN_ON if that's happend.
> >     - Be sure mask is real valid upper 16 bits.
> >
> > Changes in v2:
> >  - Collect Acked-by from Arnd Bergmann for PATCH 1.
> >  - Collect Reviewed-by from Krzysztof Wilczy=C5=84ski for PATCH 4.
> >  - Adjust some patches commit subject and message as pointed out by Bjo=
rn in review of v1 of the series[2].
> >
> > This patchset is the good way of properly compile driver as a module re=
moving
> > all MIPS specific code into arch ralink mt7621 place. To avoid mips:all=
modconfig reported
> > problems for v5.16 the following patch has been sent[3]. This series ar=
e rebased onto this patch to provide
> > a real fix for this problem.
> >
> > [0]: https://lore.kernel.org/linux-mips/CAMhs-H8ShoaYiFOOzJaGC68nZz=3DV=
365RXN_Kjuj=3DfPFENGJiiw@mail.gmail.com/T/#t
> > [1]: https://lkml.org/lkml/2021/11/14/436
> > [2]: https://lore.kernel.org/r/20211115070809.15529-1-sergio.paracuello=
s@gmail.com
> > [3]: https://lore.kernel.org/linux-pci/20211203192454.32624-1-sergio.pa=
racuellos@gmail.com/T/#u
> >
> > Thanks in advance for your time.
> >
> > Best regards,
> >    Sergio Paracuellos
> >
> > Sergio Paracuellos (5):
> >   PCI: Let pcibios_root_bridge_prepare() access to 'bridge->windows'
> >   MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
> >   PCI: mt7621: Avoid custom MIPS code in driver code
> >   PCI: mt7621: Add missing 'MODULE_LICENSE()' definition
> >   PCI: mt7621: Allow COMPILE_TEST for all arches
> >
> >  arch/mips/ralink/mt7621.c            | 31 ++++++++++++++++++++++
> >  drivers/pci/controller/Kconfig       |  4 +--
> >  drivers/pci/controller/pcie-mt7621.c | 39 ++--------------------------
> >  drivers/pci/probe.c                  |  4 +--
> >  4 files changed, 37 insertions(+), 41 deletions(-)
>
> I tentatively put this on my pci/host/mt7621 branch.  The only
> non-mt7621 change is the pci_register_host_bridge() change, which
> seems innocuous, so maybe we can still squeeze it in.
>
> I squashed these patches together:
>
>   MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
>   PCI: mt7621: Avoid custom MIPS code in driver code
>
> because the first adds the coherency setup to the MIPS
> pcibios_root_bridge_prepare(), and the second removes that same code
> from pcie-mt7621.c.  I think it makes more sense to do it as a move in
> a single patch, both for ease of reviewing and for potential
> bisection.

Makes sense. Thanks for letting me know.

Best regards,
    Sergio Paracuellos