Re: [PATCH] PCI: mt7621: Fix the compile error in cross complication

Hi weirongguang,

[+cc linux-pci]

On Thu, Dec 30, 2021 at 6:47 AM weirongguang [off-list ref] 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 <redacted>

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

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

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

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@gmail.com/ (local)
[1]: https://lore.kernel.org/linux-pci/20211207104924.21327-4-sergio.paracuellos@gmail.com/ (local)

Best regards,
    Sergio Paracuellos

No virus found
                Checked by Hillstone Network AntiVirus

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek