[PATCH v3 00/27] kill devm_ioremap_nocache

[PATCH v3 00/27] kill devm_ioremap_nocache

[Yisheng Xie]

Hi all,

When I tried to use devm_ioremap function and review related code, I found
devm_ioremap and devm_ioremap_nocache is almost the same with each other,
except one use ioremap while the other use ioremap_nocache. While ioremap's
default function is ioremap_nocache, so devm_ioremap_nocache also have the
same function with devm_ioremap, which can just be killed to reduce the size
of devres.o(from 20304 bytes to 18992 bytes in my compile environment).

I have posted two versions, which use macro instead of function for
devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
devm_ioremap_nocache for no need to keep a macro around for the duplicate
thing. So here comes v3 and please help to review.

Thanks so much!
Yisheng Xie

[1] https://lkml.org/lkml/2017/11/20/135
[2] https://lkml.org/lkml/2017/11/25/21

Yisheng Xie (27):
  ASOC: replace devm_ioremap_nocache with devm_ioremap
  spi: replace devm_ioremap_nocache with devm_ioremap
  staging: replace devm_ioremap_nocache with devm_ioremap
  ipack: replace devm_ioremap_nocache with devm_ioremap
  media: replace devm_ioremap_nocache with devm_ioremap
  gpio: replace devm_ioremap_nocache with devm_ioremap
  mmc: replace devm_ioremap_nocache with devm_ioremap
  PCI: replace devm_ioremap_nocache with devm_ioremap
  platform/x86: replace devm_ioremap_nocache with devm_ioremap
  tty: replace devm_ioremap_nocache with devm_ioremap
  video: replace devm_ioremap_nocache with devm_ioremap
  rtc: replace devm_ioremap_nocache with devm_ioremap
  char: replace devm_ioremap_nocache with devm_ioremap
  mtd: nand: replace devm_ioremap_nocache with devm_ioremap
  dmaengine: replace devm_ioremap_nocache with devm_ioremap
  ata: replace devm_ioremap_nocache with devm_ioremap
  irqchip: replace devm_ioremap_nocache with devm_ioremap
  pinctrl: replace devm_ioremap_nocache with devm_ioremap
  drm: replace devm_ioremap_nocache with devm_ioremap
  regulator: replace devm_ioremap_nocache with devm_ioremap
  watchdog: replace devm_ioremap_nocache with devm_ioremap
  tools/testing/nvdimm: replace devm_ioremap_nocache with devm_ioremap
  MIPS: pci: replace devm_ioremap_nocache with devm_ioremap
  can: replace devm_ioremap_nocache with devm_ioremap
  wireless: replace devm_ioremap_nocache with devm_ioremap
  ethernet: replace devm_ioremap_nocache with devm_ioremap
  devres: kill devm_ioremap_nocache

 Documentation/driver-model/devres.txt           |  1 -
 arch/mips/pci/pci-ar2315.c                      |  3 +--
 drivers/ata/pata_arasan_cf.c                    |  3 +--
 drivers/ata/pata_octeon_cf.c                    |  9 ++++----
 drivers/ata/pata_rb532_cf.c                     |  2 +-
 drivers/char/hw_random/bcm63xx-rng.c            |  3 +--
 drivers/char/hw_random/octeon-rng.c             | 10 ++++-----
 drivers/dma/altera-msgdma.c                     |  3 +--
 drivers/dma/sprd-dma.c                          |  4 ++--
 drivers/gpio/gpio-ath79.c                       |  3 +--
 drivers/gpio/gpio-em.c                          |  6 ++---
 drivers/gpio/gpio-htc-egpio.c                   |  4 ++--
 drivers/gpio/gpio-xgene.c                       |  3 +--
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c |  2 +-
 drivers/gpu/drm/msm/msm_drv.c                   |  2 +-
 drivers/gpu/drm/sti/sti_dvo.c                   |  3 +--
 drivers/gpu/drm/sti/sti_hda.c                   |  4 ++--
 drivers/gpu/drm/sti/sti_hdmi.c                  |  2 +-
 drivers/gpu/drm/sti/sti_tvout.c                 |  2 +-
 drivers/gpu/drm/sti/sti_vtg.c                   |  2 +-
 drivers/ipack/devices/ipoctal.c                 | 13 +++++------
 drivers/irqchip/irq-renesas-intc-irqpin.c       |  4 ++--
 drivers/media/platform/tegra-cec/tegra_cec.c    |  4 ++--
 drivers/mmc/host/sdhci-acpi.c                   |  3 +--
 drivers/mtd/nand/fsl_upm.c                      |  4 ++--
 drivers/net/can/sja1000/sja1000_platform.c      |  4 ++--
 drivers/net/ethernet/altera/altera_tse_main.c   |  3 +--
 drivers/net/ethernet/ethoc.c                    |  8 +++----
 drivers/net/ethernet/lantiq_etop.c              |  4 ++--
 drivers/net/ethernet/ti/netcp_core.c            |  2 +-
 drivers/net/wireless/ath/ath9k/ahb.c            |  2 +-
 drivers/pci/dwc/pci-dra7xx.c                    |  2 +-
 drivers/pinctrl/bcm/pinctrl-ns2-mux.c           |  2 +-
 drivers/pinctrl/bcm/pinctrl-nsp-mux.c           |  4 ++--
 drivers/pinctrl/freescale/pinctrl-imx1-core.c   |  2 +-
 drivers/pinctrl/pinctrl-amd.c                   |  4 ++--
 drivers/platform/x86/intel_pmc_core.c           |  5 ++---
 drivers/regulator/ti-abb-regulator.c            |  6 ++---
 drivers/rtc/rtc-sh.c                            |  4 ++--
 drivers/spi/spi-jcore.c                         |  3 +--
 drivers/staging/fsl-mc/bus/mc-io.c              |  8 +++----
 drivers/tty/mips_ejtag_fdc.c                    |  4 ++--
 drivers/tty/serial/8250/8250_omap.c             |  3 +--
 drivers/tty/serial/lantiq.c                     |  3 +--
 drivers/tty/serial/meson_uart.c                 |  3 +--
 drivers/tty/serial/owl-uart.c                   |  2 +-
 drivers/tty/serial/pic32_uart.c                 |  4 ++--
 drivers/video/fbdev/mbx/mbxfb.c                 |  9 ++++----
 drivers/video/fbdev/mmp/hw/mmp_ctrl.c           |  2 +-
 drivers/video/fbdev/pxa168fb.c                  |  4 ++--
 drivers/watchdog/bcm63xx_wdt.c                  |  4 ++--
 drivers/watchdog/rc32434_wdt.c                  |  4 ++--
 include/linux/io.h                              |  2 --
 lib/devres.c                                    | 29 -------------------------
 scripts/coccinelle/free/devm_free.cocci         |  2 --
 sound/soc/au1x/ac97c.c                          |  4 ++--
 sound/soc/au1x/i2sc.c                           |  4 ++--
 sound/soc/intel/atom/sst/sst_acpi.c             | 20 ++++++++---------
 sound/soc/intel/boards/mfld_machine.c           |  4 ++--
 sound/soc/sh/fsi.c                              |  4 ++--
 tools/testing/nvdimm/Kbuild                     |  2 +-
 tools/testing/nvdimm/test/iomap.c               |  2 +-
 62 files changed, 109 insertions(+), 168 deletions(-)

