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.
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/[email protected]
[3]: https://lore.kernel.org/linux-pci/[email protected]/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
When function pci_register_host_bridge() is called, 'bridge->windows' are
already available. However this windows are being moved temporarily from
there. To let pcibios_root_bridge_prepare() to have access to this windows
move this windows movement after call this function. This is interesting for
MIPS ralink mt7621 platform to be able to properly set I/O coherence units
with this information and avoid custom MIPS code in generic PCIe controller
drivers.
Acked-by: Arnd Bergmann <[email protected]>
Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/pci/probe.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 087d3658f75c..372a70efccc6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -898,8 +898,6 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
bridge->bus = bus;
- /* Temporarily move resources off the list */
- list_splice_init(&bridge->windows, &resources);
bus->sysdata = bridge->sysdata;
bus->ops = bridge->ops;
bus->number = bus->busn_res.start = bridge->busnr;
@@ -925,6 +923,8 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
if (err)
goto free;
+ /* Temporarily move resources off the list */
+ list_splice_init(&bridge->windows, &resources);
err = device_add(&bridge->dev);
if (err) {
put_device(&bridge->dev);
--
2.33.0
PCI core code call 'pcibios_root_bridge_prepare()' function inside function
'pci_register_host_bridge()'. This point is very good way to properly enter
into this MIPS ralink specific code to properly setup I/O coherency units
with PCI memory addresses.
Signed-off-by: Sergio Paracuellos <[email protected]>
---
arch/mips/ralink/mt7621.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
index bd71f5b14238..d6efffd4dd20 100644
--- a/arch/mips/ralink/mt7621.c
+++ b/arch/mips/ralink/mt7621.c
@@ -10,6 +10,8 @@
#include <linux/slab.h>
#include <linux/sys_soc.h>
#include <linux/memblock.h>
+#include <linux/pci.h>
+#include <linux/bug.h>
#include <asm/bootinfo.h>
#include <asm/mipsregs.h>
@@ -22,6 +24,35 @@
static void *detect_magic __initdata = detect_memory_region;
+int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
+{
+ struct resource_entry *entry;
+ resource_size_t mask;
+
+ entry = resource_list_first_type(&bridge->windows, IORESOURCE_MEM);
+ if (!entry) {
+ pr_err("Cannot get memory resource\n");
+ return -EINVAL;
+ }
+
+ if (mips_cps_numiocu(0)) {
+ /*
+ * Hardware doesn't accept mask values with 1s after
+ * 0s (e.g. 0xffef), so warn if that's happen
+ */
+ mask = ~(entry->res->end - entry->res->start) & CM_GCR_REGn_MASK_ADDRMASK;
+ WARN_ON(mask && BIT(ffz(~mask)) - 1 != ~mask);
+
+ write_gcr_reg1_base(entry->res->start);
+ write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
+ pr_info("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
+ (unsigned long long)read_gcr_reg1_base(),
+ (unsigned long long)read_gcr_reg1_mask());
+ }
+
+ return 0;
+}
+
phys_addr_t mips_cpc_default_phys_base(void)
{
panic("Cannot detect cpc address");
--
2.33.0
Driver code is setting up MIPS specific I/O coherency units addresses config.
This MIPS specific thing has been moved to be done when PCI code call the
'pcibios_root_bridge_prepare()' function which has been implemented for MIPS
ralink mt7621 platform. Hence, remove MIPS specific code from driver code.
After this change there is also no need to add any MIPS specific includes
to avoid some errors reported by Kernet Test Robot with W=1 builds.
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/pci/controller/pcie-mt7621.c | 37 ----------------------------
1 file changed, 37 deletions(-)
diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
index 4138c0e83513..42cce31df943 100644
--- a/drivers/pci/controller/pcie-mt7621.c
+++ b/drivers/pci/controller/pcie-mt7621.c
@@ -208,37 +208,6 @@ static inline void mt7621_control_deassert(struct mt7621_pcie_port *port)
reset_control_assert(port->pcie_rst);
}
-static int setup_cm_memory_region(struct pci_host_bridge *host)
-{
- struct mt7621_pcie *pcie = pci_host_bridge_priv(host);
- struct device *dev = pcie->dev;
- struct resource_entry *entry;
- resource_size_t mask;
-
- entry = resource_list_first_type(&host->windows, IORESOURCE_MEM);
- if (!entry) {
- dev_err(dev, "cannot get memory resource\n");
- return -EINVAL;
- }
-
- if (mips_cps_numiocu(0)) {
- /*
- * FIXME: hardware doesn't accept mask values with 1s after
- * 0s (e.g. 0xffef), so it would be great to warn if that's
- * about to happen
- */
- mask = ~(entry->res->end - entry->res->start);
-
- write_gcr_reg1_base(entry->res->start);
- write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
- dev_info(dev, "PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
- (unsigned long long)read_gcr_reg1_base(),
- (unsigned long long)read_gcr_reg1_mask());
- }
-
- return 0;
-}
-
static int mt7621_pcie_parse_port(struct mt7621_pcie *pcie,
struct device_node *node,
int slot)
@@ -557,12 +526,6 @@ static int mt7621_pci_probe(struct platform_device *pdev)
goto remove_resets;
}
- err = setup_cm_memory_region(bridge);
- if (err) {
- dev_err(dev, "error setting up iocu mem regions\n");
- goto remove_resets;
- }
-
return mt7621_pcie_register_host(bridge);
remove_resets:
--
2.33.0
MT7621 PCIe host controller driver can be built as a module but there is no
'MODULE_LICENSE()' specified in code, causing a build error due to missing
license information.
ERROR: modpost: missing MODULE_LICENSE() in drivers/pci/controller/pcie-mt7621.o
Fix this by adding 'MODULE_LICENSE()' to the driver.
Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver")
Reviewed-by: Krzysztof Wilczyński <[email protected]>
Signed-off-by: Yanteng Si <[email protected]>
Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/pci/controller/pcie-mt7621.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
index 42cce31df943..9da7452f565e 100644
--- a/drivers/pci/controller/pcie-mt7621.c
+++ b/drivers/pci/controller/pcie-mt7621.c
@@ -561,3 +561,5 @@ static struct platform_driver mt7621_pci_driver = {
},
};
builtin_platform_driver(mt7621_pci_driver);
+
+MODULE_LICENSE("GPL v2");
--
2.33.0
Since all MIPS specific code has been moved from the controller driver side
there is no more stoppers to enable the driver to be completely enable for
'COMPILE_TEST'. So mark as tristate and remove MIPS conditional statement.
Signed-off-by: Sergio Paracuellos <[email protected]>
---
drivers/pci/controller/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 7fc5135ffbbf..2d5a86f9089c 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -332,8 +332,8 @@ config PCIE_APPLE
If unsure, say Y if you have an Apple Silicon system.
config PCIE_MT7621
- bool "MediaTek MT7621 PCIe Controller"
- depends on SOC_MT7621 || (MIPS && COMPILE_TEST)
+ tristate "MediaTek MT7621 PCIe Controller"
+ depends on SOC_MT7621 || COMPILE_TEST
select PHY_MT7621_PCI
default SOC_MT7621
help
--
2.33.0
Hi Bjorn, Lorenzo,
On Tue, Dec 7, 2021 at 11:49 AM Sergio Paracuellos
<[email protected]> 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?
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/[email protected]
> [3]: https://lore.kernel.org/linux-pci/[email protected]/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
>
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
> <[email protected]> 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.
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/[email protected]
> > [3]: https://lore.kernel.org/linux-pci/[email protected]/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
> >
On Tue, Dec 07, 2021 at 11:49:21AM +0100, Sergio Paracuellos wrote:
> PCI core code call 'pcibios_root_bridge_prepare()' function inside function
> 'pci_register_host_bridge()'. This point is very good way to properly enter
> into this MIPS ralink specific code to properly setup I/O coherency units
> with PCI memory addresses.
>
> Signed-off-by: Sergio Paracuellos <[email protected]>
FWIW:
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> arch/mips/ralink/mt7621.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
> index bd71f5b14238..d6efffd4dd20 100644
> --- a/arch/mips/ralink/mt7621.c
> +++ b/arch/mips/ralink/mt7621.c
> @@ -10,6 +10,8 @@
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
> #include <linux/memblock.h>
> +#include <linux/pci.h>
> +#include <linux/bug.h>
>
> #include <asm/bootinfo.h>
> #include <asm/mipsregs.h>
> @@ -22,6 +24,35 @@
>
> static void *detect_magic __initdata = detect_memory_region;
>
> +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> +{
> + struct resource_entry *entry;
> + resource_size_t mask;
> +
> + entry = resource_list_first_type(&bridge->windows, IORESOURCE_MEM);
> + if (!entry) {
> + pr_err("Cannot get memory resource\n");
> + return -EINVAL;
> + }
> +
> + if (mips_cps_numiocu(0)) {
> + /*
> + * Hardware doesn't accept mask values with 1s after
> + * 0s (e.g. 0xffef), so warn if that's happen
> + */
> + mask = ~(entry->res->end - entry->res->start) & CM_GCR_REGn_MASK_ADDRMASK;
> + WARN_ON(mask && BIT(ffz(~mask)) - 1 != ~mask);
> +
> + write_gcr_reg1_base(entry->res->start);
> + write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> + pr_info("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> + (unsigned long long)read_gcr_reg1_base(),
> + (unsigned long long)read_gcr_reg1_mask());
> + }
> +
> + return 0;
> +}
> +
> phys_addr_t mips_cpc_default_phys_base(void)
> {
> panic("Cannot detect cpc address");
> --
> 2.33.0
>
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
>> <[email protected]> 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/[email protected]
>>> [3]: https://lore.kernel.org/linux-pci/[email protected]/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
>>>
On Wed, Jan 12, 2022 at 7:06 PM Lorenzo Pieralisi
<[email protected]> 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
> > <[email protected]> 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.
I was expecting Thomas to get ACK for this patch or get it through the
MIPS tree.
>
> It would also be great if Bjorn reviewed it to make sure he agrees
> with the approach.
Agreed.
>
> I think it is too late for this cycle, apologies, there is a significant
> review backlog.
>
> Lorenzo
Best regards,
Sergio Paracuellos
>
> > 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/[email protected]
> > > [3]: https://lore.kernel.org/linux-pci/[email protected]/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
> > >
On Wed, Jan 12, 2022 at 7:20 PM Guenter Roeck <[email protected]> wrote:
>
> On Tue, Dec 07, 2021 at 11:49:21AM +0100, Sergio Paracuellos wrote:
> > PCI core code call 'pcibios_root_bridge_prepare()' function inside function
> > 'pci_register_host_bridge()'. This point is very good way to properly enter
> > into this MIPS ralink specific code to properly setup I/O coherency units
> > with PCI memory addresses.
> >
> > Signed-off-by: Sergio Paracuellos <[email protected]>
>
> FWIW:
>
> Reviewed-by: Guenter Roeck <[email protected]>
Thanks!
Best regards,
Sergio Paracuellos
>
> > ---
> > arch/mips/ralink/mt7621.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
> > index bd71f5b14238..d6efffd4dd20 100644
> > --- a/arch/mips/ralink/mt7621.c
> > +++ b/arch/mips/ralink/mt7621.c
> > @@ -10,6 +10,8 @@
> > #include <linux/slab.h>
> > #include <linux/sys_soc.h>
> > #include <linux/memblock.h>
> > +#include <linux/pci.h>
> > +#include <linux/bug.h>
> >
> > #include <asm/bootinfo.h>
> > #include <asm/mipsregs.h>
> > @@ -22,6 +24,35 @@
> >
> > static void *detect_magic __initdata = detect_memory_region;
> >
> > +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> > +{
> > + struct resource_entry *entry;
> > + resource_size_t mask;
> > +
> > + entry = resource_list_first_type(&bridge->windows, IORESOURCE_MEM);
> > + if (!entry) {
> > + pr_err("Cannot get memory resource\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (mips_cps_numiocu(0)) {
> > + /*
> > + * Hardware doesn't accept mask values with 1s after
> > + * 0s (e.g. 0xffef), so warn if that's happen
> > + */
> > + mask = ~(entry->res->end - entry->res->start) & CM_GCR_REGn_MASK_ADDRMASK;
> > + WARN_ON(mask && BIT(ffz(~mask)) - 1 != ~mask);
> > +
> > + write_gcr_reg1_base(entry->res->start);
> > + write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> > + pr_info("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> > + (unsigned long long)read_gcr_reg1_base(),
> > + (unsigned long long)read_gcr_reg1_mask());
> > + }
> > +
> > + return 0;
> > +}
> > +
> > phys_addr_t mips_cpc_default_phys_base(void)
> > {
> > panic("Cannot detect cpc address");
> > --
> > 2.33.0
> >
On Tue, Dec 07, 2021 at 11:49:21AM +0100, Sergio Paracuellos wrote:
> PCI core code call 'pcibios_root_bridge_prepare()' function inside function
> 'pci_register_host_bridge()'. This point is very good way to properly enter
> into this MIPS ralink specific code to properly setup I/O coherency units
> with PCI memory addresses.
>
> Signed-off-by: Sergio Paracuellos <[email protected]>
> ---
> arch/mips/ralink/mt7621.c | 31 +++++++++++++++++++++++++++++++
> 1 file changed, 31 insertions(+)
>
> diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
> index bd71f5b14238..d6efffd4dd20 100644
> --- a/arch/mips/ralink/mt7621.c
> +++ b/arch/mips/ralink/mt7621.c
> @@ -10,6 +10,8 @@
> #include <linux/slab.h>
> #include <linux/sys_soc.h>
> #include <linux/memblock.h>
> +#include <linux/pci.h>
> +#include <linux/bug.h>
>
> #include <asm/bootinfo.h>
> #include <asm/mipsregs.h>
> @@ -22,6 +24,35 @@
>
> static void *detect_magic __initdata = detect_memory_region;
>
> +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> +{
> + struct resource_entry *entry;
> + resource_size_t mask;
> +
> + entry = resource_list_first_type(&bridge->windows, IORESOURCE_MEM);
> + if (!entry) {
> + pr_err("Cannot get memory resource\n");
> + return -EINVAL;
> + }
> +
> + if (mips_cps_numiocu(0)) {
> + /*
> + * Hardware doesn't accept mask values with 1s after
> + * 0s (e.g. 0xffef), so warn if that's happen
> + */
> + mask = ~(entry->res->end - entry->res->start) & CM_GCR_REGn_MASK_ADDRMASK;
> + WARN_ON(mask && BIT(ffz(~mask)) - 1 != ~mask);
> +
> + write_gcr_reg1_base(entry->res->start);
> + write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> + pr_info("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> + (unsigned long long)read_gcr_reg1_base(),
> + (unsigned long long)read_gcr_reg1_mask());
> + }
> +
> + return 0;
> +}
> +
> phys_addr_t mips_cpc_default_phys_base(void)
> {
> panic("Cannot detect cpc address");
> --
> 2.33.0
Acked-by: Thomas Bogendoerfer <[email protected]>
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
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 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.
>
> 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/[email protected]
> [3]: https://lore.kernel.org/linux-pci/[email protected]/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.
Hi Bjorn,
On Wed, Jan 12, 2022 at 10:52 PM Bjorn Helgaas <[email protected]> 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 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.
> >
> > 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/[email protected]
> > [3]: https://lore.kernel.org/linux-pci/[email protected]/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
On Wed, Jan 12, 2022 at 9:11 PM Thomas Bogendoerfer
<[email protected]> wrote:
>
> On Tue, Dec 07, 2021 at 11:49:21AM +0100, Sergio Paracuellos wrote:
> > PCI core code call 'pcibios_root_bridge_prepare()' function inside function
> > 'pci_register_host_bridge()'. This point is very good way to properly enter
> > into this MIPS ralink specific code to properly setup I/O coherency units
> > with PCI memory addresses.
> >
> > Signed-off-by: Sergio Paracuellos <[email protected]>
> > ---
> > arch/mips/ralink/mt7621.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/arch/mips/ralink/mt7621.c b/arch/mips/ralink/mt7621.c
> > index bd71f5b14238..d6efffd4dd20 100644
> > --- a/arch/mips/ralink/mt7621.c
> > +++ b/arch/mips/ralink/mt7621.c
> > @@ -10,6 +10,8 @@
> > #include <linux/slab.h>
> > #include <linux/sys_soc.h>
> > #include <linux/memblock.h>
> > +#include <linux/pci.h>
> > +#include <linux/bug.h>
> >
> > #include <asm/bootinfo.h>
> > #include <asm/mipsregs.h>
> > @@ -22,6 +24,35 @@
> >
> > static void *detect_magic __initdata = detect_memory_region;
> >
> > +int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge)
> > +{
> > + struct resource_entry *entry;
> > + resource_size_t mask;
> > +
> > + entry = resource_list_first_type(&bridge->windows, IORESOURCE_MEM);
> > + if (!entry) {
> > + pr_err("Cannot get memory resource\n");
> > + return -EINVAL;
> > + }
> > +
> > + if (mips_cps_numiocu(0)) {
> > + /*
> > + * Hardware doesn't accept mask values with 1s after
> > + * 0s (e.g. 0xffef), so warn if that's happen
> > + */
> > + mask = ~(entry->res->end - entry->res->start) & CM_GCR_REGn_MASK_ADDRMASK;
> > + WARN_ON(mask && BIT(ffz(~mask)) - 1 != ~mask);
> > +
> > + write_gcr_reg1_base(entry->res->start);
> > + write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> > + pr_info("PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> > + (unsigned long long)read_gcr_reg1_base(),
> > + (unsigned long long)read_gcr_reg1_mask());
> > + }
> > +
> > + return 0;
> > +}
> > +
> > phys_addr_t mips_cpc_default_phys_base(void)
> > {
> > panic("Cannot detect cpc address");
> > --
> > 2.33.0
>
> Acked-by: Thomas Bogendoerfer <[email protected]>
Thanks, Thomas!
Best regards,
Sergio Paracuellos
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea. [ RFC1925, 2.3 ]