diff mbox series

[v3,09/11] s390: mm: Convert to GENERIC_IOREMAP

Message ID 20221009103114.149036-10-bhe@redhat.com (mailing list archive)
State New
Headers show
Series mm: ioremap: Convert architectures to take GENERIC_IOREMAP way | expand

Commit Message

Baoquan He Oct. 9, 2022, 10:31 a.m. UTC
By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
iounmap() are visible and available to arch. Arch only needs to
provide implementation of arch_ioremap() or arch_iounmap() if there's
arch specific handling needed in its ioremap() or iounmap(). This
change will simplify implementation by removing duplicated codes with
generic ioremap() and iounmap(), and has the equivalent functioality
as before.

For s390, add hooks arch_ioremap() and arch_iounmap() for s390's special
operation when ioremap() and iounmap(), then ioremap_[wc|wt]() are
converted to use ioremap_prot() from GENERIC_IOREMAP.

Signed-off-by: Baoquan He <bhe@redhat.com>
Cc: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Cc: Heiko Carstens <hca@linux.ibm.com>
Cc: Vasily Gorbik <gor@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>
Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
Cc: Sven Schnelle <svens@linux.ibm.com>
Cc: linux-s390@vger.kernel.org
---
v2->v3:
- Add code comment inside arch_ioremap() to help uderstand the
  obsucre code. Christoph suggested this, Niklas provided the
  paragraph of text.

 arch/s390/Kconfig          |  1 +
 arch/s390/include/asm/io.h | 25 +++++++++------
 arch/s390/pci/pci.c        | 65 ++++++++------------------------------
 3 files changed, 30 insertions(+), 61 deletions(-)

Comments

kernel test robot Oct. 9, 2022, 1:54 p.m. UTC | #1
Hi Baoquan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on next-20221007]
[cannot apply to akpm-mm/mm-everything openrisc/for-next deller-parisc/for-next s390/features v6.0]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
config: s390-buildonly-randconfig-r006-20221009
compiler: s390-linux-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/69f65149d2e87de076edbb2b4dd9532f8f57dd8b
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
        git checkout 69f65149d2e87de076edbb2b4dd9532f8f57dd8b
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   s390-linux-ld: mm/ioremap.o: in function `ioremap_prot':
>> ioremap.c:(.text+0x9a): undefined reference to `arch_ioremap'
   s390-linux-ld: mm/ioremap.o: in function `iounmap':
>> ioremap.c:(.text+0x234): undefined reference to `arch_iounmap'
   s390-linux-ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
   hidma.c:(.text+0x4b46): undefined reference to `devm_ioremap_resource'
   s390-linux-ld: hidma.c:(.text+0x4b9e): undefined reference to `devm_ioremap_resource'
Baoquan He Oct. 10, 2022, 10:38 a.m. UTC | #2
On 10/09/22 at 09:54pm, kernel test robot wrote:
> Hi Baoquan,
> 
> I love your patch! Yet something to improve:
> 
> [auto build test ERROR on linus/master]
> [also build test ERROR on next-20221007]
> [cannot apply to akpm-mm/mm-everything openrisc/for-next deller-parisc/for-next s390/features v6.0]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
> config: s390-buildonly-randconfig-r006-20221009
> compiler: s390-linux-gcc (GCC) 12.1.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # https://github.com/intel-lab-lkp/linux/commit/69f65149d2e87de076edbb2b4dd9532f8f57dd8b
>         git remote add linux-review https://github.com/intel-lab-lkp/linux
>         git fetch --no-tags linux-review Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
>         git checkout 69f65149d2e87de076edbb2b4dd9532f8f57dd8b
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
> 
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> 
> All errors (new ones prefixed by >>):
> 
>    s390-linux-ld: mm/ioremap.o: in function `ioremap_prot':
> >> ioremap.c:(.text+0x9a): undefined reference to `arch_ioremap'
>    s390-linux-ld: mm/ioremap.o: in function `iounmap':
> >> ioremap.c:(.text+0x234): undefined reference to `arch_iounmap'
>    s390-linux-ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
>    hidma.c:(.text+0x4b46): undefined reference to `devm_ioremap_resource'
>    s390-linux-ld: hidma.c:(.text+0x4b9e): undefined reference to `devm_ioremap_resource'

The above compiling errors are caused by unsetting CONFIG_PCI in
s390-buildonly-randconfig-r006-20221009 attached. I keep the items for
reference. Because s390 puts io mem functions in arch/s390/pci/pci.c.
While building arch/s390/pci/pci.c in needs CONFIG_PCI enabled. I don't
think disabling CONFIG_PCI in s390x makes sense in reality, except of
the randconfig testing.

Hi Niklas, lkp

What do you think about this? We can just ignore this building error
with randconfig in lkp?

> 
> -- 
> 0-DAY CI Kernel Test Service
> https://01.org/lkp