-- 
1.8.3.1

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Greg KH]

On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
> Hi all,
> 
> When I tried to use devm_ioremap function and review related code, I found
> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
> except one use ioremap while the other use ioremap_nocache.

For all arches?  Really?  Look at MIPS, and x86, they have different
functions.

> While ioremap's
> default function is ioremap_nocache, so devm_ioremap_nocache also have the
> same function with devm_ioremap, which can just be killed to reduce the size
> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
> 
> I have posted two versions, which use macro instead of function for
> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
> devm_ioremap_nocache for no need to keep a macro around for the duplicate
> thing. So here comes v3 and please help to review.

I don't think this can be done, what am I missing?  These functions are
not identical, sorry for missing that before.

thanks,

greg k-h

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Guenter Roeck]

On 12/23/2017 05:48 AM, Greg KH wrote:
> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>> Hi all,
>>
>> When I tried to use devm_ioremap function and review related code, I found
>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>> except one use ioremap while the other use ioremap_nocache.
> 
> For all arches?  Really?  Look at MIPS, and x86, they have different
> functions.
> 

Both mips and x86 end up mapping the same function, but other arches don't.
mn10300 is one where ioremap and ioremap_nocache are definitely different.

Guenter

>> While ioremap's
>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>> same function with devm_ioremap, which can just be killed to reduce the size
>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>
>> I have posted two versions, which use macro instead of function for
>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>> thing. So here comes v3 and please help to review.
> 
> I don't think this can be done, what am I missing?  These functions are
> not identical, sorry for missing that before.
> 
> thanks,
> 
> greg k-h
> 

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[christophe leroy]

Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
> On 12/23/2017 05:48 AM, Greg KH wrote:
>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>> Hi all,
>>>
>>> When I tried to use devm_ioremap function and review related code, I 
>>> found
>>> devm_ioremap and devm_ioremap_nocache is almost the same with each 
>>> other,
>>> except one use ioremap while the other use ioremap_nocache.
>>
>> For all arches?  Really?  Look at MIPS, and x86, they have different
>> functions.
>>
> 
> Both mips and x86 end up mapping the same function, but other arches don't.
> mn10300 is one where ioremap and ioremap_nocache are definitely different.

