Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/12/22 10:06 AM, Lorenzo Pieralisi wrote:
On Wed, Jan 12, 2022 at 03:42:56PM +0100, Sergio Paracuellos wrote:
Hi Bjorn, Lorenzo,

On Tue, Dec 7, 2021 at 11:49 AM Sergio Paracuellos
<sergio.paracuellos@xxxxxxxxx> wrote:

Hi all,

MIPS specific code can be removed from driver and put into ralink mt7621
instead which is a more accurate place to do this. To make this possible
we need to have access to 'bridge->windows' in 'pcibios_root_bridge_prepare()'
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 function
   '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 that
   I have rewrite commit message and long description a bit.
- Remove MIPS conditional code from Kconfig and mark driver as 'tristate'.

This patchset is a real fix for some errors reported by Kernel Test Robot about
implicit mips functions used in driver code and fix errors in driver when
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] for
    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.

What are your plans for this series? Can we merge this?

I was waiting for an ACK on patch (2) since it affects MIPS code.


Presumably not from me, but since some of the code is the result
of my suggestion I just sent a Reviewed-by: tag for patch 2.

Guenter

It would also be great if Bjorn reviewed it to make sure he agrees
with the approach.

I think it is too late for this cycle, apologies, there is a significant
review backlog.

Lorenzo

Best regards,
     Sergio Paracuellos


Changes in v2:
  - Collect Acked-by from Arnd Bergmann for PATCH 1.
  - Collect Reviewed-by from Krzysztof Wilczyński for PATCH 4.
  - Adjust some patches commit subject and message as pointed out by Bjorn in review of v1 of the series[2].

This patchset is the good way of properly compile driver as a module removing
all MIPS specific code into arch ralink mt7621 place. To avoid mips:allmodconfig reported
problems for v5.16 the following patch has been sent[3]. This series are rebased onto this patch to provide
a real fix for this problem.

[0]: https://lore.kernel.org/linux-mips/CAMhs-H8ShoaYiFOOzJaGC68nZz=V365RXN_Kjuj=fPFENGJiiw@xxxxxxxxxxxxxx/T/#t
[1]: https://lkml.org/lkml/2021/11/14/436
[2]: https://lore.kernel.org/r/20211115070809.15529-1-sergio.paracuellos@xxxxxxxxx
[3]: https://lore.kernel.org/linux-pci/20211203192454.32624-1-sergio.paracuellos@xxxxxxxxx/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(-)

--
2.33.0





[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux