| Message ID | 1479806768-39911-5-git-send-email-vladimir.murzin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tuesday, November 22, 2016 9:26:01 AM CET Vladimir Murzin wrote: > This driver uses functionality which available for MMU build only, > thus add dependency on MMU. > > Cc: Thierry Reding <thierry.reding@gmail.com> > Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > Can you be more specific about what requires the MMU here? Is it the I/O space remapping or something else? Arnd
On 22/11/16 09:31, Arnd Bergmann wrote: > On Tuesday, November 22, 2016 9:26:01 AM CET Vladimir Murzin wrote: >> This driver uses functionality which available for MMU build only, >> thus add dependency on MMU. >> >> Cc: Thierry Reding <thierry.reding@gmail.com> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >> > > Can you be more specific about what requires the MMU here? > > Is it the I/O space remapping or something else? Yes it comes from I/O space remapping. The fill error log: CC drivers/pci/host/pci-tegra.o In file included from ./arch/arm/include/asm/page.h:22:0, from ./arch/arm/include/asm/thread_info.h:17, from ./include/linux/thread_info.h:58, from ./include/asm-generic/current.h:4, from ./arch/arm/include/generated/asm/current.h:1, from ./include/linux/mutex.h:13, from ./include/linux/notifier.h:13, from ./include/linux/clk.h:17, from drivers/pci/host/pci-tegra.c:29: drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_bus_alloc': drivers/pci/host/pci-tegra.c:388:27: error: 'L_PTE_PRESENT' undeclared (first use in this function) pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c:388:27: note: each undeclared identifier is reported only once for each function it appears in pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c:388:43: error: 'L_PTE_YOUNG' undeclared (first use in this function) pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c:388:57: error: 'L_PTE_DIRTY' undeclared (first use in this function) pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c:389:6: error: 'L_PTE_XN' undeclared (first use in this function) L_PTE_XN | L_PTE_MT_DEV_SHARED | L_PTE_SHARED); ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c:389:17: error: 'L_PTE_MT_DEV_SHARED' undeclared (first use in this function) L_PTE_XN | L_PTE_MT_DEV_SHARED | L_PTE_SHARED); ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c:389:39: error: 'L_PTE_SHARED' undeclared (first use in this function) L_PTE_XN | L_PTE_MT_DEV_SHARED | L_PTE_SHARED); ^ ./arch/arm/include/asm/page-nommu.h:41:26: note: in definition of macro '__pgprot' #define __pgprot(x) (x) ^ drivers/pci/host/pci-tegra.c: At top level: drivers/pci/host/pci-tegra.c:501:10: error: 'pci_generic_config_read32' undeclared here (not in a function) .read = pci_generic_config_read32, ^ drivers/pci/host/pci-tegra.c:502:11: error: 'pci_generic_config_write32' undeclared here (not in a function) .write = pci_generic_config_write32, ^ drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_relax_enable': drivers/pci/host/pci-tegra.c:609:2: error: implicit declaration of function 'pcie_capability_set_word' [-Werror=implicit-function-declaration] pcie_capability_set_word(dev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_RELAX_EN); ^ drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_setup': drivers/pci/host/pci-tegra.c:626:8: error: implicit declaration of function 'pci_remap_iospace' [-Werror=implicit-function-declaration] err = pci_remap_iospace(&pcie->pio, pcie->io.start); ^ drivers/pci/host/pci-tegra.c:628:3: error: implicit declaration of function 'pci_add_resource_offset' [-Werror=implicit-function-declaration] pci_add_resource_offset(&sys->resources, &pcie->pio, ^ drivers/pci/host/pci-tegra.c:634:2: error: implicit declaration of function 'pci_add_resource' [-Werror=implicit-function-declaration] pci_add_resource(&sys->resources, &pcie->busn); ^ drivers/pci/host/pci-tegra.c:636:8: error: implicit declaration of function 'devm_request_pci_bus_resources' [-Werror=implicit-function-declaration] err = devm_request_pci_bus_resources(dev, &sys->resources); ^ cc1: some warnings being treated as errors make[1]: *** [drivers/pci/host/pci-tegra.o] Error 1 make: *** [drivers/pci/host/pci-tegra.o] Error 2 Cheers Vladimir > > Arnd > >
On Tuesday, November 22, 2016 9:40:39 AM CET Vladimir Murzin wrote: > On 22/11/16 09:31, Arnd Bergmann wrote: > > On Tuesday, November 22, 2016 9:26:01 AM CET Vladimir Murzin wrote: > >> This driver uses functionality which available for MMU build only, > >> thus add dependency on MMU. > >> > >> Cc: Thierry Reding <thierry.reding@gmail.com> > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > >> > > > > Can you be more specific about what requires the MMU here? > > > > Is it the I/O space remapping or something else? > > Yes it comes from I/O space remapping. > > The fill error log: > > CC drivers/pci/host/pci-tegra.o > In file included from ./arch/arm/include/asm/page.h:22:0, > from ./arch/arm/include/asm/thread_info.h:17, > from ./include/linux/thread_info.h:58, > from ./include/asm-generic/current.h:4, > from ./arch/arm/include/generated/asm/current.h:1, > from ./include/linux/mutex.h:13, > from ./include/linux/notifier.h:13, > from ./include/linux/clk.h:17, > from drivers/pci/host/pci-tegra.c:29: > drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_bus_alloc': > drivers/pci/host/pci-tegra.c:388:27: error: 'L_PTE_PRESENT' undeclared (first use in this function) > pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | That is not the I/O space, that is the config space. Not sure what a better alternative would be, but the manual selection of page flags makes the driver non-portable and dependent on architecture specifics that it really shouldn't have to worry about. In common PCI code, we use pgprot_device(PAGE_KERNEL)) at some point, and that sounds like the right thing to do, but ARM doesn't provide an override for it and the fallback is pgprot_noncached(), which is probably wrong here. Unless someone has a good idea for how to change the driver, just mention this in the changelog. Arnd
On 22/11/16 09:54, Arnd Bergmann wrote: > On Tuesday, November 22, 2016 9:40:39 AM CET Vladimir Murzin wrote: >> On 22/11/16 09:31, Arnd Bergmann wrote: >>> On Tuesday, November 22, 2016 9:26:01 AM CET Vladimir Murzin wrote: >>>> This driver uses functionality which available for MMU build only, >>>> thus add dependency on MMU. >>>> >>>> Cc: Thierry Reding <thierry.reding@gmail.com> >>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >>>> >>> >>> Can you be more specific about what requires the MMU here? >>> >>> Is it the I/O space remapping or something else? >> >> Yes it comes from I/O space remapping. >> >> The fill error log: >> >> CC drivers/pci/host/pci-tegra.o >> In file included from ./arch/arm/include/asm/page.h:22:0, >> from ./arch/arm/include/asm/thread_info.h:17, >> from ./include/linux/thread_info.h:58, >> from ./include/asm-generic/current.h:4, >> from ./arch/arm/include/generated/asm/current.h:1, >> from ./include/linux/mutex.h:13, >> from ./include/linux/notifier.h:13, >> from ./include/linux/clk.h:17, >> from drivers/pci/host/pci-tegra.c:29: >> drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_bus_alloc': >> drivers/pci/host/pci-tegra.c:388:27: error: 'L_PTE_PRESENT' undeclared (first use in this function) >> pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | > > > That is not the I/O space, that is the config space. Sorry for that, I'm not in PCI :( > > Not sure what a better alternative would be, but the manual selection > of page flags makes the driver non-portable and dependent on architecture > specifics that it really shouldn't have to worry about. > > In common PCI code, we use pgprot_device(PAGE_KERNEL)) at some point, > and that sounds like the right thing to do, but ARM doesn't provide > an override for it and the fallback is pgprot_noncached(), which is > probably wrong here. > > Unless someone has a good idea for how to change the driver, just > mention this in the changelog. Thanks for explanation! I'll wait for Thierry's thoughts on this. Cheers Vladimir > > Arnd >
On Tue, Nov 22, 2016 at 10:54:01AM +0100, Arnd Bergmann wrote: > On Tuesday, November 22, 2016 9:40:39 AM CET Vladimir Murzin wrote: > > On 22/11/16 09:31, Arnd Bergmann wrote: > > > On Tuesday, November 22, 2016 9:26:01 AM CET Vladimir Murzin wrote: > > >> This driver uses functionality which available for MMU build only, > > >> thus add dependency on MMU. > > >> > > >> Cc: Thierry Reding <thierry.reding@gmail.com> > > >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > > >> > > > > > > Can you be more specific about what requires the MMU here? > > > > > > Is it the I/O space remapping or something else? > > > > Yes it comes from I/O space remapping. > > > > The fill error log: > > > > CC drivers/pci/host/pci-tegra.o > > In file included from ./arch/arm/include/asm/page.h:22:0, > > from ./arch/arm/include/asm/thread_info.h:17, > > from ./include/linux/thread_info.h:58, > > from ./include/asm-generic/current.h:4, > > from ./arch/arm/include/generated/asm/current.h:1, > > from ./include/linux/mutex.h:13, > > from ./include/linux/notifier.h:13, > > from ./include/linux/clk.h:17, > > from drivers/pci/host/pci-tegra.c:29: > > drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_bus_alloc': > > drivers/pci/host/pci-tegra.c:388:27: error: 'L_PTE_PRESENT' undeclared (first use in this function) > > pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | > > > That is not the I/O space, that is the config space. > > Not sure what a better alternative would be, but the manual selection > of page flags makes the driver non-portable and dependent on architecture > specifics that it really shouldn't have to worry about. > > In common PCI code, we use pgprot_device(PAGE_KERNEL)) at some point, > and that sounds like the right thing to do, but ARM doesn't provide > an override for it and the fallback is pgprot_noncached(), which is > probably wrong here. Actually I think pgprot_noncached() is correct. Very early on we used to map this using ioremap() and I remember that working. I also just tested the pci-tegra driver with pgprot_device(PAGE_KERNEL) instead of the ARM- specific flags and it seems to work well. Thierry
On 11/22/2016 02:26 AM, Vladimir Murzin wrote: > This driver uses functionality which available for MMU build only, > thus add dependency on MMU. I'd expect ARCH_TEGRA to depend on MMU instead.
On Tuesday, November 22, 2016 9:15:13 AM CET Stephen Warren wrote: > On 11/22/2016 02:26 AM, Vladimir Murzin wrote: > > This driver uses functionality which available for MMU build only, > > thus add dependency on MMU. > > I'd expect ARCH_TEGRA to depend on MMU instead. > Any particular reason for why we'd single out Tegra? Arnd
On 11/22/2016 02:16 PM, Arnd Bergmann wrote: > On Tuesday, November 22, 2016 9:15:13 AM CET Stephen Warren wrote: >> On 11/22/2016 02:26 AM, Vladimir Murzin wrote: >>> This driver uses functionality which available for MMU build only, >>> thus add dependency on MMU. >> >> I'd expect ARCH_TEGRA to depend on MMU instead. >> > > Any particular reason for why we'd single out Tegra? From the cover letter, it sounded like the point of the series was to all !MMU builds for a bunch of SoCs with M-class cores. Tegra doesn't have an M-class core so I wouldn't expect it to be included in such a build. I'd expect the same fix that I suggested to apply to all non-M-class SoCs, rather than fixing up individual drivers to depend on !MMU.
On Tuesday, November 22, 2016 3:27:35 PM CET Stephen Warren wrote: > On 11/22/2016 02:16 PM, Arnd Bergmann wrote: > > On Tuesday, November 22, 2016 9:15:13 AM CET Stephen Warren wrote: > >> On 11/22/2016 02:26 AM, Vladimir Murzin wrote: > >>> This driver uses functionality which available for MMU build only, > >>> thus add dependency on MMU. > >> > >> I'd expect ARCH_TEGRA to depend on MMU instead. > >> > > > > Any particular reason for why we'd single out Tegra? > > From the cover letter, it sounded like the point of the series was to > all !MMU builds for a bunch of SoCs with M-class cores. Tegra doesn't > have an M-class core so I wouldn't expect it to be included in such a > build. I'd expect the same fix that I suggested to apply to all > non-M-class SoCs, rather than fixing up individual drivers to depend on > !MMU. I think you misread that, the point is to get NOMMU support back for machines other than ARMv7-M, the Cortex-M based machines already work without this. Arnd
On 22/11/16 11:26, Thierry Reding wrote: > On Tue, Nov 22, 2016 at 10:54:01AM +0100, Arnd Bergmann wrote: >> On Tuesday, November 22, 2016 9:40:39 AM CET Vladimir Murzin wrote: >>> On 22/11/16 09:31, Arnd Bergmann wrote: >>>> On Tuesday, November 22, 2016 9:26:01 AM CET Vladimir Murzin wrote: >>>>> This driver uses functionality which available for MMU build only, >>>>> thus add dependency on MMU. >>>>> >>>>> Cc: Thierry Reding <thierry.reding@gmail.com> >>>>> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >>>>> >>>> >>>> Can you be more specific about what requires the MMU here? >>>> >>>> Is it the I/O space remapping or something else? >>> >>> Yes it comes from I/O space remapping. >>> >>> The fill error log: >>> >>> CC drivers/pci/host/pci-tegra.o >>> In file included from ./arch/arm/include/asm/page.h:22:0, >>> from ./arch/arm/include/asm/thread_info.h:17, >>> from ./include/linux/thread_info.h:58, >>> from ./include/asm-generic/current.h:4, >>> from ./arch/arm/include/generated/asm/current.h:1, >>> from ./include/linux/mutex.h:13, >>> from ./include/linux/notifier.h:13, >>> from ./include/linux/clk.h:17, >>> from drivers/pci/host/pci-tegra.c:29: >>> drivers/pci/host/pci-tegra.c: In function 'tegra_pcie_bus_alloc': >>> drivers/pci/host/pci-tegra.c:388:27: error: 'L_PTE_PRESENT' undeclared (first use in this function) >>> pgprot_t prot = __pgprot(L_PTE_PRESENT | L_PTE_YOUNG | L_PTE_DIRTY | >> >> >> That is not the I/O space, that is the config space. >> >> Not sure what a better alternative would be, but the manual selection >> of page flags makes the driver non-portable and dependent on architecture >> specifics that it really shouldn't have to worry about. >> >> In common PCI code, we use pgprot_device(PAGE_KERNEL)) at some point, >> and that sounds like the right thing to do, but ARM doesn't provide >> an override for it and the fallback is pgprot_noncached(), which is >> probably wrong here. > > Actually I think pgprot_noncached() is correct. Very early on we used to > map this using ioremap() and I remember that working. I also just tested > the pci-tegra driver with pgprot_device(PAGE_KERNEL) instead of the ARM- > specific flags and it seems to work well. > I did try that, but tegra_pcie_bus_alloc() calls get_vm_area() so drivers/built-in.o: In function `tegra_pcie_add_bus': :(.text+0x7763c): undefined reference to `get_vm_area' make: *** [vmlinux] Error 1 Cheers Vladimir > Thierry >
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index d7e7c0a..836fa02 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -69,7 +69,7 @@ config PCI_IMX6 config PCI_TEGRA bool "NVIDIA Tegra PCIe controller" - depends on ARCH_TEGRA && !ARM64 + depends on ARCH_TEGRA && !ARM64 && MMU help Say Y here if you want support for the PCIe host controller found on NVIDIA Tegra SoCs.
This driver uses functionality which available for MMU build only, thus add dependency on MMU. Cc: Thierry Reding <thierry.reding@gmail.com> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> --- drivers/pci/host/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)