> #
> # Automatically generated file; DO NOT EDIT.
> # Linux/s390 6.0.0 Kernel Configuration
> #
...... 
> # end of General setup
> 
> CONFIG_MMU=y
......
> # Device Drivers
> #
> CONFIG_HAVE_PCI=y
> # CONFIG_PCI is not set
......
Niklas Schnelle Oct. 10, 2022, 11:53 a.m. UTC | #3
On Mon, 2022-10-10 at 18:38 +0800, Baoquan He wrote:
> On 10/09/22 at 09:54pm, kernel test robot wrote:
> > Hi Baoquan,
> > 
> > I love your patch! Yet something to improve:
> > 
> > [auto build test ERROR on linus/master]
> > [also build test ERROR on next-20221007]
> > [cannot apply to akpm-mm/mm-everything openrisc/for-next deller-parisc/for-next s390/features v6.0]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > 
> > url:    https://github.com/intel-lab-lkp/linux/commits/Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
> > config: s390-buildonly-randconfig-r006-20221009
> > compiler: s390-linux-gcc (GCC) 12.1.0
> > reproduce (this is a W=1 build):
> >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> >         chmod +x ~/bin/make.cross
> >         # https://github.com/intel-lab-lkp/linux/commit/69f65149d2e87de076edbb2b4dd9532f8f57dd8b
> >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> >         git fetch --no-tags linux-review Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
> >         git checkout 69f65149d2e87de076edbb2b4dd9532f8f57dd8b
> >         # save the config file
> >         mkdir build_dir && cp config build_dir/.config
> >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
> > 
> > If you fix the issue, kindly add following tag where applicable
> > > Reported-by: kernel test robot <lkp@intel.com>
> > 
> > All errors (new ones prefixed by >>):
> > 
> >    s390-linux-ld: mm/ioremap.o: in function `ioremap_prot':
> > > > ioremap.c:(.text+0x9a): undefined reference to `arch_ioremap'
> >    s390-linux-ld: mm/ioremap.o: in function `iounmap':
> > > > ioremap.c:(.text+0x234): undefined reference to `arch_iounmap'
> >    s390-linux-ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
> >    hidma.c:(.text+0x4b46): undefined reference to `devm_ioremap_resource'
> >    s390-linux-ld: hidma.c:(.text+0x4b9e): undefined reference to `devm_ioremap_resource'
> 
> The above compiling errors are caused by unsetting CONFIG_PCI in
> s390-buildonly-randconfig-r006-20221009 attached. I keep the items for
> reference. Because s390 puts io mem functions in arch/s390/pci/pci.c.
> While building arch/s390/pci/pci.c in needs CONFIG_PCI enabled. I don't
> think disabling CONFIG_PCI in s390x makes sense in reality, except of
> the randconfig testing.
> 
> Hi Niklas, lkp
> 
> What do you think about this? We can just ignore this building error
> with randconfig in lkp?

Hmm, that's a bummer. As s390 systems (aka mainframes) do have classic
channel devices for networking and permanent storage that are currently
even more common than PCI devices you can definitely have a fully
functional system with CONFIG_PCI=n. Also the drivers for these channel
devices do not use ioremap() which is only used for PCI, so in theory
it should be fine not to have ioremap() for CONFIG_PCI=n.

I think the reason for this concrete failure to compile is a missing
HAS_IOMEM dependency for CONFIG_QCOM_HIDMA. I'm not sure how many other
cases there are though as I think we might be the only ones where
HAS_IOMEM is only sometimes available (it depends on CONFIG_PCI for
us). Ideally I think we would have the driver dependencies. I'm a bit
confused though since in the current code it looks to me like
ioremap_prot() will be declared but not defined for CONFIG_PCI=n too as
far as I can tell at least.

> 
> > -- 
> > 0-DAY CI Kernel Test Service
> > https://01.org/lkp
> > #
> > # Automatically generated file; DO NOT EDIT.
> > # Linux/s390 6.0.0 Kernel Configuration
> > #
> ...... 
> > # end of General setup
> > 
> > CONFIG_MMU=y
> ......
> > # Device Drivers
> > #
> > CONFIG_HAVE_PCI=y
> > # CONFIG_PCI is not set
> ......
>
Baoquan He Oct. 11, 2022, 3 a.m. UTC | #4
On 10/10/22 at 01:53pm, Niklas Schnelle wrote:
> On Mon, 2022-10-10 at 18:38 +0800, Baoquan He wrote:
> > On 10/09/22 at 09:54pm, kernel test robot wrote:
> > > Hi Baoquan,
> > > 
> > > I love your patch! Yet something to improve:
> > > 
> > > [auto build test ERROR on linus/master]
> > > [also build test ERROR on next-20221007]
> > > [cannot apply to akpm-mm/mm-everything openrisc/for-next deller-parisc/for-next s390/features v6.0]
> > > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > > And when submitting patch, we suggest to use '--base' as documented in
> > > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> > > 
> > > url:    https://github.com/intel-lab-lkp/linux/commits/Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
> > > base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a6afa4199d3d038fbfdff5511f7523b0e30cb774
> > > config: s390-buildonly-randconfig-r006-20221009
> > > compiler: s390-linux-gcc (GCC) 12.1.0
> > > reproduce (this is a W=1 build):
> > >         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >         chmod +x ~/bin/make.cross
> > >         # https://github.com/intel-lab-lkp/linux/commit/69f65149d2e87de076edbb2b4dd9532f8f57dd8b
> > >         git remote add linux-review https://github.com/intel-lab-lkp/linux
> > >         git fetch --no-tags linux-review Baoquan-He/mm-ioremap-Convert-architectures-to-take-GENERIC_IOREMAP-way/20221009-183524
> > >         git checkout 69f65149d2e87de076edbb2b4dd9532f8f57dd8b
> > >         # save the config file
> > >         mkdir build_dir && cp config build_dir/.config
> > >         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All errors (new ones prefixed by >>):
> > > 
> > >    s390-linux-ld: mm/ioremap.o: in function `ioremap_prot':
> > > > > ioremap.c:(.text+0x9a): undefined reference to `arch_ioremap'
> > >    s390-linux-ld: mm/ioremap.o: in function `iounmap':
> > > > > ioremap.c:(.text+0x234): undefined reference to `arch_iounmap'
> > >    s390-linux-ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
> > >    hidma.c:(.text+0x4b46): undefined reference to `devm_ioremap_resource'
> > >    s390-linux-ld: hidma.c:(.text+0x4b9e): undefined reference to `devm_ioremap_resource'
> > 
> > The above compiling errors are caused by unsetting CONFIG_PCI in
> > s390-buildonly-randconfig-r006-20221009 attached. I keep the items for
> > reference. Because s390 puts io mem functions in arch/s390/pci/pci.c.
> > While building arch/s390/pci/pci.c in needs CONFIG_PCI enabled. I don't
> > think disabling CONFIG_PCI in s390x makes sense in reality, except of
> > the randconfig testing.
> > 
> > Hi Niklas, lkp
> > 
> > What do you think about this? We can just ignore this building error
> > with randconfig in lkp?
> 
> Hmm, that's a bummer. As s390 systems (aka mainframes) do have classic
> channel devices for networking and permanent storage that are currently
> even more common than PCI devices you can definitely have a fully
> functional system with CONFIG_PCI=n. Also the drivers for these channel
> devices do not use ioremap() which is only used for PCI, so in theory
> it should be fine not to have ioremap() for CONFIG_PCI=n.

Thanks for detailed explanation.

I built the latest upstream kernel with the randconfig, it has the issue
either, please see below build log snippet. Means if CONFIG_PCI is unset,
it has problem with the current s390 code.

ld: drivers/dma/qcom/hidma.o: in function `hidma_probe':
hidma.c:(.text+0x4b46): undefined reference to `devm_ioremap_resource'
ld: hidma.c:(.text+0x4b9e): undefined reference to `devm_ioremap_resource'
ld: drivers/pcmcia/cistpl.o: in function `set_cis_map':
cistpl.c:(.text+0x1202): undefined reference to `ioremap'
ld: cistpl.c:(.text+0x13b0): undefined reference to `iounmap'
ld: cistpl.c:(.text+0x14a6): undefined reference to `iounmap'
ld: cistpl.c:(.text+0x1544): undefined reference to `ioremap'
ld: drivers/pcmcia/cistpl.o: in function `release_cis_mem':
cistpl.c:(.text+0x3f14): undefined reference to `iounmap'
make[1]: *** [scripts/Makefile.vmlinux:34: vmlinux] Error 1
make: *** [Makefile:1235: vmlinux] Error 2

> 
> I think the reason for this concrete failure to compile is a missing
> HAS_IOMEM dependency for CONFIG_QCOM_HIDMA. I'm not sure how many other
> cases there are though as I think we might be the only ones where
> HAS_IOMEM is only sometimes available (it depends on CONFIG_PCI for
> us). Ideally I think we would have the driver dependencies. I'm a bit
> confused though since in the current code it looks to me like
> ioremap_prot() will be declared but not defined for CONFIG_PCI=n too as
> far as I can tell at least.

Yeah, depending on HAS_IOMEM for QCOM_HIDMA can fix it. And in
drivers/pcmcia/cistpl.c, there are ioremap()/iounmap() calling too.
Make PCMCIA depend on HAS_IOMEM can fix it. 

ioremap_prot() will called outside if CONFIG_HAVE_IOREMAP_PROT is
enabled. With this patchset, it will only be declared and defined when
CONFIG_GENERIC_IOREMAP is enabled. Please correct me if I'm wrong.

Below draft change can fix the compiling errors with the randconfig on
s390 as you suggested. We may need post the driver code change in
another patch since it's not related to this patchset?

From 1e169ce8e825d3a33be891ad06e14008582c7011 Mon Sep 17 00:00:00 2001
From: Baoquan He <bhe@redhat.com>
Date: Tue, 11 Oct 2022 09:30:49 +0800
Subject: [PATCH] s390, ioremap: fix
Content-type: text/plain

On s390 systems (aka mainframes), it has classic channel devices for
networking and permanent storage that are currently even more common
than PCI devices. Hence it's likely to have a fully functional s390
kernel with CONFIG_PCI=n.

So let GENERIC_IOREMAP depend on PCI in s390 arch since the drivers for
those channel devices do not use ioremap() which is only used for PCI,
in theory it should be fine not to have ioremap() for CONFIG_PCI=n.

And let QCOM_HIDMA and PCMCIA PCMCIA depend on HAS_IOMEM so that they
won't be built if PCI is unset.

Signed-off-by: Baoquan He <bhe@redhat.com>
---
 arch/s390/Kconfig        | 2 +-
 drivers/dma/qcom/Kconfig | 1 +
 drivers/pcmcia/Kconfig   | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index c59e1b25f59d..f6a7f1752f0f 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -135,7 +135,7 @@ config S390
 	select GENERIC_SMP_IDLE_THREAD
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_VDSO_TIME_NS
-	select GENERIC_IOREMAP
+	select GENERIC_IOREMAP if PCI
 	select HAVE_ALIGNED_STRUCT_PAGE if SLUB
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
diff --git a/drivers/dma/qcom/Kconfig b/drivers/dma/qcom/Kconfig
index 3f926a653bd8..ace75d7b835a 100644
--- a/drivers/dma/qcom/Kconfig
+++ b/drivers/dma/qcom/Kconfig
@@ -45,6 +45,7 @@ config QCOM_HIDMA_MGMT
 
 config QCOM_HIDMA
 	tristate "Qualcomm Technologies HIDMA Channel support"
+	depends on HAS_IOMEM
 	select DMA_ENGINE
 	help
 	  Enable support for the Qualcomm Technologies HIDMA controller.
diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
index 1525023e49b6..7c412bbe8bbe 100644
--- a/drivers/pcmcia/Kconfig
+++ b/drivers/pcmcia/Kconfig
@@ -20,6 +20,7 @@ if PCCARD
 
 config PCMCIA
 	tristate "16-bit PCMCIA support"
+	depends on HAS_IOMEM
 	select CRC32
 	default y
 	help
Niklas Schnelle Oct. 11, 2022, 3:16 p.m. UTC | #5
On Sun, 2022-10-09 at 18:31 +0800, Baoquan He wrote:
> By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
> iounmap() are visible and available to arch. Arch only needs to
> provide implementation of arch_ioremap() or arch_iounmap() if there's
> arch specific handling needed in its ioremap() or iounmap(). This
> change will simplify implementation by removing duplicated codes with
> generic ioremap() and iounmap(), and has the equivalent functioality
> as before.
> 
> For s390, add hooks arch_ioremap() and arch_iounmap() for s390's special
> operation when ioremap() and iounmap(), then ioremap_[wc|wt]() are
> converted to use ioremap_prot() from GENERIC_IOREMAP.
> 
> Signed-off-by: Baoquan He <bhe@redhat.com>
> Cc: Niklas Schnelle <schnelle@linux.ibm.com>
> Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> Cc: Heiko Carstens <hca@linux.ibm.com>
> Cc: Vasily Gorbik <gor@linux.ibm.com>
> Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> Cc: Sven Schnelle <svens@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> ---
> v2->v3:
> - Add code comment inside arch_ioremap() to help uderstand the
>   obsucre code. Christoph suggested this, Niklas provided the
>   paragraph of text.
> 
>  arch/s390/Kconfig          |  1 +
>  arch/s390/include/asm/io.h | 25 +++++++++------
>  arch/s390/pci/pci.c        | 65 ++++++++------------------------------
>  3 files changed, 30 insertions(+), 61 deletions(-)
> 
> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 318fce77601d..c59e1b25f59d 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -135,6 +135,7 @@ config S390
>  	select GENERIC_SMP_IDLE_THREAD
>  	select GENERIC_TIME_VSYSCALL
>  	select GENERIC_VDSO_TIME_NS
> +	select GENERIC_IOREMAP

I think you should add the "if PCI" from the diff in your last mail to
this patch.

>  	select HAVE_ALIGNED_STRUCT_PAGE if SLUB
>  	select HAVE_ARCH_AUDITSYSCALL
>  	select HAVE_ARCH_JUMP_LABEL
> diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
> index e3882b012bfa..db201563baa6 100644
> --- a/arch/s390/include/asm/io.h
> +++ b/arch/s390/include/asm/io.h
> @@ -22,11 +22,22 @@ 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);
> +/*
> + * I/O memory mapping functions.
> + */
> +void __iomem *
> +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val);
> +#define arch_ioremap arch_ioremap
> +
> +bool arch_iounmap(void __iomem *addr);
> +#define arch_iounmap arch_iounmap
> +
> +#define _PAGE_IOREMAP pgprot_val(PAGE_KERNEL)
> +
> +#define ioremap_wc(addr, size)  \
> +	ioremap_prot((addr), (size), pgprot_val(pgprot_writecombine(PAGE_KERNEL)))
> +#define ioremap_wt(addr, size)  \
> +	ioremap_prot((addr), (size), pgprot_val(pgprot_writethrough(PAGE_KERNEL)))
>  
>  static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
>  {
> @@ -51,10 +62,6 @@ static inline void ioport_unmap(void __iomem *p)
>  #define pci_iomap_wc pci_iomap_wc
>  #define pci_iomap_wc_range pci_iomap_wc_range
>  
> -#define ioremap ioremap
> -#define ioremap_wt ioremap_wt
> -#define ioremap_wc ioremap_wc
> -
>  #define memcpy_fromio(dst, src, count)	zpci_memcpy_fromio(dst, src, count)
>  #define memcpy_toio(dst, src, count)	zpci_memcpy_toio(dst, src, count)
>  #define memset_io(dst, val, count)	zpci_memset_io(dst, val, count)
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 73cdc5539384..3c00dc7d79bc 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -244,64 +244,25 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
>         zpci_memcpy_toio(to, from, count);
>  }
>  
> -static void __iomem *__ioremap(phys_addr_t addr, size_t size, pgprot_t prot)
> +void __iomem *
> +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
>  {
> -	unsigned long offset, vaddr;
> -	struct vm_struct *area;
> -	phys_addr_t last_addr;
> -
> -	last_addr = addr + size - 1;
> -	if (!size || last_addr < addr)
> -		return NULL;
> -
> +	/*
> +	 * When PCI MIO instructions are unavailable the "physical" address
> +	 * encodes a hint for accessing the PCI memory space it represents.
> +	 * Just pass it unchanged such that ioread/iowrite can decode it.
> +	 */
>  	if (!static_branch_unlikely(&have_mio))
> -		return (void __iomem *) addr;
> -
> -	offset = addr & ~PAGE_MASK;
> -	addr &= PAGE_MASK;
> -	size = PAGE_ALIGN(size + offset);
> -	area = get_vm_area(size, VM_IOREMAP);
> -	if (!area)
> -		return NULL;
> -
> -	vaddr = (unsigned long) area->addr;
> -	if (ioremap_page_range(vaddr, vaddr + size, addr, prot)) {
> -		free_vm_area(area);
> -		return NULL;
> -	}
> -	return (void __iomem *) ((unsigned long) area->addr + offset);
> +		return (void __iomem *) *paddr;

nit: no space after the cast

> +	return NULL;
>  }
>  
> -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> +bool arch_iounmap(void __iomem *addr)
>  {
> -	return __ioremap(addr, size, __pgprot(prot));
> -}
> -EXPORT_SYMBOL(ioremap_prot);
> -
> -void __iomem *ioremap(phys_addr_t addr, size_t size)
> -{
> -	return __ioremap(addr, size, PAGE_KERNEL);
> -}
> -EXPORT_SYMBOL(ioremap);
> -
> -void __iomem *ioremap_wc(phys_addr_t addr, size_t size)
> -{
> -	return __ioremap(addr, size, pgprot_writecombine(PAGE_KERNEL));
> -}
> -EXPORT_SYMBOL(ioremap_wc);
> -
> -void __iomem *ioremap_wt(phys_addr_t addr, size_t size)
> -{
> -	return __ioremap(addr, size, pgprot_writethrough(PAGE_KERNEL));
> -}
> -EXPORT_SYMBOL(ioremap_wt);
> -
> -void iounmap(volatile void __iomem *addr)
> -{
> -	if (static_branch_likely(&have_mio))
> -		vunmap((__force void *) ((unsigned long) addr & PAGE_MASK));
> +	if (!static_branch_likely(&have_mio))
> +		return false;
> +	return true;
>  }
> -EXPORT_SYMBOL(iounmap);
>  
>  /* Create a virtual mapping cookie for a PCI BAR */
>  static void __iomem *pci_iomap_range_fh(struct pci_dev *pdev, int bar,

Gave this a round of testing on s390 with both the MIO and non-MIO
cases. I also see you addressed my previous comments and it looks good
to me. As you showed in the other mail the compile error is a pre
existing problem so shouldn't influence this change. So assuming you
add the "if PCI" and the nit above you can add my:

Tested-by: Niklas
Schnelle <schnelle@linux.ibm.com>
Reviewed-by: Niklas Schnelle
<schnelle@linux.ibm.com>
Baoquan He Oct. 12, 2022, 5:52 a.m. UTC | #6
On 10/11/22 at 05:16pm, Niklas Schnelle wrote:
> On Sun, 2022-10-09 at 18:31 +0800, Baoquan He wrote:
> > By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
> > iounmap() are visible and available to arch. Arch only needs to
> > provide implementation of arch_ioremap() or arch_iounmap() if there's
> > arch specific handling needed in its ioremap() or iounmap(). This
> > change will simplify implementation by removing duplicated codes with
> > generic ioremap() and iounmap(), and has the equivalent functioality
> > as before.
> > 
> > For s390, add hooks arch_ioremap() and arch_iounmap() for s390's special
> > operation when ioremap() and iounmap(), then ioremap_[wc|wt]() are
> > converted to use ioremap_prot() from GENERIC_IOREMAP.
> > 
> > Signed-off-by: Baoquan He <bhe@redhat.com>
> > Cc: Niklas Schnelle <schnelle@linux.ibm.com>
> > Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> > Cc: Heiko Carstens <hca@linux.ibm.com>
> > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> > Cc: Sven Schnelle <svens@linux.ibm.com>
> > Cc: linux-s390@vger.kernel.org
> > ---
> > v2->v3:
> > - Add code comment inside arch_ioremap() to help uderstand the
> >   obsucre code. Christoph suggested this, Niklas provided the
> >   paragraph of text.
> > 
> >  arch/s390/Kconfig          |  1 +
> >  arch/s390/include/asm/io.h | 25 +++++++++------
> >  arch/s390/pci/pci.c        | 65 ++++++++------------------------------
> >  3 files changed, 30 insertions(+), 61 deletions(-)
> > 
> > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > index 318fce77601d..c59e1b25f59d 100644
> > --- a/arch/s390/Kconfig
> > +++ b/arch/s390/Kconfig
> > @@ -135,6 +135,7 @@ config S390
> >  	select GENERIC_SMP_IDLE_THREAD
> >  	select GENERIC_TIME_VSYSCALL
> >  	select GENERIC_VDSO_TIME_NS
> > +	select GENERIC_IOREMAP
> 
> I think you should add the "if PCI" from the diff in your last mail to
> this patch.

That's reasonable, will do.

The code change in driver should be posted separately to get reviewing
from the relevant drvier maintainers. I may wrap it into this series in
next post so that people know its background.

> 
> >  	select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> >  	select HAVE_ARCH_AUDITSYSCALL
> >  	select HAVE_ARCH_JUMP_LABEL
......
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index 73cdc5539384..3c00dc7d79bc 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -244,64 +244,25 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
> >         zpci_memcpy_toio(to, from, count);
> >  }
> >  
> > -static void __iomem *__ioremap(phys_addr_t addr, size_t size, pgprot_t prot)
> > +void __iomem *
> > +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
> >  {
> > -	unsigned long offset, vaddr;
> > -	struct vm_struct *area;
> > -	phys_addr_t last_addr;
> > -
> > -	last_addr = addr + size - 1;
> > -	if (!size || last_addr < addr)
> > -		return NULL;
> > -
> > +	/*
> > +	 * When PCI MIO instructions are unavailable the "physical" address
> > +	 * encodes a hint for accessing the PCI memory space it represents.
> > +	 * Just pass it unchanged such that ioread/iowrite can decode it.
> > +	 */
> >  	if (!static_branch_unlikely(&have_mio))
> > -		return (void __iomem *) addr;
> > -
> > -	offset = addr & ~PAGE_MASK;
> > -	addr &= PAGE_MASK;
> > -	size = PAGE_ALIGN(size + offset);
> > -	area = get_vm_area(size, VM_IOREMAP);
> > -	if (!area)
> > -		return NULL;
> > -
> > -	vaddr = (unsigned long) area->addr;
> > -	if (ioremap_page_range(vaddr, vaddr + size, addr, prot)) {
> > -		free_vm_area(area);
> > -		return NULL;
> > -	}
> > -	return (void __iomem *) ((unsigned long) area->addr + offset);
> > +		return (void __iomem *) *paddr;
> 
> nit: no space after the cast

Sorry, remember you pointed this out in v2, while I didn't get what
it is. Could you be more specific or give the right line of code?

Are you suggesting below line? 

-	return (void __iomem *) ((unsigned long) area->addr + offset);
+		return (void __iomem *)*paddr;

> 
> > +	return NULL;
> >  }
> >  
> > -void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
> > +bool arch_iounmap(void __iomem *addr)
> >  {
> > -	return __ioremap(addr, size, __pgprot(prot));
> > -}
> > -EXPORT_SYMBOL(ioremap_prot);
> > -
> > -void __iomem *ioremap(phys_addr_t addr, size_t size)
> > -{
> > -	return __ioremap(addr, size, PAGE_KERNEL);
> > -}
> > -EXPORT_SYMBOL(ioremap);
> > -
> > -void __iomem *ioremap_wc(phys_addr_t addr, size_t size)
> > -{
> > -	return __ioremap(addr, size, pgprot_writecombine(PAGE_KERNEL));
> > -}
> > -EXPORT_SYMBOL(ioremap_wc);
> > -
> > -void __iomem *ioremap_wt(phys_addr_t addr, size_t size)
> > -{
> > -	return __ioremap(addr, size, pgprot_writethrough(PAGE_KERNEL));
> > -}
> > -EXPORT_SYMBOL(ioremap_wt);
> > -
> > -void iounmap(volatile void __iomem *addr)
> > -{
> > -	if (static_branch_likely(&have_mio))
> > -		vunmap((__force void *) ((unsigned long) addr & PAGE_MASK));
> > +	if (!static_branch_likely(&have_mio))
> > +		return false;
> > +	return true;
> >  }
> > -EXPORT_SYMBOL(iounmap);
> >  
> >  /* Create a virtual mapping cookie for a PCI BAR */
> >  static void __iomem *pci_iomap_range_fh(struct pci_dev *pdev, int bar,
> 
> Gave this a round of testing on s390 with both the MIO and non-MIO
> cases. I also see you addressed my previous comments and it looks good
> to me. As you showed in the other mail the compile error is a pre
> existing problem so shouldn't influence this change. So assuming you
> add the "if PCI" and the nit above you can add my:

Thanks a lot, will add that part to this patch and add your tags.

> 
> Tested-by: Niklas
> Schnelle <schnelle@linux.ibm.com>
> Reviewed-by: Niklas Schnelle
> <schnelle@linux.ibm.com>
> 
>
Niklas Schnelle Oct. 12, 2022, 7:37 a.m. UTC | #7
On Wed, 2022-10-12 at 13:52 +0800, Baoquan He wrote:
> On 10/11/22 at 05:16pm, Niklas Schnelle wrote:
> > On Sun, 2022-10-09 at 18:31 +0800, Baoquan He wrote:
> > > By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
> > > iounmap() are visible and available to arch. Arch only needs to
> > > provide implementation of arch_ioremap() or arch_iounmap() if there's
> > > arch specific handling needed in its ioremap() or iounmap(). This
> > > change will simplify implementation by removing duplicated codes with
> > > generic ioremap() and iounmap(), and has the equivalent functioality
> > > as before.
> > > 
> > > For s390, add hooks arch_ioremap() and arch_iounmap() for s390's special
> > > operation when ioremap() and iounmap(), then ioremap_[wc|wt]() are
> > > converted to use ioremap_prot() from GENERIC_IOREMAP.
> > > 
> > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > Cc: Niklas Schnelle <schnelle@linux.ibm.com>
> > > Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> > > Cc: Heiko Carstens <hca@linux.ibm.com>
> > > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > > Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> > > Cc: Sven Schnelle <svens@linux.ibm.com>
> > > Cc: linux-s390@vger.kernel.org
> > > ---
> > > v2->v3:
> > > - Add code comment inside arch_ioremap() to help uderstand the
> > >   obsucre code. Christoph suggested this, Niklas provided the
> > >   paragraph of text.
> > > 
> > >  arch/s390/Kconfig          |  1 +
> > >  arch/s390/include/asm/io.h | 25 +++++++++------
> > >  arch/s390/pci/pci.c        | 65 ++++++++------------------------------
> > >  3 files changed, 30 insertions(+), 61 deletions(-)
> > > 
> > > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > > index 318fce77601d..c59e1b25f59d 100644
> > > --- a/arch/s390/Kconfig
> > > +++ b/arch/s390/Kconfig
> > > @@ -135,6 +135,7 @@ config S390
> > >  	select GENERIC_SMP_IDLE_THREAD
> > >  	select GENERIC_TIME_VSYSCALL
> > >  	select GENERIC_VDSO_TIME_NS
> > > +	select GENERIC_IOREMAP
> > 
> > I think you should add the "if PCI" from the diff in your last mail to
> > this patch.
> 
> That's reasonable, will do.
> 
> The code change in driver should be posted separately to get reviewing
> from the relevant drvier maintainers. I may wrap it into this series in
> next post so that people know its background.

I agree about doing the driver change separately. Since the problem
already exists one could send it separately. If you want I can take of
that too.

> 
> > >  	select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> > >  	select HAVE_ARCH_AUDITSYSCALL
> > >  	select HAVE_ARCH_JUMP_LABEL
> ......
> > > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > > index 73cdc5539384..3c00dc7d79bc 100644
> > > --- a/arch/s390/pci/pci.c
> > > +++ b/arch/s390/pci/pci.c
> > > @@ -244,64 +244,25 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
> > >         zpci_memcpy_toio(to, from, count);
> > >  }
> > >  
> > > -static void __iomem *__ioremap(phys_addr_t addr, size_t size, pgprot_t prot)
> > > +void __iomem *
> > > +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
> > >  {
> > > -	unsigned long offset, vaddr;
> > > -	struct vm_struct *area;
> > > -	phys_addr_t last_addr;
> > > -
> > > -	last_addr = addr + size - 1;
> > > -	if (!size || last_addr < addr)
> > > -		return NULL;
> > > -
> > > +	/*
> > > +	 * When PCI MIO instructions are unavailable the "physical" address
> > > +	 * encodes a hint for accessing the PCI memory space it represents.
> > > +	 * Just pass it unchanged such that ioread/iowrite can decode it.
> > > +	 */
> > >  	if (!static_branch_unlikely(&have_mio))
> > > -		return (void __iomem *) addr;
> > > -
> > > -	offset = addr & ~PAGE_MASK;
> > > -	addr &= PAGE_MASK;
> > > -	size = PAGE_ALIGN(size + offset);
> > > -	area = get_vm_area(size, VM_IOREMAP);
> > > -	if (!area)
> > > -		return NULL;
> > > -
> > > -	vaddr = (unsigned long) area->addr;
> > > -	if (ioremap_page_range(vaddr, vaddr + size, addr, prot)) {
> > > -		free_vm_area(area);
> > > -		return NULL;
> > > -	}
> > > -	return (void __iomem *) ((unsigned long) area->addr + offset);
> > > +		return (void __iomem *) *paddr;
> > 
> > nit: no space after the cast
> 
> Sorry, remember you pointed this out in v2, while I didn't get what
> it is. Could you be more specific or give the right line of code?
> 
> Are you suggesting below line? 
> 
> -	return (void __iomem *) ((unsigned long) area->addr + offset);
> +		return (void __iomem *)*paddr;

Yes, though I did just check and somehow checkpatch doesn't complain,
maybe because of the dereference. I do think I remember it complaining
but I guess if it doesn't you might as well keep it this way.

> 
> > > +	return NULL;
> > >  }
> > > 
---8<---
Baoquan He Oct. 12, 2022, 9:20 a.m. UTC | #8
On 10/12/22 at 09:37am, Niklas Schnelle wrote:
> On Wed, 2022-10-12 at 13:52 +0800, Baoquan He wrote:
> > On 10/11/22 at 05:16pm, Niklas Schnelle wrote:
> > > On Sun, 2022-10-09 at 18:31 +0800, Baoquan He wrote:
> > > > By taking GENERIC_IOREMAP method, the generic ioremap_prot() and
> > > > iounmap() are visible and available to arch. Arch only needs to
> > > > provide implementation of arch_ioremap() or arch_iounmap() if there's
> > > > arch specific handling needed in its ioremap() or iounmap(). This
> > > > change will simplify implementation by removing duplicated codes with
> > > > generic ioremap() and iounmap(), and has the equivalent functioality
> > > > as before.
> > > > 
> > > > For s390, add hooks arch_ioremap() and arch_iounmap() for s390's special
> > > > operation when ioremap() and iounmap(), then ioremap_[wc|wt]() are
> > > > converted to use ioremap_prot() from GENERIC_IOREMAP.
> > > > 
> > > > Signed-off-by: Baoquan He <bhe@redhat.com>
> > > > Cc: Niklas Schnelle <schnelle@linux.ibm.com>
> > > > Cc: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
> > > > Cc: Heiko Carstens <hca@linux.ibm.com>
> > > > Cc: Vasily Gorbik <gor@linux.ibm.com>
> > > > Cc: Alexander Gordeev <agordeev@linux.ibm.com>
> > > > Cc: Christian Borntraeger <borntraeger@linux.ibm.com>
> > > > Cc: Sven Schnelle <svens@linux.ibm.com>
> > > > Cc: linux-s390@vger.kernel.org
> > > > ---
> > > > v2->v3:
> > > > - Add code comment inside arch_ioremap() to help uderstand the
> > > >   obsucre code. Christoph suggested this, Niklas provided the
> > > >   paragraph of text.
> > > > 
> > > >  arch/s390/Kconfig          |  1 +
> > > >  arch/s390/include/asm/io.h | 25 +++++++++------
> > > >  arch/s390/pci/pci.c        | 65 ++++++++------------------------------
> > > >  3 files changed, 30 insertions(+), 61 deletions(-)
> > > > 
> > > > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> > > > index 318fce77601d..c59e1b25f59d 100644
> > > > --- a/arch/s390/Kconfig
> > > > +++ b/arch/s390/Kconfig
> > > > @@ -135,6 +135,7 @@ config S390
> > > >  	select GENERIC_SMP_IDLE_THREAD
> > > >  	select GENERIC_TIME_VSYSCALL
> > > >  	select GENERIC_VDSO_TIME_NS
> > > > +	select GENERIC_IOREMAP
> > > 
> > > I think you should add the "if PCI" from the diff in your last mail to
> > > this patch.
> > 
> > That's reasonable, will do.
> > 
> > The code change in driver should be posted separately to get reviewing
> > from the relevant drvier maintainers. I may wrap it into this series in
> > next post so that people know its background.
> 
> I agree about doing the driver change separately. Since the problem
> already exists one could send it separately. If you want I can take of
> that too.

