| Message ID | 20210516190014.25664-1-rdunlap@infradead.org |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: ftpci100: rename macro name collision | expand |
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
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
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.
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
--- 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;
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(-)