linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
@ 2021-12-07 10:49 Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 1/5] PCI: Let pcibios_root_bridge_prepare() access to 'bridge->windows' Sergio Paracuellos
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2021-12-07 10:49 UTC (permalink / raw)
  To: linux-pci; +Cc: tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel

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/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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/5] PCI: Let pcibios_root_bridge_prepare() access to 'bridge->windows'
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
@ 2021-12-07 10:49 ` Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()' Sergio Paracuellos
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2021-12-07 10:49 UTC (permalink / raw)
  To: linux-pci
  Cc: tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel,
	Arnd Bergmann

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 <arnd@arndb.de>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 1/5] PCI: Let pcibios_root_bridge_prepare() access to 'bridge->windows' Sergio Paracuellos
@ 2021-12-07 10:49 ` Sergio Paracuellos
  2022-01-12 18:20   ` Guenter Roeck
  2022-01-12 20:10   ` Thomas Bogendoerfer
  2021-12-07 10:49 ` [PATCH v3 3/5] PCI: mt7621: Avoid custom MIPS code in driver code Sergio Paracuellos
                   ` (4 subsequent siblings)
  6 siblings, 2 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2021-12-07 10:49 UTC (permalink / raw)
  To: linux-pci; +Cc: tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel

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 <sergio.paracuellos@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/5] PCI: mt7621: Avoid custom MIPS code in driver code
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 1/5] PCI: Let pcibios_root_bridge_prepare() access to 'bridge->windows' Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()' Sergio Paracuellos
@ 2021-12-07 10:49 ` Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 4/5] PCI: mt7621: Add missing 'MODULE_LICENSE()' definition Sergio Paracuellos
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2021-12-07 10:49 UTC (permalink / raw)
  To: linux-pci
  Cc: tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel,
	kernel test robot

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 <lkp@intel.com>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 4/5] PCI: mt7621: Add missing 'MODULE_LICENSE()' definition
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
                   ` (2 preceding siblings ...)
  2021-12-07 10:49 ` [PATCH v3 3/5] PCI: mt7621: Avoid custom MIPS code in driver code Sergio Paracuellos
@ 2021-12-07 10:49 ` Sergio Paracuellos
  2021-12-07 10:49 ` [PATCH v3 5/5] PCI: mt7621: Allow COMPILE_TEST for all arches Sergio Paracuellos
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2021-12-07 10:49 UTC (permalink / raw)
  To: linux-pci
  Cc: tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel,
	Krzysztof Wilczyński, Yanteng Si

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 <kw@linux.com>
Signed-off-by: Yanteng Si <siyanteng@loongson.cn>
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 5/5] PCI: mt7621: Allow COMPILE_TEST for all arches
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
                   ` (3 preceding siblings ...)
  2021-12-07 10:49 ` [PATCH v3 4/5] PCI: mt7621: Add missing 'MODULE_LICENSE()' definition Sergio Paracuellos
@ 2021-12-07 10:49 ` Sergio Paracuellos
  2022-01-12 14:42 ` [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
  2022-01-12 21:52 ` Bjorn Helgaas
  6 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2021-12-07 10:49 UTC (permalink / raw)
  To: linux-pci; +Cc: tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel

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 <sergio.paracuellos@gmail.com>
---
 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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
                   ` (4 preceding siblings ...)
  2021-12-07 10:49 ` [PATCH v3 5/5] PCI: mt7621: Allow COMPILE_TEST for all arches Sergio Paracuellos