Both is fine to me, since you suggested the fix.

> 
> > 
> > > >  	select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> > > >  	select HAVE_ARCH_AUDITSYSCALL
> > > >  	select HAVE_ARCH_JUMP_LABEL
> > ......
> > > > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > > > index 73cdc5539384..3c00dc7d79bc 100644
> > > > --- a/arch/s390/pci/pci.c
> > > > +++ b/arch/s390/pci/pci.c
> > > > @@ -244,64 +244,25 @@ void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
> > > >         zpci_memcpy_toio(to, from, count);
> > > >  }
> > > >  
> > > > -static void __iomem *__ioremap(phys_addr_t addr, size_t size, pgprot_t prot)
> > > > +void __iomem *
> > > > +arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
> > > >  {
> > > > -	unsigned long offset, vaddr;
> > > > -	struct vm_struct *area;
> > > > -	phys_addr_t last_addr;
> > > > -
> > > > -	last_addr = addr + size - 1;
> > > > -	if (!size || last_addr < addr)
> > > > -		return NULL;
> > > > -
> > > > +	/*
> > > > +	 * When PCI MIO instructions are unavailable the "physical" address
> > > > +	 * encodes a hint for accessing the PCI memory space it represents.
> > > > +	 * Just pass it unchanged such that ioread/iowrite can decode it.
> > > > +	 */
> > > >  	if (!static_branch_unlikely(&have_mio))
> > > > -		return (void __iomem *) addr;
> > > > -
> > > > -	offset = addr & ~PAGE_MASK;
> > > > -	addr &= PAGE_MASK;
> > > > -	size = PAGE_ALIGN(size + offset);
> > > > -	area = get_vm_area(size, VM_IOREMAP);
> > > > -	if (!area)
> > > > -		return NULL;
> > > > -
> > > > -	vaddr = (unsigned long) area->addr;
> > > > -	if (ioremap_page_range(vaddr, vaddr + size, addr, prot)) {
> > > > -		free_vm_area(area);
> > > > -		return NULL;
> > > > -	}
> > > > -	return (void __iomem *) ((unsigned long) area->addr + offset);
> > > > +		return (void __iomem *) *paddr;
> > > 
> > > nit: no space after the cast
> > 
> > Sorry, remember you pointed this out in v2, while I didn't get what
> > it is. Could you be more specific or give the right line of code?
> > 
> > Are you suggesting below line? 
> > 
> > -	return (void __iomem *) ((unsigned long) area->addr + offset);
> > +		return (void __iomem *)*paddr;
> 
> Yes, though I did just check and somehow checkpatch doesn't complain,
> maybe because of the dereference. I do think I remember it complaining
> but I guess if it doesn't you might as well keep it this way.

OK, I will keep it unless checkpatch complaim about it. Thanks.
diff mbox series

Patch

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 318fce77601d..c59e1b25f59d 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -135,6 +135,7 @@  config S390
 	select GENERIC_SMP_IDLE_THREAD
 	select GENERIC_TIME_VSYSCALL
 	select GENERIC_VDSO_TIME_NS
+	select GENERIC_IOREMAP
 	select HAVE_ALIGNED_STRUCT_PAGE if SLUB
 	select HAVE_ARCH_AUDITSYSCALL
 	select HAVE_ARCH_JUMP_LABEL
diff --git a/arch/s390/include/asm/io.h b/arch/s390/include/asm/io.h
index e3882b012bfa..db201563baa6 100644
--- a/arch/s390/include/asm/io.h
+++ b/arch/s390/include/asm/io.h
@@ -22,11 +22,22 @@  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);
+/*
+ * I/O memory mapping functions.
+ */
+void __iomem *
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val);
+#define arch_ioremap arch_ioremap
+
+bool arch_iounmap(void __iomem *addr);
+#define arch_iounmap arch_iounmap
+
+#define _PAGE_IOREMAP pgprot_val(PAGE_KERNEL)
+
+#define ioremap_wc(addr, size)  \
+	ioremap_prot((addr), (size), pgprot_val(pgprot_writecombine(PAGE_KERNEL)))
+#define ioremap_wt(addr, size)  \
+	ioremap_prot((addr), (size), pgprot_val(pgprot_writethrough(PAGE_KERNEL)))
 
 static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
 {
@@ -51,10 +62,6 @@  static inline void ioport_unmap(void __iomem *p)
 #define pci_iomap_wc pci_iomap_wc
 #define pci_iomap_wc_range pci_iomap_wc_range
 
-#define ioremap ioremap
-#define ioremap_wt ioremap_wt
-#define ioremap_wc ioremap_wc
-
 #define memcpy_fromio(dst, src, count)	zpci_memcpy_fromio(dst, src, count)
 #define memcpy_toio(dst, src, count)	zpci_memcpy_toio(dst, src, count)
 #define memset_io(dst, val, count)	zpci_memset_io(dst, val, count)
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 73cdc5539384..3c00dc7d79bc 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -244,64 +244,25 @@  void __iowrite64_copy(void __iomem *to, const void *from, size_t count)
        zpci_memcpy_toio(to, from, count);
 }
 
