Hi weirongguang,
[+cc linux-pci]
On Thu, Dec 30, 2021 at 6:47 AM weirongguang <weirongguang@xxxxxxxxxx> wrote:
Your subject is a bit generic I think it would be better:
s/Fix the compile error in cross complication/Add missing arch include
'asm/mips-cps.h' to avoid implicit function declarations/
>
> When I was compile the latest kernel in x86 platform and
> the build environment like this:
>
> Compiler: gcc
> Compiler version: 10
> Compiler string: mips-linux-gnu-gcc (Debian 10.2.1-6) 10.2.1 20210110
> Cross-compile: mips-linux-gnu-
>
> It make a compile error:
>
> drivers/pci/controller/pcie-mt7621.c: In function 'setup_cm_memory_region':
> drivers/pci/controller/pcie-mt7621.c:224:6: error: implicit declaration of function 'mips_cps_numiocu' [-Werror=implicit-function-declaration]
> 224 | if (mips_cps_numiocu(0)) {
> | ^~~~~~~~~~~~~~~~
> drivers/pci/controller/pcie-mt7621.c:232:3: error: implicit declaration of function 'write_gcr_reg1_base'; did you mean 'write_gc0_ebase'? [-Werror=implicit-function-declaration]
> 232 | write_gcr_reg1_base(entry->res->start);
> | ^~~~~~~~~~~~~~~~~~~
> | write_gc0_ebase
> drivers/pci/controller/pcie-mt7621.c:233:3: error: implicit declaration of function 'write_gcr_reg1_mask'; did you mean 'write_gc0_pagemask'? [-Werror=implicit-function-declaration]
> 233 | write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> | ^~~~~~~~~~~~~~~~~~~
> | write_gc0_pagemask
> drivers/pci/controller/pcie-mt7621.c:233:30: error: 'CM_GCR_REGn_MASK_CMTGT_IOCU0' undeclared (first use in this function)
> 233 | write_gcr_reg1_mask(mask | CM_GCR_REGn_MASK_CMTGT_IOCU0);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/pci/controller/pcie-mt7621.c:233:30: note: each undeclared identifier is reported only once for each function it appears in
> In file included from ./include/linux/device.h:15,
> from ./include/linux/of_platform.h:9,
> from drivers/pci/controller/pcie-mt7621.c:26:
> drivers/pci/controller/pcie-mt7621.c:235:25: error: implicit declaration of function 'read_gcr_reg1_base'; did you mean 'read_gc0_ebase'? [-Werror=implicit-function-declaration]
> 235 | (unsigned long long)read_gcr_reg1_base(),
> | ^~~~~~~~~~~~~~~~~~
> ./include/linux/dev_printk.h:110:23: note: in definition of macro 'dev_printk_index_wrap'
> 110 | _p_func(dev, fmt, ##__VA_ARGS__); \
> | ^~~~~~~~~~~
> drivers/pci/controller/pcie-mt7621.c:234:3: note: in expansion of macro 'dev_info'
> 234 | dev_info(dev, "PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> | ^~~~~~~~
> drivers/pci/controller/pcie-mt7621.c:236:25: error: implicit declaration of function 'read_gcr_reg1_mask'; did you mean 'read_gc0_pagemask'? [-Werror=implicit-function-declaration]
> 236 | (unsigned long long)read_gcr_reg1_mask());
> | ^~~~~~~~~~~~~~~~~~
> ./include/linux/dev_printk.h:110:23: note: in definition of macro 'dev_printk_index_wrap'
> 110 | _p_func(dev, fmt, ##__VA_ARGS__); \
> | ^~~~~~~~~~~
> drivers/pci/controller/pcie-mt7621.c:234:3: note: in expansion of macro 'dev_info'
> 234 | dev_info(dev, "PCI coherence region base: 0x%08llx, mask/settings: 0x%08llx\n",
> | ^~~~~~~~
> cc1: all warnings being treated as errors
>
> The problem is that the <asm/mips-cps.h> head file was missing
> and it can resolved when include the file.
True. This include header is not explicitly included because platform
Makefiles seems to add include paths for MIPS. I am using:
mipsel-unknown-linux-gnu-gcc (GCC) 9.4.1 20211208 compiled as:
../configure --target=mipsel-unknown-linux-gnu --prefix=/opt/cross \
--enable-languages=c --without-headers \
--with-gnu-ld --with-gnu-as \
--disable-shared --disable-threads \
--disable-libmudflap --disable-libgomp \
--disable-libssp --disable-libquadmath \
--disable-libatomic
and don't get any warning with normal compilation, but it looks like
other toolchains complain about this and fails if -Werror is set, like
your case here.
This was already reported once by kernel test robot, so I think you
should add 'Reported-by' tag also:
Reported-by: kernel test robot <lkp@xxxxxxxxx>
I could not find Kernel Test Robot complaining about lore's link but
it was in a randconfig W=1 build using mips64-linux-gcc (GCC) 11.2.0.
IMHO, the correct thing to do is remove architecture specific code
from the driver side. See [0] (I mention this in PATCH 3 of the series
where Reported-by is also added [1]).
>
> Fix: <2bdd5238e756> ("PCI: mt7621: Add MediaTek MT7621 PCIe host controller driver")
This is not the correct way of "Fixes" tag. It should be:
Fixes: 2bdd5238e756 ("PCI: mt7621: Add MediaTek MT7621 PCIe host
controller driver").
> Signed-off-by: weirongguang <weirongguang@xxxxxxxxxx>
I don't know if "weirongguang" is a valid name or not for a
'Signed-off-by' tag, but just in case I mention it. Is this the name
you use to sign documents? (sorry, I don't want to be rude, but I
don't really know the way chinese names should be used in kernel).
> ---
> drivers/pci/controller/pcie-mt7621.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/controller/pcie-mt7621.c b/drivers/pci/controller/pcie-mt7621.c
> index b60dfb45ef7b..8a009e427a25 100644
> --- a/drivers/pci/controller/pcie-mt7621.c
> +++ b/drivers/pci/controller/pcie-mt7621.c
> @@ -29,6 +29,7 @@
> #include <linux/platform_device.h>
> #include <linux/reset.h>
> #include <linux/sys_soc.h>
> +#include <asm/mips-cps.h>
>
> /* MediaTek-specific configuration registers */
> #define PCIE_FTS_NUM 0x70c
> --
> 2.25.1
As I said, there is a proper approach of removing all the MIPS
specific code from the driver and moving into RALINK platform code
which is the proper place for this arch code here [0]. This should be
material for 5.17. If Lorenzo or Bjorn prefer to add this PATCH
before, you can add my:
Acked-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
when you submit v2 and I will send a revert when the real fix is added.
[0]: https://lore.kernel.org/linux-pci/20211207104924.21327-1-sergio.paracuellos@xxxxxxxxx/
[1]: https://lore.kernel.org/linux-pci/20211207104924.21327-4-sergio.paracuellos@xxxxxxxxx/
Best regards,
Sergio Paracuellos
>
>
> No virus found
> Checked by Hillstone Network AntiVirus
