| Message ID | 1569945867-82243-4-git-send-email-john.garry@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hisi_lpc: Improve build test coverage | expand |
On Tue, Oct 1, 2019 at 6:07 PM John Garry <john.garry@huawei.com> wrote: > > Currently the driver will only ever be built for ARM64 because it selects > CONFIG_INDIRECT_PIO, which itself depends on ARM64. > > Expand build test coverage for the driver to other architectures by only > selecting CONFIG_INDIRECT_PIO for ARM64, when we really want it. > > Signed-off-by: John Garry <john.garry@huawei.com> Good idea, but doesn't this cause a link failure against logic_pio_register_range() when INDIRECT_PIO is disabled? Arnd
On 02/10/2019 16:43, Arnd Bergmann wrote: > On Tue, Oct 1, 2019 at 6:07 PM John Garry <john.garry@huawei.com> wrote: >> >> Currently the driver will only ever be built for ARM64 because it selects >> CONFIG_INDIRECT_PIO, which itself depends on ARM64. >> >> Expand build test coverage for the driver to other architectures by only >> selecting CONFIG_INDIRECT_PIO for ARM64, when we really want it. >> >> Signed-off-by: John Garry <john.garry@huawei.com> > Hi Arnd, > Good idea, but doesn't this cause a link failure against > logic_pio_register_range() when INDIRECT_PIO is disabled? No, it shouldn't do. Function lib/logic_pio.c::logic_pio_register_range() is built always, outside any INDIRECT_PIO checking. A some stage, for completeness we should probably change logic_pio_register_range() and friends to be under PCI_IOBASE. But then we would need stubs for !PCI_IOBASE, due to this above change and also references from the device tree code. I'd have to consider this a bit more. Let me know what you think. Thanks, John > > Arnd > >
On Wed, Oct 2, 2019 at 6:25 PM John Garry <john.garry@huawei.com> wrote: > On 02/10/2019 16:43, Arnd Bergmann wrote: > > On Tue, Oct 1, 2019 at 6:07 PM John Garry <john.garry@huawei.com> wrote: > >> > >> Currently the driver will only ever be built for ARM64 because it selects > >> CONFIG_INDIRECT_PIO, which itself depends on ARM64. > >> > >> Expand build test coverage for the driver to other architectures by only > >> selecting CONFIG_INDIRECT_PIO for ARM64, when we really want it. > >> > >> Signed-off-by: John Garry <john.garry@huawei.com> > > > > Hi Arnd, > > > Good idea, but doesn't this cause a link failure against > > logic_pio_register_range() when INDIRECT_PIO is disabled? > > No, it shouldn't do. Function > lib/logic_pio.c::logic_pio_register_range() is built always, outside any > INDIRECT_PIO checking. Ok, I see. > A some stage, for completeness we should probably change > logic_pio_register_range() and friends to be under PCI_IOBASE. But then > we would need stubs for !PCI_IOBASE, due to this above change and also > references from the device tree code. I'd have to consider this a bit > more. Let me know what you think. It's probably not to do this with the usual 'static inline' stubs in the header files. There is no rush here, but it would be nice to not build this code when it is not needed. I wonder if this one-line change would take care of the !CONFIG_OF case already (it probably doesn't): --- a/lib/Makefile +++ b/lib/Makefile @@ -107,7 +107,7 @@ obj-$(CONFIG_HAS_IOMEM) += iomap_copy.o devres.o obj-$(CONFIG_CHECK_SIGNATURE) += check_signature.o obj-$(CONFIG_DEBUG_LOCKING_API_SELFTESTS) += locking-selftest.o -obj-y += logic_pio.o +lib-y += logic_pio.o obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o On a related note: At some point we may want to add an indirect method for readl()/writel(), to deal with some of the weirder 32-bit ARM platforms. We'll have to see how well this can fit into the infrastructure we already have for indirect PIO. Arnd
On 02/10/2019 19:22, Arnd Bergmann wrote: > On Wed, Oct 2, 2019 at 6:25 PM John Garry <john.garry@huawei.com> wrote: >> On 02/10/2019 16:43, Arnd Bergmann wrote: >>> On Tue, Oct 1, 2019 at 6:07 PM John Garry <john.garry@huawei.com> wrote: >>>> >>>> Currently the driver will only ever be built for ARM64 because it selects >>>> CONFIG_INDIRECT_PIO, which itself depends on ARM64. >>>> >>>> Expand build test coverage for the driver to other architectures by only >>>> selecting CONFIG_INDIRECT_PIO for ARM64, when we really want it. >>>> >>>> Signed-off-by: John Garry <john.garry@huawei.com> >>> >> >> Hi Arnd, >> >>> Good idea, but doesn't this cause a link failure against >>> logic_pio_register_range() when INDIRECT_PIO is disabled? >> >> No, it shouldn't do. Function >> lib/logic_pio.c::logic_pio_register_range() is built always, outside any >> INDIRECT_PIO checking. > > Ok, I see. > >> A some stage, for completeness we should probably change >> logic_pio_register_range() and friends to be under PCI_IOBASE. But then >> we would need stubs for !PCI_IOBASE, due to this above change and also >> references from the device tree code. I'd have to consider this a bit >> more. Let me know what you think. > > It's probably not to do this with the usual 'static inline' stubs in the > header files. There is no rush here, but it would be nice to not build > this code when it is not needed. Agreed, I'll add it to my todo list. So we want to build logic_pio.c for archs which define PCI_IOBASE, but there is no specific config option for that. > > I wonder if this one-line change would take care of the !CONFIG_OF > case already (it probably doesn't): > > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -107,7 +107,7 @@ obj-$(CONFIG_HAS_IOMEM) += iomap_copy.o devres.o > obj-$(CONFIG_CHECK_SIGNATURE) += check_signature.o > obj-$(CONFIG_DEBUG_LOCKING_API_SELFTESTS) += locking-selftest.o > > -obj-y += logic_pio.o > +lib-y += logic_pio.o So that means that this obj will not be included in the vmlinux file for !CONFIG_OF (for archs with !PCI_IOBASE or !PCI). It is an improvement, but I still would like to make the complete change and not to build at all for archs which don't define PCI_IOBASE. > > obj-$(CONFIG_GENERIC_HWEIGHT) += hweight.o > > On a related note: At some point we may want to add an indirect > method for readl()/writel(), to deal with some of the weirder 32-bit > ARM platforms. We'll have to see how well this can fit into the > infrastructure we already have for indirect PIO. It should be possible. We would just need to manage 2 separate logical spaces instead of one: a. mmio b. pio I'm guessing that all the code for logical <-> hw bus address translations would be the same. Thanks, John > > Arnd > > . >
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig index 6b331061d34b..44cb4b6bea18 100644 --- a/drivers/bus/Kconfig +++ b/drivers/bus/Kconfig @@ -41,8 +41,8 @@ config MOXTET config HISILICON_LPC bool "Support for ISA I/O space on HiSilicon Hip06/7" - depends on ARM64 && (ARCH_HISI || COMPILE_TEST) - select INDIRECT_PIO + depends on (ARM64 && ARCH_HISI) || COMPILE_TEST + select INDIRECT_PIO if ARM64 help Driver to enable I/O access to devices attached to the Low Pin Count bus on the HiSilicon Hip06/7 SoC.
Currently the driver will only ever be built for ARM64 because it selects CONFIG_INDIRECT_PIO, which itself depends on ARM64. Expand build test coverage for the driver to other architectures by only selecting CONFIG_INDIRECT_PIO for ARM64, when we really want it. Signed-off-by: John Garry <john.garry@huawei.com> --- drivers/bus/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.17.1