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
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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
quoted
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
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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
quoted
quoted
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
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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.
quoted
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
hi Christophe and Greg,
On 2017/12/24 16:55, christophe leroy wrote:
Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
quoted
On 12/23/2017 05:48 AM, Greg KH wrote:
quoted
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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
quoted
Guenter
quoted
quoted
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.
quoted
quoted
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
.
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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.
quoted
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
quoted
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
.
+ 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:
quoted
Le 23/12/2017 à 16:57, Guenter Roeck a écrit :
quoted
On 12/23/2017 05:48 AM, Greg KH wrote:
quoted
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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
quoted
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
quoted
Christophe
quoted
Guenter
quoted
quoted
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.
quoted
quoted
quoted
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
.
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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.
quoted
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
quoted
Christophe
quoted
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
.
Hi Christophe ,
On 2018/1/4 16:05, Christophe LEROY wrote:
Le 25/12/2017 à 02:34, Yisheng Xie a écrit :
quoted
On 2017/12/24 17:05, christophe leroy wrote:
quoted
Le 23/12/2017 à 14:48, Greg KH a écrit :
quoted
On Sat, Dec 23, 2017 at 06:55:25PM +0800, Yisheng Xie wrote:
quoted
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.
quoted
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