Thread (42 messages) 42 messages, 5 authors, 2016-03-03

Re: [rtc-linux] Re: [RFC 15/15] mfd: syscon: Fix build of missing ioremap on UM

From: Krzysztof Kozlowski <hidden>
Date: 2016-03-03 12:24:15
Also in: linux-arm-kernel, linux-arm-msm, linux-gpio, linux-media, linux-pm, linux-samsung-soc, lkml, netdev

2016-03-03 19:50 GMT+09:00 Arnd Bergmann [off-list ref]:
On Thursday 03 March 2016 17:03:41 Krzysztof Kozlowski wrote:
quoted
Since commit c89c0114955a ("mfd: syscon: Set regmap max_register in
of_syscon_register") the syscon uses ioremap so it fails on COMPILE_TEST
without HAS_IOMEM:

drivers/mfd/syscon.c: In function =E2=80=98of_syscon_register=E2=80=99:
drivers/mfd/syscon.c:67:9: error: implicit declaration of function =E2=
=80=98ioremap=E2=80=99 [-Werror=3Dimplicit-function-declaration]
quoted
  base =3D ioremap(res.start, resource_size(&res));
         ^
drivers/mfd/syscon.c:67:7: warning: assignment makes pointer from intege=
r without a cast [-Wint-conversion]
quoted
  base =3D ioremap(res.start, resource_size(&res));
       ^
drivers/mfd/syscon.c:109:2: error: implicit declaration of function =E2=
=80=98iounmap=E2=80=99 [-Werror=3Dimplicit-function-declaration]
quoted
  iounmap(base);

When selecting MFD_SYSCON, depend on HAS_IOMEM to avoid unmet direct
dependencies.

Reported-by: kbuild test robot <redacted>
Fixes: c89c0114955a ("mfd: syscon: Set regmap max_register in of_syscon_=
register")
quoted
Signed-off-by: Krzysztof Kozlowski <redacted>
---
Thanks for looking into this, the patches all look right to me, but
I fear we are forever playing catch-up here, as the number of syscon user=
s
is only growing, and it's not obvious to the average driver developer
why they have to select this symbol.
Actually I screwed something because entire MFD menuconfig (including
MFD_SYSCON) is already guarded by if HAS_IOMEM.

I was fixing the problem from the end (the build error) and then hit
these unmet direct dependencies (mentioned this in other mail).

I think this patch 15/15 is not needed.
Interestingly, when I try to build an allmodconfig kernel for UML,
it seems to reject any driver calling ioremap/iounmap but not
the wrapper functions around that (of_iomap, devm_ioremap,
devm_ioremap_resource, ...)
The declaration of devm-like functions is always there. However it
should fail during linking stage (see my other patches like for
thermal, nvram etc. this is still in progress because I did not manage
to build allyesconfig on UML yet).
or the actual accessors that make no
sense without ioremap (readl, writel, inb, outb, iowrite32, ...).

I've played with this a bit, and arrived at the patch below, is this
something we want as well?

        Arnd
quoted
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index aa21dc55eb15..2e5b1e525a1d 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -1034,6 +1034,7 @@ config MFD_SUN6I_PRCM

 config MFD_SYSCON
      bool "System Controller Register R/W Based on Regmap"
+     depends on HAS_IOMEM
      select REGMAP_MMIO
      help
        Select this option to enable accessing system control registers

 arch/um/include/asm/io.h           |    1 +
 drivers/char/Kconfig               |    3 +++
 drivers/char/mem.c                 |   16 ++++++++++++++++
 drivers/clocksource/Kconfig        |    3 +++
 drivers/fmc/Kconfig                |    1 +
 drivers/fpga/Kconfig               |    1 +
 drivers/hwtracing/intel_th/Kconfig |    1 +
 drivers/mfd/Kconfig                |    1 +
 drivers/misc/altera-stapl/Kconfig  |    2 +-
 drivers/mtd/chips/Kconfig          |    4 ++++
 drivers/mtd/lpddr/Kconfig          |    1 +
 drivers/mtd/maps/Kconfig           |    3 ++-
 drivers/mtd/nand/Kconfig           |    2 +-
 drivers/mtd/spi-nor/Kconfig        |    1 +
 drivers/net/can/Kconfig            |    1 +
 drivers/net/hamradio/Kconfig       |    9 ++++++---
 drivers/nvmem/Kconfig              |    1 +
 drivers/phy/Kconfig                |    9 +++++++--
 drivers/power/reset/Kconfig        |    4 +++-
 drivers/staging/comedi/Kconfig     |    6 +++++-
 drivers/thermal/Kconfig            |   11 +++++++++--
For some of these I already started fixing:
https://lkml.org/lkml/2016/3/3/147
http://comments.gmane.org/gmane.linux.kernel/2167664

quoted hunk ↗ jump to hunk
 include/linux/irq.h                |    2 ++
 include/linux/mtd/cfi.h            |    2 ++
 include/linux/mtd/map.h            |    2 ++
 kernel/resource.c                  |    2 ++
 lib/Kconfig                        |    7 ++++++-
 mm/bootmem.c                       |    4 ++--
 27 files changed, 85 insertions(+), 15 deletions(-)
diff --git a/arch/um/include/asm/io.h b/arch/um/include/asm/io.h
new file mode 100644
index 0000000..618ff13
--- /dev/null
+++ b/arch/um/include/asm/io.h
@@ -0,0 +1 @@
+/* no IOPORT or IOMEM suport */
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index a043107..f6dc17a 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -8,6 +8,7 @@ source "drivers/tty/Kconfig"

 config DEVMEM
        bool "/dev/mem virtual device support"
+       depends on HAS_IOMEM
        default y
        help
          Say Y here if you want to support the /dev/mem device.
@@ -17,6 +18,7 @@ config DEVMEM

 config DEVKMEM
        bool "/dev/kmem virtual device support"
+       depends on HAS_IOMEM
        default y
        help
          Say Y here if you want to support the /dev/kmem device. The
@@ -94,6 +96,7 @@ config BFIN_OTP_WRITE_ENABLE
 config PRINTER
        tristate "Parallel printer support"
        depends on PARPORT
+       depends on HAS_IOPORT
        ---help---
          If you intend to attach a printer to the parallel port of your =
Linux
          box (as opposed to using a serial printer; if the connector at =
the
quoted hunk ↗ jump to hunk
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 4f6f94c..eedd129 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -91,6 +91,7 @@ void __weak unxlate_dev_mem_ptr(phys_addr_t phys, void =
*addr)
 }
 #endif

+#ifdef CONFIG_DEVMEM
 /*
  * This funcion reads the *physical* memory. The f_pos points directly t=
o the
quoted hunk ↗ jump to hunk
  * memory location.
@@ -219,6 +220,7 @@ static ssize_t write_mem(struct file *file, const cha=
r __user *buf,
quoted hunk ↗ jump to hunk
        *ppos +=3D written;
        return written;
 }
+#endif

 int __weak phys_mem_access_prot_allowed(struct file *file,
        unsigned long pfn, unsigned long size, pgprot_t *vma_prot)
@@ -312,6 +314,7 @@ static inline int private_mapping_ok(struct vm_area_s=
truct *vma)
quoted hunk ↗ jump to hunk
 }
 #endif

+#ifdef CONFIG_DEVMEM
 static const struct vm_operations_struct mmap_mem_ops =3D {
 #ifdef CONFIG_HAVE_IOREMAP_PROT
        .access =3D generic_access_phys
@@ -351,7 +354,9 @@ static int mmap_mem(struct file *file, struct vm_area=
_struct *vma)
quoted hunk ↗ jump to hunk
        }
        return 0;
 }
+#endif

+#ifdef CONFIG_DEVKMEM
 static int mmap_kmem(struct file *file, struct vm_area_struct *vma)
 {
        unsigned long pfn;
@@ -552,7 +557,9 @@ static ssize_t write_kmem(struct file *file, const ch=
ar __user *buf,
quoted hunk ↗ jump to hunk
        *ppos =3D p;
        return virtr + wrote ? : err;
 }
+#endif

+#ifdef CONFIG_DEVPORT
 static ssize_t read_port(struct file *file, char __user *buf,
                         size_t count, loff_t *ppos)
 {
@@ -594,6 +601,7 @@ static ssize_t write_port(struct file *file, const ch=
ar __user *buf,
quoted hunk ↗ jump to hunk
        *ppos =3D i;
        return tmp-buf;
 }
+#endif

 static ssize_t read_null(struct file *file, char __user *buf,
                         size_t count, loff_t *ppos)
@@ -710,10 +718,12 @@ static loff_t memory_lseek(struct file *file, loff_=
t offset, int orig)
quoted hunk ↗ jump to hunk
        return ret;
 }

+#ifdef CONFIG_DEVPORT
 static int open_port(struct inode *inode, struct file *filp)
 {
        return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
 }
+#endif

 #define zero_lseek     null_lseek
 #define full_lseek      null_lseek
@@ -722,6 +732,7 @@ static int open_port(struct inode *inode, struct file=
 *filp)
quoted hunk ↗ jump to hunk
 #define open_mem       open_port
 #define open_kmem      open_mem

+#ifdef CONFIG_DEVMEM
 static const struct file_operations __maybe_unused mem_fops =3D {
        .llseek         =3D memory_lseek,
        .read           =3D read_mem,
@@ -733,7 +744,9 @@ static const struct file_operations __maybe_unused me=
m_fops =3D {
quoted hunk ↗ jump to hunk
        .mmap_capabilities =3D memory_mmap_capabilities,
 #endif
 };
+#endif

+#ifdef CONFIG_DEVKMEM
 static const struct file_operations __maybe_unused kmem_fops =3D {
        .llseek         =3D memory_lseek,
        .read           =3D read_kmem,
@@ -745,6 +758,7 @@ static const struct file_operations __maybe_unused km=
em_fops =3D {
quoted hunk ↗ jump to hunk
        .mmap_capabilities =3D memory_mmap_capabilities,
 #endif
 };
+#endif

 static const struct file_operations null_fops =3D {
        .llseek         =3D null_lseek,
@@ -755,12 +769,14 @@ static const struct file_operations null_fops =3D {
        .splice_write   =3D splice_write_null,
 };

+#ifdef CONFIG_DEVPORT
 static const struct file_operations __maybe_unused port_fops =3D {
        .llseek         =3D memory_lseek,
        .read           =3D read_port,
        .write          =3D write_port,
        .open           =3D open_port,
 };
+#endif

 static const struct file_operations zero_fops =3D {
        .llseek         =3D zero_lseek,
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 33db740..6b58974 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -38,6 +38,7 @@ config DIGICOLOR_TIMER
 config DW_APB_TIMER
        bool "DW APB timer driver" if COMPILE_TEST
        depends on GENERIC_CLOCKEVENTS
+       depends on HAS_IOMEM
        help
          Enables the support for the dw_apb timer.
@@ -64,6 +65,7 @@ config ARMADA_370_XP_TIMER
 config MESON6_TIMER
        bool "Meson6 timer driver" if COMPILE_TEST
        depends on GENERIC_CLOCKEVENTS
+       depends on HAS_IOMEM
        select CLKSRC_MMIO
        help
          Enables the support for the Meson6 timer driver.
@@ -114,6 +116,7 @@ config CADENCE_TTC_TIMER
 config ASM9260_TIMER
        bool "ASM9260 timer driver" if COMPILE_TEST
        depends on GENERIC_CLOCKEVENTS
+       depends on HAS_IOMEM
        select CLKSRC_MMIO
        select CLKSRC_OF
        help
diff --git a/drivers/fmc/Kconfig b/drivers/fmc/Kconfig
index 3a75f42..adf1391 100644
--- a/drivers/fmc/Kconfig
+++ b/drivers/fmc/Kconfig
@@ -4,6 +4,7 @@

 menuconfig FMC
        tristate "FMC support"
+       depends on HAS_IOMEM
        help

          FMC (FPGA Mezzanine Carrier) is a mechanical and electrical
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index c9b9fdf..36c54af 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -21,6 +21,7 @@ config FPGA_MGR_SOCFPGA

 config FPGA_MGR_ZYNQ_FPGA
        tristate "Xilinx Zynq FPGA"
+       depends on HAS_IOMEM
        help
          FPGA manager driver support for Xilinx Zynq FPGAs.
diff --git a/drivers/hwtracing/intel_th/Kconfig b/drivers/hwtracing/intel=
_th/Kconfig
quoted hunk ↗ jump to hunk
index b7a9073..467dae9 100644
--- a/drivers/hwtracing/intel_th/Kconfig
+++ b/drivers/hwtracing/intel_th/Kconfig
@@ -1,5 +1,6 @@
 config INTEL_TH
        tristate "Intel(R) Trace Hub controller"
+       depends on HAS_IOMEM
        help
          Intel(R) Trace Hub (TH) is a set of hardware blocks (subdevices=
) that
          produce, switch and output trace data from multiple hardware an=
d
quoted hunk ↗ jump to hunk
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 9ca66de..6ff6246 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -999,6 +999,7 @@ config MFD_SUN6I_PRCM

 config MFD_SYSCON
        bool "System Controller Register R/W Based on Regmap"
+       depends on HAS_IOMEM
This shouldn't be needed. There is if HAS_IOMEM at beginning.

Best regards,
Krzysztof

--=20
--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help