Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1650600 > unrolled thread

[PATCH v2] um: add dummy ioremap and iounmap functions

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] um: add dummy ioremap and iounmap functions Logan Gunthorpe <logang@deltatee.com> - 2017-05-25 17:50 +0200
    Re: [PATCH v2] um: add dummy ioremap and iounmap functions Richard Weinberger <richard@nod.at> - 2017-05-25 17:50 +0200
      Re: [PATCH v2] um: add dummy ioremap and iounmap functions Logan Gunthorpe <logang@deltatee.com> - 2017-05-25 18:00 +0200
        Re: [PATCH v2] um: add dummy ioremap and iounmap functions Geert Uytterhoeven <geert@linux-m68k.org> - 2017-05-27 20:10 +0200
          Re: [PATCH v2] um: add dummy ioremap and iounmap functions Logan Gunthorpe <logang@deltatee.com> - 2017-05-27 20:20 +0200

#1650600 — [PATCH v2] um: add dummy ioremap and iounmap functions

The user mode architecture does not provide ioremap or iounmap, and
because of this, the arch won't build when the functions are used in some
core libraries.

I have designs to use these functions in scatterlist.c where they'd
almost certainly never be called on the um architecture but it does need
to compile. Thus, if the function is ever hit it returns NULL.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Stephen Bates <sbates@raithlin.com>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Richard Weinberger <richard@nod.at>
Cc: Al Viro <viro@ZenIV.linux.org.uk>
---

Changes for v2:

* Per feedback from Al Viro, the ioremap function was changed to
  fail if it ever actually gets used.

 arch/um/include/asm/io.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 arch/um/include/asm/io.h

diff --git a/arch/um/include/asm/io.h b/arch/um/include/asm/io.h
new file mode 100644
index 0000000..f734673
--- /dev/null
+++ b/arch/um/include/asm/io.h
@@ -0,0 +1,17 @@
+#ifndef _ASM_UM_IO_H
+#define _ASM_UM_IO_H
+
+#define ioremap ioremap
+static inline void __iomem *ioremap(phys_addr_t offset, size_t size)
+{
+	return NULL;
+}
+
+#define iounmap iounmap
+static inline void iounmap(void __iomem *addr)
+{
+}
+
+#include <asm-generic/io.h>
+
+#endif
--
2.1.4

[toc] | [next] | [standalone]

#1650601

Logan,

Am 25.05.2017 um 17:42 schrieb Logan Gunthorpe:
> The user mode architecture does not provide ioremap or iounmap, and
> because of this, the arch won't build when the functions are used in some
> core libraries.

Which ones are failing?
I thought we killed the problem by making CONFIG_COMPILE_TEST depend on !UML.

> I have designs to use these functions in scatterlist.c where they'd
> almost certainly never be called on the um architecture but it does need
> to compile. Thus, if the function is ever hit it returns NULL.

I was never a fan of that approach because in my opinion drivers should have
proper dependencies, including a dependency on HAS_IOMEM.

But I'll no longer block these attempts if we can get rid of tons of fallout
every kernel release.

Thanks,
//richard

[toc] | [prev] | [next] | [standalone]

#1650611

On 25/05/17 09:48 AM, Richard Weinberger wrote:
> Which ones are failing?
> I thought we killed the problem by making CONFIG_COMPILE_TEST depend on !UML.

None, at the moment. My work is trying to add iomem support to scatter
lists and thus I want to call ioremap in scatterlist.c. That's when
things fail to build. We could put an '#ifdef HAS_IOMEM' around only the
uses, but I thought this approach was cleaner. However, if people would
rather I do that, let me know and I will.

> I was never a fan of that approach because in my opinion drivers should have
> proper dependencies, including a dependency on HAS_IOMEM.

This doesn't really work when we try to use it in a core library which
can't depend on HAS_IOMEM.

Thanks,

Logan

[toc] | [prev] | [next] | [standalone]

#1651951

Hi Logan,

On Thu, May 25, 2017 at 5:53 PM, Logan Gunthorpe <logang@deltatee.com> wrote:
> On 25/05/17 09:48 AM, Richard Weinberger wrote:
>> Which ones are failing?
>> I thought we killed the problem by making CONFIG_COMPILE_TEST depend on !UML.
>
> None, at the moment. My work is trying to add iomem support to scatter
> lists and thus I want to call ioremap in scatterlist.c. That's when
> things fail to build. We could put an '#ifdef HAS_IOMEM' around only the
> uses, but I thought this approach was cleaner. However, if people would
> rather I do that, let me know and I will.
>
>> I was never a fan of that approach because in my opinion drivers should have
>> proper dependencies, including a dependency on HAS_IOMEM.
>
> This doesn't really work when we try to use it in a core library which
> can't depend on HAS_IOMEM.

Still, those code patch could be protected by #ifdef CONFIG_HAS_IOMEM,
or better, if (IS_ENABLED(CONFIG_HAS_IOMEM)).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1651953

Hi,

On 27/05/17 12:08 PM, Geert Uytterhoeven wrote:
> Still, those code patch could be protected by #ifdef CONFIG_HAS_IOMEM,
> or better, if (IS_ENABLED(CONFIG_HAS_IOMEM)).

Well I think it would have to be the former seeing the latter would
still end up trying to compile the missing function. But having ifdefs
inside code is not generally seen as good idea[1].

I'd really like to go forward with either this patch or something like
it. The other two arches that have this problem are fine with merging a
fix and adding ifdefs to work around a single arch doesn't feel right to me.

Thanks,

Logan

[1] http://yarchive.net/comp/linux/ifdefs.html

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web