-static void __iomem *__ioremap(phys_addr_t addr, size_t size, pgprot_t prot)
+void __iomem *
+arch_ioremap(phys_addr_t *paddr, size_t size, unsigned long *prot_val)
 {
-	unsigned long offset, vaddr;
-	struct vm_struct *area;
-	phys_addr_t last_addr;
-
-	last_addr = addr + size - 1;
-	if (!size || last_addr < addr)
-		return NULL;
-
+	/*
+	 * When PCI MIO instructions are unavailable the "physical" address
+	 * encodes a hint for accessing the PCI memory space it represents.
+	 * Just pass it unchanged such that ioread/iowrite can decode it.
+	 */
 	if (!static_branch_unlikely(&have_mio))
-		return (void __iomem *) addr;
-
-	offset = addr & ~PAGE_MASK;
-	addr &= PAGE_MASK;
-	size = PAGE_ALIGN(size + offset);
-	area = get_vm_area(size, VM_IOREMAP);
-	if (!area)
-		return NULL;
-
-	vaddr = (unsigned long) area->addr;
-	if (ioremap_page_range(vaddr, vaddr + size, addr, prot)) {
-		free_vm_area(area);
-		return NULL;
-	}
-	return (void __iomem *) ((unsigned long) area->addr + offset);
+		return (void __iomem *) *paddr;
+	return NULL;
 }
 