alpha: identical
arc: identical
arm: identical
arm64: identical
cris: different        <==
frv: identical
hexagone: identical
ia64: different        <==
m32r: identical
m68k: identical
metag: identical
microblaze: identical
mips: identical
mn10300: different     <==
nios: identical
openrisc: different    <==
parisc: identical
riscv: identical
s390: identical
sh: identical
sparc: identical
tile: identical
um: rely on asm/generic
unicore32: identical
x86: identical
asm/generic (no mmu): identical

So 4 among all arches seems to have ioremap() and ioremap_nocache() 
being different.

Could we have a define set by the 4 arches on which ioremap() and 
ioremap_nocache() are different, something like 
HAVE_DIFFERENT_IOREMAP_NOCACHE ?

Christophe

> 
> Guenter
> 
>>> While ioremap's
>>> default function is ioremap_nocache, so devm_ioremap_nocache also 
>>> have the
>>> same function with devm_ioremap, which can just be killed to reduce 
>>> the size
>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>
>>> I have posted two versions, which use macro instead of function for
>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>> devm_ioremap_nocache for no need to keep a macro around for the 
>>> duplicate
>>> thing. So here comes v3 and please help to review.
>>
>> I don't think this can be done, what am I missing?  These functions are
>> not identical, sorry for missing that before.
>>
>> thanks,
>>
>> greg k-h
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe 
> linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[christophe leroy]

Le 23/12/2017 à 14:48, Greg KH a écrit :
> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>> Hi all,
>>
>> When I tried to use devm_ioremap function and review related code, I found
>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>> except one use ioremap while the other use ioremap_nocache.
> 
> For all arches?  Really?  Look at MIPS, and x86, they have different
> functions.
> 
>> While ioremap's
>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>> same function with devm_ioremap, which can just be killed to reduce the size
>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>
>> I have posted two versions, which use macro instead of function for
>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>> thing. So here comes v3 and please help to review.
> 
> I don't think this can be done, what am I missing?  These functions are
> not identical, sorry for missing that before.

devm_ioremap() and devm_ioremap_nocache() are quite similar, both use 
devm_ioremap_release() for the release, why not just defining:

static void __iomem *__devm_ioremap(struct device *dev, resource_size_t 
offset,
			   resource_size_t size, bool nocache)
{
[...]
	if (nocache)
		addr = ioremap_nocache(offset, size);
	else
		addr = ioremap(offset, size);
[...]
}

then in include/linux/io.h

static inline void __iomem *devm_ioremap(struct device *dev, 
resource_size_t offset,
			   resource_size_t size)
{return __devm_ioremap(dev, offset, size, false);}

static inline void __iomem *devm_ioremap_nocache(struct device *dev, 
resource_size_t offset,
				   resource_size_t size);
{return __devm_ioremap(dev, offset, size, true);}

Christophe

> 
> thanks,
> 
> greg k-h
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Yisheng Xie]

hi Christophe and Greg,

On 2017/12/24 16:55, christophe leroy wrote:
> 
> 
> Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
>> On 12/23/2017 05:48 AM, Greg KH wrote:
>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>> Hi all,
>>>>
>>>> When I tried to use devm_ioremap function and review related code, I found
>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>>> except one use ioremap while the other use ioremap_nocache.
>>>
>>> For all arches?  Really?  Look at MIPS, and x86, they have different
>>> functions.
>>>
>>
>> Both mips and x86 end up mapping the same function, but other arches don't.
>> mn10300 is one where ioremap and ioremap_nocache are definitely different.
> 
> alpha: identical
> arc: identical
> arm: identical
> arm64: identical
> cris: different        <==
> frv: identical
> hexagone: identical
> ia64: different        <==
> m32r: identical
> m68k: identical
> metag: identical
> microblaze: identical
> mips: identical
> mn10300: different     <==
> nios: identical
> openrisc: different    <==
> parisc: identical
> riscv: identical
> s390: identical
> sh: identical
> sparc: identical
> tile: identical
> um: rely on asm/generic
> unicore32: identical
> x86: identical
> asm/generic (no mmu): identical

Wow, that's correct, sorry for I have just checked the main archs, I means
x86,arm, arm64, mips.

However, I stall have no idea about why these 4 archs want different ioremap
function with others. Drivers seems cannot aware this? If driver call ioremap
want he really want for there 4 archs, cache or nocache?

> 
> So 4 among all arches seems to have ioremap() and ioremap_nocache() being different.
> 
> Could we have a define set by the 4 arches on which ioremap() and ioremap_nocache() are different, something like HAVE_DIFFERENT_IOREMAP_NOCACHE ?

Then, what the HAVE_DIFFERENT_IOREMAP_NOCACHE is uesed for ?

Thanks
Yisheng
> 
> Christophe
> 
>>
>> Guenter
>>
>>>> While ioremap's
>>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>>> same function with devm_ioremap, which can just be killed to reduce the size
>>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>>
>>>> I have posted two versions, which use macro instead of function for
>>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>>> thing. So here comes v3 and please help to review.
>>>
>>> I don't think this can be done, what am I missing?  These functions are
>>> not identical, sorry for missing that before.

Never mind, I should checked all the arches, sorry about that.

>>>
>>> thanks,
>>>
>>> greg k-h
>>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
> https://www.avast.com/antivirus
> 
> 
> .
> 

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Yisheng Xie]

On 2017/12/24 17:05, christophe leroy wrote:
> 
> 
> Le 23/12/2017 à 14:48, Greg KH a écrit :
>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>> Hi all,
>>>
>>> When I tried to use devm_ioremap function and review related code, I found
>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>> except one use ioremap while the other use ioremap_nocache.
>>
>> For all arches?  Really?  Look at MIPS, and x86, they have different
>> functions.
>>
>>> While ioremap's
>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>> same function with devm_ioremap, which can just be killed to reduce the size
>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>
>>> I have posted two versions, which use macro instead of function for
>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>> thing. So here comes v3 and please help to review.
>>
>> I don't think this can be done, what am I missing?  These functions are
>> not identical, sorry for missing that before.
> 
> devm_ioremap() and devm_ioremap_nocache() are quite similar, both use devm_ioremap_release() for the release, why not just defining:
> 
> static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>                resource_size_t size, bool nocache)
> {
> [...]
>     if (nocache)
>         addr = ioremap_nocache(offset, size);
>     else
>         addr = ioremap(offset, size);
> [...]
> }
> 
> then in include/linux/io.h
> 
> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>                resource_size_t size)
> {return __devm_ioremap(dev, offset, size, false);}
> 
> static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>                    resource_size_t size);
> {return __devm_ioremap(dev, offset, size, true);}

Yeah, this seems good to me, right now we have devm_ioremap, devm_ioremap_wc, devm_ioremap_nocache
May be we can use an enum like:
typedef enum {
	DEVM_IOREMAP = 0,
	DEVM_IOREMAP_NOCACHE,
	DEVM_IOREMAP_WC,
} devm_ioremap_type;

static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
                resource_size_t size)
 {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP);}

 static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
                    resource_size_t size);
 {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NOCACHE);}

 static inline void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
                    resource_size_t size);
 {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);}

 static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
                resource_size_t size, devm_ioremap_type type)
 {
     void __iomem **ptr, *addr = NULL;
 [...]
     switch (type){
     case DEVM_IOREMAP:
         addr = ioremap(offset, size);
         break;
     case DEVM_IOREMAP_NOCACHE:
         addr = ioremap_nocache(offset, size);
         break;
     case DEVM_IOREMAP_WC:
         addr = ioremap_wc(offset, size);
         break;
     }
 [...]
 }

Thanks
Yisheng

> 
> Christophe
> 
>>
>> thanks,
>>
>> greg k-h
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
> https://www.avast.com/antivirus
> 
> 
> .
> 

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Yisheng Xie]

+ cris/ia64/mn10300/openrisc maintainers

On 2017/12/25 9:09, Yisheng Xie wrote:
> hi Christophe and Greg,
> 
> On 2017/12/24 16:55, christophe leroy wrote:
>>
>>
>> Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
>>> On 12/23/2017 05:48 AM, Greg KH wrote:
>>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>>> Hi all,
>>>>>
>>>>> When I tried to use devm_ioremap function and review related code, I found
>>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>>>> except one use ioremap while the other use ioremap_nocache.
>>>>
>>>> For all arches?  Really?  Look at MIPS, and x86, they have different
>>>> functions.
>>>>
>>>
>>> Both mips and x86 end up mapping the same function, but other arches don't.
>>> mn10300 is one where ioremap and ioremap_nocache are definitely different.
>>
>> alpha: identical
>> arc: identical
>> arm: identical
>> arm64: identical
>> cris: different        <==
>> frv: identical
>> hexagone: identical
>> ia64: different        <==
>> m32r: identical
>> m68k: identical
>> metag: identical
>> microblaze: identical
>> mips: identical
>> mn10300: different     <==
>> nios: identical
>> openrisc: different    <==
>> parisc: identical
>> riscv: identical
>> s390: identical
>> sh: identical
>> sparc: identical
>> tile: identical
>> um: rely on asm/generic
>> unicore32: identical
>> x86: identical
>> asm/generic (no mmu): identical
> 
> Wow, that's correct, sorry for I have just checked the main archs, I means
> x86,arm, arm64, mips.
> 
> However, I stall have no idea about why these 4 archs want different ioremap
> function with others. Drivers seems cannot aware this? If driver call ioremap
> want he really want for there 4 archs, cache or nocache?

Could you please help about this? it is out of my knowledge.

Thanks
Yisheng

> 
>>
>> So 4 among all arches seems to have ioremap() and ioremap_nocache() being different.
>>
>> Could we have a define set by the 4 arches on which ioremap() and ioremap_nocache() are different, something like HAVE_DIFFERENT_IOREMAP_NOCACHE ?
> 
> Then, what the HAVE_DIFFERENT_IOREMAP_NOCACHE is uesed for ?
> 
> Thanks
> Yisheng
>>
>> Christophe
>>
>>>
>>> Guenter
>>>
>>>>> While ioremap's
>>>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>>>> same function with devm_ioremap, which can just be killed to reduce the size
>>>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>>>
>>>>> I have posted two versions, which use macro instead of function for
>>>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>>>> thing. So here comes v3 and please help to review.
>>>>
>>>> I don't think this can be done, what am I missing?  These functions are
>>>> not identical, sorry for missing that before.
> 
> Never mind, I should checked all the arches, sorry about that.
> 
>>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>>>
>>>
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> ---
>> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
>> https://www.avast.com/antivirus
>>
>>
>> .
>>
> 
> 
> .
> 

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Arnd Bergmann]

On Sun, Dec 24, 2017 at 9:55 AM, christophe leroy
<christophe.leroy@c-s.fr> wrote:
> Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
>>
>> On 12/23/2017 05:48 AM, Greg KH wrote:
>>>
>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>>
>>>> Hi all,
>>>>
>>>> When I tried to use devm_ioremap function and review related code, I
>>>> found
>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each
>>>> other,
>>>> except one use ioremap while the other use ioremap_nocache.
>>>
>>>
>>> For all arches?  Really?  Look at MIPS, and x86, they have different
>>> functions.
>>>
>>
>> Both mips and x86 end up mapping the same function, but other arches
>> don't.
>> mn10300 is one where ioremap and ioremap_nocache are definitely different.
>
>
> alpha: identical
> arc: identical
> arm: identical
> arm64: identical
> cris: different        <==
> frv: identical
> hexagone: identical
> ia64: different        <==
> m32r: identical
> m68k: identical
> metag: identical
> microblaze: identical
> mips: identical
> mn10300: different     <==
> nios: identical
> openrisc: different    <==
> parisc: identical
> riscv: identical
> s390: identical
> sh: identical
> sparc: identical
> tile: identical
> um: rely on asm/generic
> unicore32: identical
> x86: identical
> asm/generic (no mmu): identical
>
> So 4 among all arches seems to have ioremap() and ioremap_nocache() being
> different.
>
> Could we have a define set by the 4 arches on which ioremap() and
> ioremap_nocache() are different, something like
> HAVE_DIFFERENT_IOREMAP_NOCACHE ?

I wonder if those are actually correct or not. What I found looking at
those architectures:

- openrisc only has one driver using ioremap (drivers/net/ethernet/ethoc.c)
  and that calls ioremap_nocache(). Presumably the authors went with the
  implementation for ioremap that made sense (using default attributes)
  rather than the one that actually works (using uncached).

- On ia64, ioremap() checks the attributes for the physical
  address based on firmware tables and then picks either cached
  or uncached mappings. ioremap_nocache() does the same but
  returns NULL instead of a cached mapping for anything that is
  not an MMIO address. Presumably it would just work to always
  call ioremap().

