Sender: Guenter Roeck <groeck7@gmail.com>
Subject: Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
To:     Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Sergio Paracuellos <sergio.paracuellos@gmail.com>
Cc:     linux-pci <linux-pci@vger.kernel.org>,
        Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
        Bjorn Helgaas <bhelgaas@google.com>,
        linux-kernel <linux-kernel@vger.kernel.org>
References: <20211207104924.21327-1-sergio.paracuellos@gmail.com>
 <CAMhs-H886ZPZED-qmMtZcWFabRLNL14Y7SSz_Ko7d45zXL+v2w@mail.gmail.com>
 <20220112180627.GB1319@lpieralisi>
From:   Guenter Roeck <linux@roeck-us.net>
Message-ID: <d4780c9c-3404-5e18-808f-29108316486b@roeck-us.net>
Date:   Wed, 12 Jan 2022 10:21:20 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
MIME-Version: 1.0
In-Reply-To: <20220112180627.GB1319@lpieralisi>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@gmail.com> 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@mail.gmail.com/T/#t
>>> [1]: https://lkml.org/lkml/2021/11/14/436
>>> [2]: https://lore.kernel.org/r/20211115070809.15529-1-sergio.paracuellos@gmail.com
>>> [3]: https://lore.kernel.org/linux-pci/20211203192454.32624-1-sergio.paracuellos@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(-)
>>>
>>> --
>>> 2.33.0
>>>