-void __iomem *ioremap_prot(phys_addr_t addr, size_t size, unsigned long prot)
+bool arch_iounmap(void __iomem *addr)
 {
-	return __ioremap(addr, size, __pgprot(prot));
-}
-EXPORT_SYMBOL(ioremap_prot);
-
-void __iomem *ioremap(phys_addr_t addr, size_t size)
-{
-	return __ioremap(addr, size, PAGE_KERNEL);
-}
-EXPORT_SYMBOL(ioremap);
-
-void __iomem *ioremap_wc(phys_addr_t addr, size_t size)
-{
-	return __ioremap(addr, size, pgprot_writecombine(PAGE_KERNEL));
-}
-EXPORT_SYMBOL(ioremap_wc);
-
-void __iomem *ioremap_wt(phys_addr_t addr, size_t size)
-{
-	return __ioremap(addr, size, pgprot_writethrough(PAGE_KERNEL));
-}
-EXPORT_SYMBOL(ioremap_wt);
-
-void iounmap(volatile void __iomem *addr)
-{
-	if (static_branch_likely(&have_mio))
-		vunmap((__force void *) ((unsigned long) addr & PAGE_MASK));
+	if (!static_branch_likely(&have_mio))
+		return false;
+	return true;
 }
-EXPORT_SYMBOL(iounmap);
 
 /* Create a virtual mapping cookie for a PCI BAR */
 static void __iomem *pci_iomap_range_fh(struct pci_dev *pdev, int bar,