* [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390
@ 2021-09-03 7:56 Chunyan Zhang
2021-09-03 8:03 ` Chunyan Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Chunyan Zhang @ 2021-09-03 7:56 UTC (permalink / raw)
To: Vasily Gorbik, Heiko Carstens, Christian Borntraeger,
Sebastian Ott, Martin Schwidefsky
Cc: linux-s390, kernel test robot, Thomas Gleixner, Chunyan Zhang,
Chunyan Zhang, LKML
From: Chunyan Zhang <chunyan.zhang@unisoc.com>
There would not be ioremap and iounmap implementations if CONFIG_PCI is
not set for s390, so add default declarations of these two functionsi
for the case to avoid 'undefined reference' issue.
Fixes: 71ba41c9b1d9 ("s390/pci: provide support for MIO instructions")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
---
The issue was reported from https://lkml.org/lkml/2021/8/1/18
---
arch/s390/include/asm/io.h | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index e3882b012bfa..9438370c6445 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -23,11 +23,8 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
#define IO_SPACE_LIMIT 0
void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
-void __iomem *ioremap(phys_addr_t addr, size_t size);
void __iomem *ioremap_wc(phys_addr_t addr, size_t size);
void __iomem *ioremap_wt(phys_addr_t addr, size_t size);
-void iounmap(volatile void __iomem *addr);
-
static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
{
return NULL;
@@ -39,6 +36,9 @@ static inline void ioport_unmap(void __iomem *p)
#ifdef CONFIG_PCI
+void __iomem *ioremap(phys_addr_t addr, size_t size);
+void iounmap(volatile void __iomem *addr);
+
/*
* s390 needs a private implementation of pci_iomap since ioremap with its
* offset parameter isn't sufficient. That's because BAR spaces are not
@@ -70,6 +70,15 @@ static inline void ioport_unmap(void __iomem *p)
#define __raw_writel zpci_write_u32
#define __raw_writeq zpci_write_u64
+#else
+static inline void __iomem *ioremap(unsigned long offset, unsigned long size)
+{
+ return (void __iomem *) offset;
+}
+
+static inline void iounmap(void __iomem *addr)
+{
+}
#endif /* CONFIG_PCI */
#include <asm-generic/io.h>
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390
2021-09-03 7:56 [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390 Chunyan Zhang
@ 2021-09-03 8:03 ` Chunyan Zhang
2021-09-03 8:24 ` Niklas Schnelle
0 siblings, 1 reply; 6+ messages in thread
From: Chunyan Zhang @ 2021-09-03 8:03 UTC (permalink / raw)
To: Vasily Gorbik, Heiko Carstens, Christian Borntraeger,
Sebastian Ott, Martin Schwidefsky
Cc: linux-s390, kernel test robot, Thomas Gleixner, Chunyan Zhang,
Chunyan Zhang, LKML
From: Chunyan Zhang <chunyan.zhang@unisoc.com>
There would not be ioremap and iounmap implementations if CONFIG_PCI is
not set for s390, so add default declarations of these two functions
for the case to avoid 'undefined reference' issue.
Fixes: 71ba41c9b1d9 ("s390/pci: provide support for MIO instructions")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
---
The issue was reported from https://lkml.org/lkml/2021/8/1/18
---
arch/s390/include/asm/io.h | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index e3882b012bfa..9438370c6445 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -23,11 +23,8 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
#define IO_SPACE_LIMIT 0
void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot);
-void __iomem *ioremap(phys_addr_t addr, size_t size);
void __iomem *ioremap_wc(phys_addr_t addr, size_t size);
void __iomem *ioremap_wt(phys_addr_t addr, size_t size);
-void iounmap(volatile void __iomem *addr);
-
static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
{
return NULL;
@@ -39,6 +36,9 @@ static inline void ioport_unmap(void __iomem *p)
#ifdef CONFIG_PCI
+void __iomem *ioremap(phys_addr_t addr, size_t size);
+void iounmap(volatile void __iomem *addr);
+
/*
* s390 needs a private implementation of pci_iomap since ioremap with its
* offset parameter isn't sufficient. That's because BAR spaces are not
@@ -70,6 +70,15 @@ static inline void ioport_unmap(void __iomem *p)
#define __raw_writel zpci_write_u32
#define __raw_writeq zpci_write_u64
+#else
+static inline void __iomem *ioremap(unsigned long offset, unsigned long size)
+{
+ return (void __iomem *) offset;
+}
+
+static inline void iounmap(void __iomem *addr)
+{
+}
#endif /* CONFIG_PCI */
#include <asm-generic/io.h>
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390
2021-09-03 8:03 ` Chunyan Zhang
@ 2021-09-03 8:24 ` Niklas Schnelle
2021-09-03 9:40 ` Chunyan Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Niklas Schnelle @ 2021-09-03 8:24 UTC (permalink / raw)
To: Chunyan Zhang, Vasily Gorbik, Heiko Carstens, Christian Borntraeger
Cc: linux-s390, kernel test robot, Thomas Gleixner, Chunyan Zhang, LKML
On Fri, 2021-09-03 at 16:03 +0800, Chunyan Zhang wrote:
> From: Chunyan Zhang <chunyan.zhang@unisoc.com>
>
> There would not be ioremap and iounmap implementations if CONFIG_PCI is
> not set for s390, so add default declarations of these two functions
> for the case to avoid 'undefined reference' issue.
>
> Fixes: 71ba41c9b1d9 ("s390/pci: provide support for MIO instructions")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> ---
> The issue was reported from https://lkml.org/lkml/2021/8/1/18
Thanks for the patch but I'm a little skeptical about adding
ioremap()/iounmap() stubs that don't do anything useful and on top ofn
that would do so silently.
In the above discussion you said that TIMER_OF should depend on
HAS_IOMEM. In arch/s390/Kconfig HAS_IOMEM is set if and only if
CONFIG_PCI is set so that sounds to me like it would prevent the
undefined reference without the risk of someone trying to use io*map()
without CONFIG_PCI.
At the very least I think the functions should do a WARN_ONCE() but
then we have the same situation as discussed below with Linus making it
pretty clear that he prefers these cases to be compile time checked:
https://lkml.org/lkml/2021/7/2/511
> ---
> arch/s390/include/asm/io.h | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
> index e3882b012bfa..9438370c6445 100644
> --- a/arch/s390/include/asm/io.h
> +++ b/arch/s390/include/asm/io.h
> @@ -23,11 +23,8 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
> #define IO_SPACE_LIMIT 0
>
>
.. snip ..
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390
2021-09-03 8:24 ` Niklas Schnelle
@ 2021-09-03 9:40 ` Chunyan Zhang
2021-09-03 10:28 ` Niklas Schnelle
0 siblings, 1 reply; 6+ messages in thread
From: Chunyan Zhang @ 2021-09-03 9:40 UTC (permalink / raw)
To: Niklas Schnelle
Cc: Vasily Gorbik, Heiko Carstens, Christian Borntraeger, linux-s390,
kernel test robot, Thomas Gleixner, Chunyan Zhang, LKML
On Fri, 3 Sept 2021 at 16:24, Niklas Schnelle <schnelle@linux.ibm.com> wrote:
>
> On Fri, 2021-09-03 at 16:03 +0800, Chunyan Zhang wrote:
> > From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> >
> > There would not be ioremap and iounmap implementations if CONFIG_PCI is
> > not set for s390, so add default declarations of these two functions
> > for the case to avoid 'undefined reference' issue.
> >
> > Fixes: 71ba41c9b1d9 ("s390/pci: provide support for MIO instructions")
> > Reported-by: kernel test robot <lkp@intel.com>
> > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > ---
> > The issue was reported from https://lkml.org/lkml/2021/8/1/18
>
> Thanks for the patch but I'm a little skeptical about adding
> ioremap()/iounmap() stubs that don't do anything useful and on top ofn
> that would do so silently.
>
> In the above discussion you said that TIMER_OF should depend on
> HAS_IOMEM. In arch/s390/Kconfig HAS_IOMEM is set if and only if
> CONFIG_PCI is set so that sounds to me like it would prevent the
> undefined reference without the risk of someone trying to use io*map()
> without CONFIG_PCI.
Humm... you can ignore my reply on that time, I later found that's not
correct :)
TIMER_OF would be selected by other configs and it seems not able to
depends on HAS_IOMEM, unless all configs which select TIMER_OF depend
on HAS_IOMEM, that would be a big change.
Actually HAS_IOMEM is set as default on other architectures, but not
for s390 which redefined it.
>
> At the very least I think the functions should do a WARN_ONCE() but
> then we have the same situation as discussed below with Linus making it
> pretty clear that he prefers these cases to be compile time checked:
Ok, if I understand correctly, if io*map is not implemented for some
case, there should be a *compile-time* error rather than adding a stub
function to make this kind of errors disappeared.
Please correct me if I missed something.
Thanks,
Chunyan
>
> https://lkml.org/lkml/2021/7/2/511
>
>
> > ---
> > arch/s390/include/asm/io.h | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
> > index e3882b012bfa..9438370c6445 100644
> > --- a/arch/s390/include/asm/io.h
> > +++ b/arch/s390/include/asm/io.h
> > @@ -23,11 +23,8 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
> > #define IO_SPACE_LIMIT 0
> >
> >
> .. snip ..
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390
2021-09-03 9:40 ` Chunyan Zhang
@ 2021-09-03 10:28 ` Niklas Schnelle
2021-09-03 10:51 ` Heiko Carstens
0 siblings, 1 reply; 6+ messages in thread
From: Niklas Schnelle @ 2021-09-03 10:28 UTC (permalink / raw)
To: Chunyan Zhang
Cc: Vasily Gorbik, Heiko Carstens, Christian Borntraeger, linux-s390,
kernel test robot, Thomas Gleixner, Chunyan Zhang, LKML
On Fri, 2021-09-03 at 17:40 +0800, Chunyan Zhang wrote:
> On Fri, 3 Sept 2021 at 16:24, Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> > On Fri, 2021-09-03 at 16:03 +0800, Chunyan Zhang wrote:
> > > From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > >
> > > There would not be ioremap and iounmap implementations if CONFIG_PCI is
> > > not set for s390, so add default declarations of these two functions
> > > for the case to avoid 'undefined reference' issue.
> > >
> > > Fixes: 71ba41c9b1d9 ("s390/pci: provide support for MIO instructions")
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > > ---
> > > The issue was reported from https://lkml.org/lkml/2021/8/1/18
> >
> > Thanks for the patch but I'm a little skeptical about adding
> > ioremap()/iounmap() stubs that don't do anything useful and on top ofn
> > that would do so silently.
> >
> > In the above discussion you said that TIMER_OF should depend on
> > HAS_IOMEM. In arch/s390/Kconfig HAS_IOMEM is set if and only if
> > CONFIG_PCI is set so that sounds to me like it would prevent the
> > undefined reference without the risk of someone trying to use io*map()
> > without CONFIG_PCI.
>
> Humm... you can ignore my reply on that time, I later found that's not
> correct :)
>
> TIMER_OF would be selected by other configs and it seems not able to
> depends on HAS_IOMEM, unless all configs which select TIMER_OF depend
> on HAS_IOMEM, that would be a big change.
Hmm I already see a lot of the options in drivers/clocksource/Kconfig
depend on HAS_IOMEM and drivers/clocksource/timer-of.c definitely calls
iounmap() so wouldn't that be the logical change even if it is bigger?
>
> Actually HAS_IOMEM is set as default on other architectures, but not
> for s390 which redefined it.
Yes because most architectures always have IOMEM and io*map() functions
I believe. s390 is an exception here as the mainframe native
functionality all works without MMIO and you can run a fully functional
system including networking and block devices without any MMIO, PCI and
without ioremap()/iounmap().
>
> > At the very least I think the functions should do a WARN_ONCE() but
> > then we have the same situation as discussed below with Linus making it
> > pretty clear that he prefers these cases to be compile time checked:
>
> Ok, if I understand correctly, if io*map is not implemented for some
> case, there should be a *compile-time* error rather than adding a stub
> function to make this kind of errors disappeared.
>
> Please correct me if I missed something.
Ideally not a compile time error but a compile time flag such as a
Kconfig option that would make sure that if HAS_IOMEM isn't set we
don't get drivers compiled which depend on working io*map(). After all
these drivers will surely not be functional.
>
> Thanks,
> Chunyan
>
> > https://lkml.org/lkml/2021/7/2/511
> >
> >
> > > ---
> > > arch/s390/include/asm/io.h | 15 ++++++++++++---
> > > 1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
> > > index e3882b012bfa..9438370c6445 100644
> > > --- a/arch/s390/include/asm/io.h
> > > +++ b/arch/s390/include/asm/io.h
> > > @@ -23,11 +23,8 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr);
> > > #define IO_SPACE_LIMIT 0
> > >
> > >
> > .. snip ..
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390
2021-09-03 10:28 ` Niklas Schnelle
@ 2021-09-03 10:51 ` Heiko Carstens
0 siblings, 0 replies; 6+ messages in thread
From: Heiko Carstens @ 2021-09-03 10:51 UTC (permalink / raw)
To: Niklas Schnelle, Arnd Bergmann
Cc: Chunyan Zhang, Vasily Gorbik, Christian Borntraeger, linux-s390,
kernel test robot, Thomas Gleixner, Chunyan Zhang, LKML
On Fri, Sep 03, 2021 at 12:28:41PM +0200, Niklas Schnelle wrote:
> On Fri, 2021-09-03 at 17:40 +0800, Chunyan Zhang wrote:
> > On Fri, 3 Sept 2021 at 16:24, Niklas Schnelle <schnelle@linux.ibm.com> wrote:
> > > On Fri, 2021-09-03 at 16:03 +0800, Chunyan Zhang wrote:
> > > > From: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > > >
> > > > There would not be ioremap and iounmap implementations if CONFIG_PCI is
> > > > not set for s390, so add default declarations of these two functions
> > > > for the case to avoid 'undefined reference' issue.
> > > >
> > > > Fixes: 71ba41c9b1d9 ("s390/pci: provide support for MIO instructions")
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > > Signed-off-by: Chunyan Zhang <chunyan.zhang@unisoc.com>
> > > > ---
> > > > The issue was reported from https://lkml.org/lkml/2021/8/1/18
...
> > Actually HAS_IOMEM is set as default on other architectures, but not
> > for s390 which redefined it.
>
> Yes because most architectures always have IOMEM and io*map() functions
> I believe. s390 is an exception here as the mainframe native
> functionality all works without MMIO and you can run a fully functional
> system including networking and block devices without any MMIO, PCI and
> without ioremap()/iounmap().
>
> >
> > > At the very least I think the functions should do a WARN_ONCE() but
> > > then we have the same situation as discussed below with Linus making it
> > > pretty clear that he prefers these cases to be compile time checked:
> >
> > Ok, if I understand correctly, if io*map is not implemented for some
> > case, there should be a *compile-time* error rather than adding a stub
> > function to make this kind of errors disappeared.
> >
> > Please correct me if I missed something.
>
> Ideally not a compile time error but a compile time flag such as a
> Kconfig option that would make sure that if HAS_IOMEM isn't set we
> don't get drivers compiled which depend on working io*map(). After all
> these drivers will surely not be functional.
Please note that Arnd Bergmann started to work on that:
https://lkml.org/lkml/2021/7/5/286
However, as far as I can tell, there is nothing like that in
linux-next currently.
Arnd, are you still working on this?
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-09-03 10:51 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03 7:56 [PATCH] s390/io: Fix ioremap and iounmap undefinded issue for s390 Chunyan Zhang
2021-09-03 8:03 ` Chunyan Zhang
2021-09-03 8:24 ` Niklas Schnelle
2021-09-03 9:40 ` Chunyan Zhang
2021-09-03 10:28 ` Niklas Schnelle
2021-09-03 10:51 ` Heiko Carstens
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.