- mn10300 appears to be wrong, broken by David Howells in
  commit 83c2dc15ce82 ("MN10300: Handle cacheable PCI regions
  in pci_iomap()") for any driver calling ioremap() by to get uncached
  memory, if I understand the comment for commit 34f1bdee1910
   ("mn10300: switch to GENERIC_PCI_IOMAP") correctly: it
  seems that PCI addresses include the 'uncached' bit by default
  to get the right behavior, but dropping that bit breaks it.

- cris seems similar to mn10300 in hardware, using an phys address
  bit for uncached access. There are two callers in arch code that
  appear to rely on the cachable output of ioremap()
arch/cris/arch-v32/kernel/signal.c:
__ioremap_prot(virt_to_phys(data), PAGE_SIZE, PAGE_SIGNAL_TRAMPOLINE);
arch/cris/arch-v32/mm/intmem.c:         intmem_virtual =
ioremap(MEM_INTMEM_START + RESERVED_SIZE,
  It's unclear whether ioremap_nocache() actually has any users
  on cris, or whether it was only added for compile-time testing,
  and calling plain ioremap() would always work too (assuming we
  pass the phys address with the uncached-bit set).

       Arnd
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Christophe LEROY]

Le 25/12/2017 à 02:34, Yisheng Xie a écrit :
> 
> 
> On 2017/12/24 17:05, christophe leroy wrote:
>>
>>
>> Le 23/12/2017 à 14:48, Greg KH a écrit :
>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>> Hi all,
>>>>
>>>> When I tried to use devm_ioremap function and review related code, I found
>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>>> except one use ioremap while the other use ioremap_nocache.
>>>
>>> For all arches?  Really?  Look at MIPS, and x86, they have different
>>> functions.
>>>
>>>> While ioremap's
>>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>>> same function with devm_ioremap, which can just be killed to reduce the size
>>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>>
>>>> I have posted two versions, which use macro instead of function for
>>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>>> thing. So here comes v3 and please help to review.
>>>
>>> I don't think this can be done, what am I missing?  These functions are
>>> not identical, sorry for missing that before.
>>
>> devm_ioremap() and devm_ioremap_nocache() are quite similar, both use devm_ioremap_release() for the release, why not just defining:
>>
>> static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>>                 resource_size_t size, bool nocache)
>> {
>> [...]
>>      if (nocache)
>>          addr = ioremap_nocache(offset, size);
>>      else
>>          addr = ioremap(offset, size);
>> [...]
>> }
>>
>> then in include/linux/io.h
>>
>> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>>                 resource_size_t size)
>> {return __devm_ioremap(dev, offset, size, false);}
>>
>> static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>>                     resource_size_t size);
>> {return __devm_ioremap(dev, offset, size, true);}
> 
> Yeah, this seems good to me, right now we have devm_ioremap, devm_ioremap_wc, devm_ioremap_nocache
> May be we can use an enum like:
> typedef enum {
> 	DEVM_IOREMAP = 0,
> 	DEVM_IOREMAP_NOCACHE,
> 	DEVM_IOREMAP_WC,
> } devm_ioremap_type;
> 
> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>                  resource_size_t size)
>   {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP);}
> 
>   static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>                      resource_size_t size);
>   {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NOCACHE);}
> 
>   static inline void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>                      resource_size_t size);
>   {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);}
> 
>   static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>                  resource_size_t size, devm_ioremap_type type)
>   {
>       void __iomem **ptr, *addr = NULL;
>   [...]
>       switch (type){
>       case DEVM_IOREMAP:
>           addr = ioremap(offset, size);
>           break;
>       case DEVM_IOREMAP_NOCACHE:
>           addr = ioremap_nocache(offset, size);
>           break;
>       case DEVM_IOREMAP_WC:
>           addr = ioremap_wc(offset, size);
>           break;
>       }
>   [...]
>   }


That looks good to me, will you submit a v4 ?

Christophe

> 
> Thanks
> Yisheng
> 
>>
>> Christophe
>>
>>>
>>> thanks,
>>>
>>> greg k-h
>>> -- 
>>> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>> ---
>> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
>> https://www.avast.com/antivirus
>>
>>
>> .
>>

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[David Howells]

Arnd Bergmann <arnd@arndb.de> wrote:

