[PATCH] PCI: ftpci100: rename macro name collision

[PATCH] PCI: ftpci100: rename macro name collision

[Randy Dunlap]

PCI_IOSIZE is defined in mach-loongson64/spaces.h, so change the name
of this macro in pci-ftpci100.c.

../drivers/pci/controller/pci-ftpci100.c:37: warning: "PCI_IOSIZE" redefined
   37 | #define PCI_IOSIZE 0x00
      | 
In file included from ../arch/mips/include/asm/addrspace.h:13,
...              from ../drivers/pci/controller/pci-ftpci100.c:15:
arch/mips/include/asm/mach-loongson64/spaces.h:11: note: this is the location of the previous definition
   11 | #define PCI_IOSIZE SZ_16M

Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: linux-mips@vger.kernel.org
---
 drivers/pci/controller/pci-ftpci100.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-next-20210514.orig/drivers/pci/controller/pci-ftpci100.c
+++ linux-next-20210514/drivers/pci/controller/pci-ftpci100.c
@@ -34,7 +34,7 @@
  * Special configuration registers directly in the first few words
  * in I/O space.
  */
-#define PCI_IOSIZE	0x00
+#define PCI_IOLIMIT	0x00
 #define PCI_PROT	0x04 /* AHB protection */
 #define PCI_CTRL	0x08 /* PCI control signal */
 #define PCI_SOFTRST	0x10 /* Soft reset counter and response error enable */
@@ -469,7 +469,7 @@ static int faraday_pci_probe(struct plat
 		if (!faraday_res_to_memcfg(io->start - win->offset,
 					   resource_size(io), &val)) {
 			/* setup I/O space size */
-			writel(val, p->base + PCI_IOSIZE);
+			writel(val, p->base + PCI_IOLIMIT);
 		} else {
 			dev_err(dev, "illegal IO mem size\n");
 			return -EINVAL;

Re: [PATCH] PCI: ftpci100: rename macro name collision

[Linus Walleij]

On Sun, May 16, 2021 at 9:00 PM Randy Dunlap <rdunlap@infradead.org> wrote:

> PCI_IOSIZE is defined in mach-loongson64/spaces.h, so change the name
> of this macro in pci-ftpci100.c.
>
> ../drivers/pci/controller/pci-ftpci100.c:37: warning: "PCI_IOSIZE" redefined
>    37 | #define PCI_IOSIZE 0x00
>       |
> In file included from ../arch/mips/include/asm/addrspace.h:13,
> ...              from ../drivers/pci/controller/pci-ftpci100.c:15:
> arch/mips/include/asm/mach-loongson64/spaces.h:11: note: this is the location of the previous definition
>    11 | #define PCI_IOSIZE SZ_16M
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Jiaxun Yang <jiaxun.yang@flygoat.com>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> Cc: linux-mips@vger.kernel.org

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Though I suspect the real solution is to prefix all macros with FTPCI_*?

Yours,
Linus Walleij

Re: [PATCH] PCI: ftpci100: rename macro name collision

[Krzysztof Wilczyński]

Hi Randy and Linus,

[...]
> > PCI_IOSIZE is defined in mach-loongson64/spaces.h, so change the name
> > of this macro in pci-ftpci100.c.
[...]
> Though I suspect the real solution is to prefix all macros with FTPCI_*?

Agreed, especially since some of the constants and macros in this
driver already prefix various names using "FARADAY_".  We could keep
this pattern and apply this prefix to other things.  There are also
other constants and macros named starting with "PCI_" that could
potentially be renamed too.

Having said that, I actually wonder if some of these constants and
macros are would be something we already have declared (people tend to
often solve the same problems)and could be reused, as per:

  #define PCI_IOSIZE	0x00
  #define PCI_PROT	0x04 /* AHB protection */
  #define PCI_CTRL	0x08 /* PCI control signal */
  #define PCI_SOFTRST	0x10 /* Soft reset counter and response error enable */
  #define PCI_CONFIG	0x28 /* PCI configuration command register */
  #define PCI_DATA	0x2C

Or these:

  #define PCI_CONF_ENABLE	BIT(31)
  #define PCI_CONF_WHERE(r)	((r) & 0xFC)
  #define PCI_CONF_BUS(b)	(((b) & 0xFF) << 16)
  #define PCI_CONF_DEVICE(d)	(((d) & 0x1F) << 11)
  #define PCI_CONF_FUNCTION(f)	(((f) & 0x07) << 8)

Krzysztof

Re: [PATCH] PCI: ftpci100: rename macro name collision

[Randy Dunlap]

On 5/17/21 3:34 AM, Krzysztof Wilczyński wrote:
> Hi Randy and Linus,
> 
> [...]
>>> PCI_IOSIZE is defined in mach-loongson64/spaces.h, so change the name
>>> of this macro in pci-ftpci100.c.
> [...]
>> Though I suspect the real solution is to prefix all macros with FTPCI_*?

I'm willing to go that far.

> Agreed, especially since some of the constants and macros in this
> driver already prefix various names using "FARADAY_".  We could keep
> this pattern and apply this prefix to other things.  There are also
> other constants and macros named starting with "PCI_" that could
> potentially be renamed too.
> 
> Having said that, I actually wonder if some of these constants and
> macros are would be something we already have declared (people tend to
> often solve the same problems)and could be reused, as per:
> 
>   #define PCI_IOSIZE	0x00
>   #define PCI_PROT	0x04 /* AHB protection */
>   #define PCI_CTRL	0x08 /* PCI control signal */
>   #define PCI_SOFTRST	0x10 /* Soft reset counter and response error enable */
>   #define PCI_CONFIG	0x28 /* PCI configuration command register */
>   #define PCI_DATA	0x2C
> 
> Or these:
> 
>   #define PCI_CONF_ENABLE	BIT(31)
>   #define PCI_CONF_WHERE(r)	((r) & 0xFC)
>   #define PCI_CONF_BUS(b)	(((b) & 0xFF) << 16)
>   #define PCI_CONF_DEVICE(d)	(((d) & 0x1F) << 11)
>   #define PCI_CONF_FUNCTION(f)	(((f) & 0x07) << 8)


If you would like to take that and run with it, please go ahead.

thanks.
-- 
~Randy

Re: [PATCH] PCI: ftpci100: rename macro name collision

[Linus Walleij]

On Mon, May 17, 2021 at 6:16 PM Randy Dunlap <rdunlap@infradead.org> wrote:

> On 5/17/21 3:34 AM, Krzysztof Wilczyński wrote:
> > Hi Randy and Linus,
> >
> > [...]
> >>> PCI_IOSIZE is defined in mach-loongson64/spaces.h, so change the name
> >>> of this macro in pci-ftpci100.c.
> > [...]
> >> Though I suspect the real solution is to prefix all macros with FTPCI_*?
>
> I'm willing to go that far.

Go for it.

> > Having said that, I actually wonder if some of these constants and
> > macros are would be something we already have declared (people tend to
> > often solve the same problems)and could be reused, as per:
(...)
> If you would like to take that and run with it, please go ahead.

I will look into it on top of your syntactic fix.

Yours,
Linus Walleij