@ 2022-01-12 14:42 ` Sergio Paracuellos
  2022-01-12 18:06   ` Lorenzo Pieralisi
  2022-01-12 21:52 ` Bjorn Helgaas
  6 siblings, 1 reply; 16+ messages in thread
From: Sergio Paracuellos @ 2022-01-12 14:42 UTC (permalink / raw)
  To: linux-pci
  Cc: Thomas Bogendoerfer, Lorenzo Pieralisi, Bjorn Helgaas,
	Guenter Roeck, linux-kernel

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?

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
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
  2022-01-12 14:42 ` [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
@ 2022-01-12 18:06   ` Lorenzo Pieralisi
  2022-01-12 18:21     ` Guenter Roeck
  2022-01-12 20:09     ` Sergio Paracuellos
  0 siblings, 2 replies; 16+ messages in thread
From: Lorenzo Pieralisi @ 2022-01-12 18:06 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: linux-pci, Thomas Bogendoerfer, Bjorn Helgaas, Guenter Roeck,
	linux-kernel

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.

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
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
  2021-12-07 10:49 ` [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()' Sergio Paracuellos
@ 2022-01-12 18:20   ` Guenter Roeck
  2022-01-12 20:10     ` Sergio Paracuellos
  2022-01-12 20:10   ` Thomas Bogendoerfer
  1 sibling, 1 reply; 16+ messages in thread
From: Guenter Roeck @ 2022-01-12 18:20 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: linux-pci, tsbogend, lorenzo.pieralisi, bhelgaas, linux-kernel

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 <sergio.paracuellos@gmail.com>

FWIW:

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  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
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
  2022-01-12 18:06   ` Lorenzo Pieralisi
@ 2022-01-12 18:21     ` Guenter Roeck
  2022-01-12 20:09     ` Sergio Paracuellos
  1 sibling, 0 replies; 16+ messages in thread
From: Guenter Roeck @ 2022-01-12 18:21 UTC (permalink / raw)
  To: Lorenzo Pieralisi, Sergio Paracuellos
  Cc: linux-pci, Thomas Bogendoerfer, Bjorn Helgaas, linux-kernel

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
>>>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
  2022-01-12 18:06   ` Lorenzo Pieralisi
  2022-01-12 18:21     ` Guenter Roeck
@ 2022-01-12 20:09     ` Sergio Paracuellos
  1 sibling, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2022-01-12 20:09 UTC (permalink / raw)
  To: Lorenzo Pieralisi
  Cc: linux-pci, Thomas Bogendoerfer, Bjorn Helgaas, Guenter Roeck,
	linux-kernel

On Wed, Jan 12, 2022 at 7:06 PM Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> 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.

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/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
> > >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
  2022-01-12 18:20   ` Guenter Roeck
@ 2022-01-12 20:10     ` Sergio Paracuellos
  0 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2022-01-12 20:10 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-pci, Thomas Bogendoerfer, Lorenzo Pieralisi, Bjorn Helgaas,
	linux-kernel

On Wed, Jan 12, 2022 at 7:20 PM Guenter Roeck <linux@roeck-us.net> 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 <sergio.paracuellos@gmail.com>
>
> FWIW:
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>

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
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
  2021-12-07 10:49 ` [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()' Sergio Paracuellos
  2022-01-12 18:20   ` Guenter Roeck
@ 2022-01-12 20:10   ` Thomas Bogendoerfer
  2022-01-13  5:53     ` Sergio Paracuellos
  1 sibling, 1 reply; 16+ messages in thread
From: Thomas Bogendoerfer @ 2022-01-12 20:10 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: linux-pci, lorenzo.pieralisi, bhelgaas, linux, linux-kernel

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 <sergio.paracuellos@gmail.com>
> ---
>  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 <tsbogend@alpha.franken.de>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
  2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
                   ` (5 preceding siblings ...)
  2022-01-12 14:42 ` [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
@ 2022-01-12 21:52 ` Bjorn Helgaas
  2022-01-13  5:52   ` Sergio Paracuellos
  6 siblings, 1 reply; 16+ messages in thread
From: Bjorn Helgaas @ 2022-01-12 21:52 UTC (permalink / raw)
  To: Sergio Paracuellos
  Cc: linux-pci, tsbogend, lorenzo.pieralisi, bhelgaas, linux, linux-kernel

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/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(-)

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.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver
  2022-01-12 21:52 ` Bjorn Helgaas
@ 2022-01-13  5:52   ` Sergio Paracuellos
  0 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2022-01-13  5:52 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, Thomas Bogendoerfer, Lorenzo Pieralisi, Bjorn Helgaas,
	Guenter Roeck, linux-kernel

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 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/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(-)
>
> 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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()'
  2022-01-12 20:10   ` Thomas Bogendoerfer
@ 2022-01-13  5:53     ` Sergio Paracuellos
  0 siblings, 0 replies; 16+ messages in thread
From: Sergio Paracuellos @ 2022-01-13  5:53 UTC (permalink / raw)
  To: Thomas Bogendoerfer
  Cc: linux-pci, Lorenzo Pieralisi, Bjorn Helgaas, Guenter Roeck, linux-kernel

On Wed, Jan 12, 2022 at 9:11 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> 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 <sergio.paracuellos@gmail.com>
> > ---
> >  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 <tsbogend@alpha.franken.de>

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 ]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-01-13  5:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07 10:49 [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
2021-12-07 10:49 ` [PATCH v3 1/5] PCI: Let pcibios_root_bridge_prepare() access to 'bridge->windows' Sergio Paracuellos
2021-12-07 10:49 ` [PATCH v3 2/5] MIPS: ralink: implement 'pcibios_root_bridge_prepare()' Sergio Paracuellos
2022-01-12 18:20   ` Guenter Roeck
2022-01-12 20:10     ` Sergio Paracuellos
2022-01-12 20:10   ` Thomas Bogendoerfer
2022-01-13  5:53     ` Sergio Paracuellos
2021-12-07 10:49 ` [PATCH v3 3/5] PCI: mt7621: Avoid custom MIPS code in driver code Sergio Paracuellos
2021-12-07 10:49 ` [PATCH v3 4/5] PCI: mt7621: Add missing 'MODULE_LICENSE()' definition Sergio Paracuellos
2021-12-07 10:49 ` [PATCH v3 5/5] PCI: mt7621: Allow COMPILE_TEST for all arches Sergio Paracuellos
2022-01-12 14:42 ` [PATCH v3 0/5] PCI: mt7621: Remove specific MIPS code from driver Sergio Paracuellos
2022-01-12 18:06   ` Lorenzo Pieralisi
2022-01-12 18:21     ` Guenter Roeck
2022-01-12 20:09     ` Sergio Paracuellos
2022-01-12 21:52 ` Bjorn Helgaas
2022-01-13  5:52   ` Sergio Paracuellos

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).