> - mn10300 appears to be wrong, broken by David Howells in
>   commit 83c2dc15ce82 ("MN10300: Handle cacheable PCI regions
>   in pci_iomap()") for any driver calling ioremap() by to get uncached
>   memory,

It's not clear what the right thing to do was, given that there's an ioremap()
and an ioremap_uncached().

But the asb2305's pci_iomap() will use ioremap() (the cacheable window) if
IORESOURCE_CACHEABLE is set, but IORESOURCE_IO is not and ioremap_uncached()
otherwise.

The other supported units don't have PCI buses.

> if I understand the comment for commit 34f1bdee1910 ("mn10300: switch to
> GENERIC_PCI_IOMAP") correctly: it seems that PCI addresses include the
> 'uncached' bit by default to get the right behavior, but dropping that bit
> breaks it.

Not exactly.  The CPU has a window in the range 0xa0000000-0xbfffffff which is
an uncached view of its hardware buses.  It has another window in the range
0x80000000-0x9fffffff which is a cached view of that region.  These windows
cannot be changed and addresses above 0x80000000 are statically mapped and are
only accessible by the kernel (this is hardwired in the MMU).

So the arch has two subwindows to the PCI bus, one cached and one uncached.
These subwindows are further subdivided into ioport and iomem spaces, an SRAM
and some control registers for the CPU-PCI bridge.

David
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 00/27] kill devm_ioremap_nocache

[Yisheng Xie]

Hi Christophe ，

On 2018/1/4 16:05, Christophe LEROY wrote:
> 
> 
> Le 25/12/2017 à 02:34, Yisheng Xie a écrit :
>>
>>
>> On 2017/12/24 17:05, christophe leroy wrote:
>>>
>>>
>>> Le 23/12/2017 à 14:48, Greg KH a écrit :
>>>> On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
>>>>> Hi all,
>>>>>
>>>>> When I tried to use devm_ioremap function and review related code, I found
>>>>> devm_ioremap and devm_ioremap_nocache is almost the same with each other,
>>>>> except one use ioremap while the other use ioremap_nocache.
>>>>
>>>> For all arches?  Really?  Look at MIPS, and x86, they have different
>>>> functions.
>>>>
>>>>> While ioremap's
>>>>> default function is ioremap_nocache, so devm_ioremap_nocache also have the
>>>>> same function with devm_ioremap, which can just be killed to reduce the size
>>>>> of devres.o(from 20304 bytes to 18992 bytes in my compile environment).
>>>>>
>>>>> I have posted two versions, which use macro instead of function for
>>>>> devm_ioremap_nocache[1] or devm_ioremap[2]. And Greg suggest me to kill
>>>>> devm_ioremap_nocache for no need to keep a macro around for the duplicate
>>>>> thing. So here comes v3 and please help to review.
>>>>
>>>> I don't think this can be done, what am I missing?  These functions are
>>>> not identical, sorry for missing that before.
>>>
>>> devm_ioremap() and devm_ioremap_nocache() are quite similar, both use devm_ioremap_release() for the release, why not just defining:
>>>
>>> static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>>>                 resource_size_t size, bool nocache)
>>> {
>>> [...]
>>>      if (nocache)
>>>          addr = ioremap_nocache(offset, size);
>>>      else
>>>          addr = ioremap(offset, size);
>>> [...]
>>> }
>>>
>>> then in include/linux/io.h
>>>
>>> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>>>                 resource_size_t size)
>>> {return __devm_ioremap(dev, offset, size, false);}
>>>
>>> static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>>>                     resource_size_t size);
>>> {return __devm_ioremap(dev, offset, size, true);}
>>
>> Yeah, this seems good to me, right now we have devm_ioremap, devm_ioremap_wc, devm_ioremap_nocache
>> May be we can use an enum like:
>> typedef enum {
>>     DEVM_IOREMAP = 0,
>>     DEVM_IOREMAP_NOCACHE,
>>     DEVM_IOREMAP_WC,
>> } devm_ioremap_type;
>>
>> static inline void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>>                  resource_size_t size)
>>   {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP);}
>>
>>   static inline void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>>                      resource_size_t size);
>>   {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NOCACHE);}
>>
>>   static inline void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>>                      resource_size_t size);
>>   {return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);}
>>
>>   static void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>>                  resource_size_t size, devm_ioremap_type type)
>>   {
>>       void __iomem **ptr, *addr = NULL;
>>   [...]
>>       switch (type){
>>       case DEVM_IOREMAP:
>>           addr = ioremap(offset, size);
>>           break;
>>       case DEVM_IOREMAP_NOCACHE:
>>           addr = ioremap_nocache(offset, size);
>>           break;
>>       case DEVM_IOREMAP_WC:
>>           addr = ioremap_wc(offset, size);
>>           break;
>>       }
>>   [...]
>>   }
> 
> 
> That looks good to me, will you submit a v4 ?

Sorry for late response. And I will submit the v4 as your suggestion.

Thanks
Yisheng

> 
> Christophe
> 
>>