* [PATCH v4] devres: combine function devm_ioremap*
@ 2018-01-12 11:51 Yisheng Xie
2018-01-13 16:39 ` christophe leroy
0 siblings, 1 reply; 3+ messages in thread
From: Yisheng Xie @ 2018-01-12 11:51 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, thomas.lendacky, christophe.leroy, Yisheng Xie
When I tried to use devm_ioremap function and review related
code, I found devm_ioremap_* almost have the similar realize
with each other, which can be combined.
In the former versions, I have tried to kill ioremap_cache to
reduce the size of devres, which can not work for ioremap is
not the same as ioremap_nocache in some ARCHs likes ia64.
Therefore, as the suggestion of Christophe, I introduce a help
function __devm_ioremap, let devm_ioremap* inline and call
__devm_ioremap with different devm_ioremap_type.
After apply the patch, the size of devres.o can be reduce from
8216 Bytes to 7352Bytes in my compile environment.
Suggested-by: Christophe LEROY <christophe.leroy@c-s.fr>
Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
---
v2:
- use MARCO for ioremap
v3:
- kill dev_ioremap_nocache
v4:
- combine function devm_ioremap*
include/linux/io.h | 33 +++++++++++++++++++-----
lib/devres.c | 76 +++++++++++-------------------------------------------
2 files changed, 42 insertions(+), 67 deletions(-)
diff --git a/include/linux/io.h b/include/linux/io.h
index 32e30e8..da33a5f 100644
--- a/include/linux/io.h
+++ b/include/linux/io.h
@@ -73,12 +73,33 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
#define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
-void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
- resource_size_t size);
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
- resource_size_t size);
-void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
- resource_size_t size);
+typedef enum {
+ DEVM_IOREMAP = 0,
+ DEVM_IOREMAP_NC,
+ DEVM_IOREMAP_WC,
+} devm_ioremap_type;
+
+void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
+ resource_size_t size, devm_ioremap_type 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_NC);
+}
+
+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);
+}
+
void devm_iounmap(struct device *dev, void __iomem *addr);
int check_signature(const volatile void __iomem *io_addr,
const unsigned char *signature, int length);
diff --git a/lib/devres.c b/lib/devres.c
index 5f2aedd..e00eb4f 100644
--- a/lib/devres.c
+++ b/lib/devres.c
@@ -23,73 +23,27 @@ static int devm_ioremap_match(struct device *dev, void *res, void *match_data)
*
* Managed ioremap(). Map is automatically unmapped on driver detach.
*/
-void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
- resource_size_t size)
+void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
+ resource_size_t size, devm_ioremap_type type)
{
- void __iomem **ptr, *addr;
-
- ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
-
- addr = ioremap(offset, size);
- if (addr) {
- *ptr = addr;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return addr;
-}
-EXPORT_SYMBOL(devm_ioremap);
-
-/**
- * devm_ioremap_nocache - Managed ioremap_nocache()
- * @dev: Generic device to remap IO address for
- * @offset: Resource address to map
- * @size: Size of map
- *
- * Managed ioremap_nocache(). Map is automatically unmapped on driver
- * detach.
- */
-void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
- resource_size_t size)
-{
- void __iomem **ptr, *addr;
+ void __iomem **ptr, *addr = NULL;
ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
if (!ptr)
return NULL;
- addr = ioremap_nocache(offset, size);
- if (addr) {
- *ptr = addr;
- devres_add(dev, ptr);
- } else
- devres_free(ptr);
-
- return addr;
-}
-EXPORT_SYMBOL(devm_ioremap_nocache);
-
-/**
- * devm_ioremap_wc - Managed ioremap_wc()
- * @dev: Generic device to remap IO address for
- * @offset: Resource address to map
- * @size: Size of map
- *
- * Managed ioremap_wc(). Map is automatically unmapped on driver detach.
- */
-void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
- resource_size_t size)
-{
- void __iomem **ptr, *addr;
-
- ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
- if (!ptr)
- return NULL;
+ switch (type) {
+ case DEVM_IOREMAP:
+ addr = ioremap(offset, size);
+ break;
+ case DEVM_IOREMAP_NC:
+ addr = ioremap_nocache(offset, size);
+ break;
+ case DEVM_IOREMAP_WC:
+ addr = ioremap_wc(offset, size);
+ break;
+ }
- addr = ioremap_wc(offset, size);
if (addr) {
*ptr = addr;
devres_add(dev, ptr);
@@ -98,7 +52,7 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
return addr;
}
-EXPORT_SYMBOL(devm_ioremap_wc);
+EXPORT_SYMBOL(__devm_ioremap);
/**
* devm_iounmap - Managed iounmap()
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] devres: combine function devm_ioremap*
2018-01-12 11:51 [PATCH v4] devres: combine function devm_ioremap* Yisheng Xie
@ 2018-01-13 16:39 ` christophe leroy
2018-01-15 1:06 ` Yisheng Xie
0 siblings, 1 reply; 3+ messages in thread
From: christophe leroy @ 2018-01-13 16:39 UTC (permalink / raw)
To: Yisheng Xie, gregkh; +Cc: linux-kernel, thomas.lendacky
Hi Yisheng,
Le 12/01/2018 à 12:51, Yisheng Xie a écrit :
> When I tried to use devm_ioremap function and review related
> code, I found devm_ioremap_* almost have the similar realize
> with each other, which can be combined.
>
> In the former versions, I have tried to kill ioremap_cache to
> reduce the size of devres, which can not work for ioremap is
> not the same as ioremap_nocache in some ARCHs likes ia64.
> Therefore, as the suggestion of Christophe, I introduce a help
> function __devm_ioremap, let devm_ioremap* inline and call
> __devm_ioremap with different devm_ioremap_type.
>
> After apply the patch, the size of devres.o can be reduce from
> 8216 Bytes to 7352Bytes in my compile environment.
>
> Suggested-by: Christophe LEROY <christophe.leroy@c-s.fr>
> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
> ---
> v2:
> - use MARCO for ioremap
> v3:
> - kill dev_ioremap_nocache
> v4:
> - combine function devm_ioremap*
>
> include/linux/io.h | 33 +++++++++++++++++++-----
> lib/devres.c | 76 +++++++++++-------------------------------------------
> 2 files changed, 42 insertions(+), 67 deletions(-)
>
> diff --git a/include/linux/io.h b/include/linux/io.h
> index 32e30e8..da33a5f 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -73,12 +73,33 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
>
> #define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
>
> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> - resource_size_t size);
> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> - resource_size_t size);
> -void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> - resource_size_t size);
> +typedef enum {
> + DEVM_IOREMAP = 0,
> + DEVM_IOREMAP_NC,
> + DEVM_IOREMAP_WC,
> +} devm_ioremap_type;
In accordance with Kernel codying style, you should not define a new
typedef for that.
Instead, use:
enum devm_ioremap_type {
DEVM_IOREMAP = 0,
DEVM_IOREMAP_NC,
DEVM_IOREMAP_WC,
};
> +
> +void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
> + resource_size_t size, devm_ioremap_type type);
Use 'enum devm_ioremap_type type' instead of 'devm_ioremap_type'
And alignment of second line should match open parenthesis.
> +
> +static inline void __iomem *devm_ioremap(struct device *dev,
> + resource_size_t offset, resource_size_t size)
Alignment of second line should match open parenthesis.
> +{
> + 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)
Alignment of second line should match open parenthesis.
> +{
> + return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NC);
> +}
> +
> +static inline void __iomem *devm_ioremap_wc(struct device *dev,
> + resource_size_t offset, resource_size_t size)
Alignment of second line should match open parenthesis.
> +{
> + return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);
> +}
> +
> void devm_iounmap(struct device *dev, void __iomem *addr);
> int check_signature(const volatile void __iomem *io_addr,
> const unsigned char *signature, int length);
> diff --git a/lib/devres.c b/lib/devres.c
> index 5f2aedd..e00eb4f 100644
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -23,73 +23,27 @@ static int devm_ioremap_match(struct device *dev, void *res, void *match_data)
> *
> * Managed ioremap(). Map is automatically unmapped on driver detach.
> */
That comment should be moved to io.h
> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> - resource_size_t size)
> +void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
> + resource_size_t size, devm_ioremap_type type)
Alignment of second line should match open parenthesis.
> {
> - void __iomem **ptr, *addr;
> -
> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> -
> - addr = ioremap(offset, size);
> - if (addr) {
> - *ptr = addr;
> - devres_add(dev, ptr);
> - } else
> - devres_free(ptr);
> -
> - return addr;
> -}
> -EXPORT_SYMBOL(devm_ioremap);
> -
> -/**
> - * devm_ioremap_nocache - Managed ioremap_nocache()
> - * @dev: Generic device to remap IO address for
> - * @offset: Resource address to map
> - * @size: Size of map
> - *
> - * Managed ioremap_nocache(). Map is automatically unmapped on driver
> - * detach.
> - */
That comment should be moved to io.h
> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
> - resource_size_t size)
> -{
> - void __iomem **ptr, *addr;
> + void __iomem **ptr, *addr = NULL;
>
> ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> if (!ptr)
> return NULL;
>
> - addr = ioremap_nocache(offset, size);
> - if (addr) {
> - *ptr = addr;
> - devres_add(dev, ptr);
> - } else
> - devres_free(ptr);
> -
> - return addr;
> -}
> -EXPORT_SYMBOL(devm_ioremap_nocache);
> -
> -/**
> - * devm_ioremap_wc - Managed ioremap_wc()
> - * @dev: Generic device to remap IO address for
> - * @offset: Resource address to map
> - * @size: Size of map
> - *
> - * Managed ioremap_wc(). Map is automatically unmapped on driver detach.
> - */
That comment should be moved to io.h
> -void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> - resource_size_t size)
> -{
> - void __iomem **ptr, *addr;
> -
> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
> - if (!ptr)
> - return NULL;
> + switch (type) {
> + case DEVM_IOREMAP:
> + addr = ioremap(offset, size);
> + break;
> + case DEVM_IOREMAP_NC:
> + addr = ioremap_nocache(offset, size);
> + break;
> + case DEVM_IOREMAP_WC:
> + addr = ioremap_wc(offset, size);
> + break;
> + }
>
> - addr = ioremap_wc(offset, size);
> if (addr) {
> *ptr = addr;
> devres_add(dev, ptr);
> @@ -98,7 +52,7 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>
> return addr;
> }
> -EXPORT_SYMBOL(devm_ioremap_wc);
> +EXPORT_SYMBOL(__devm_ioremap);
>
> /**
> * devm_iounmap - Managed iounmap()
>
Christophe
---
L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
https://www.avast.com/antivirus
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] devres: combine function devm_ioremap*
2018-01-13 16:39 ` christophe leroy
@ 2018-01-15 1:06 ` Yisheng Xie
0 siblings, 0 replies; 3+ messages in thread
From: Yisheng Xie @ 2018-01-15 1:06 UTC (permalink / raw)
To: christophe leroy, gregkh; +Cc: linux-kernel, thomas.lendacky
Hi Christophe,
Thanks for your review and comment, I will follow your suggestion listed in line.
Thanks
Yisheng
On 2018/1/14 0:39, christophe leroy wrote:
> Hi Yisheng,
>
> Le 12/01/2018 à 12:51, Yisheng Xie a écrit :
>> When I tried to use devm_ioremap function and review related
>> code, I found devm_ioremap_* almost have the similar realize
>> with each other, which can be combined.
>>
>> In the former versions, I have tried to kill ioremap_cache to
>> reduce the size of devres, which can not work for ioremap is
>> not the same as ioremap_nocache in some ARCHs likes ia64.
>> Therefore, as the suggestion of Christophe, I introduce a help
>> function __devm_ioremap, let devm_ioremap* inline and call
>> __devm_ioremap with different devm_ioremap_type.
>>
>> After apply the patch, the size of devres.o can be reduce from
>> 8216 Bytes to 7352Bytes in my compile environment.
>>
>> Suggested-by: Christophe LEROY <christophe.leroy@c-s.fr>
>> Signed-off-by: Yisheng Xie <xieyisheng1@huawei.com>
>> ---
>> v2:
>> - use MARCO for ioremap
>> v3:
>> - kill dev_ioremap_nocache
>> v4:
>> - combine function devm_ioremap*
>>
>> include/linux/io.h | 33 +++++++++++++++++++-----
>> lib/devres.c | 76 +++++++++++-------------------------------------------
>> 2 files changed, 42 insertions(+), 67 deletions(-)
>>
>> diff --git a/include/linux/io.h b/include/linux/io.h
>> index 32e30e8..da33a5f 100644
>> --- a/include/linux/io.h
>> +++ b/include/linux/io.h
>> @@ -73,12 +73,33 @@ static inline void devm_ioport_unmap(struct device *dev, void __iomem *addr)
>> #define IOMEM_ERR_PTR(err) (__force void __iomem *)ERR_PTR(err)
>> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>> - resource_size_t size);
>> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>> - resource_size_t size);
>> -void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>> - resource_size_t size);
>> +typedef enum {
>> + DEVM_IOREMAP = 0,
>> + DEVM_IOREMAP_NC,
>> + DEVM_IOREMAP_WC,
>> +} devm_ioremap_type;
>
> In accordance with Kernel codying style, you should not define a new typedef for that.
>
> Instead, use:
>
> enum devm_ioremap_type {
> DEVM_IOREMAP = 0,
> DEVM_IOREMAP_NC,
> DEVM_IOREMAP_WC,
> };
>
>> +
>> +void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>> + resource_size_t size, devm_ioremap_type type);
>
> Use 'enum devm_ioremap_type type' instead of 'devm_ioremap_type'
>
> And alignment of second line should match open parenthesis.
>
>> +
>> +static inline void __iomem *devm_ioremap(struct device *dev,
>> + resource_size_t offset, resource_size_t size)
>
> Alignment of second line should match open parenthesis.
>
>> +{
>> + 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)
>
> Alignment of second line should match open parenthesis.
>
>> +{
>> + return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NC);
>> +}
>> +
>> +static inline void __iomem *devm_ioremap_wc(struct device *dev,
>> + resource_size_t offset, resource_size_t size)
>
> Alignment of second line should match open parenthesis.
>
>> +{
>> + return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);
>> +}
>> +
>> void devm_iounmap(struct device *dev, void __iomem *addr);
>> int check_signature(const volatile void __iomem *io_addr,
>> const unsigned char *signature, int length);
>> diff --git a/lib/devres.c b/lib/devres.c
>> index 5f2aedd..e00eb4f 100644
>> --- a/lib/devres.c
>> +++ b/lib/devres.c
>> @@ -23,73 +23,27 @@ static int devm_ioremap_match(struct device *dev, void *res, void *match_data)
>> *
>> * Managed ioremap(). Map is automatically unmapped on driver detach.
>> */
>
> That comment should be moved to io.h
>
>> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>> - resource_size_t size)
>> +void __iomem *__devm_ioremap(struct device *dev, resource_size_t offset,
>> + resource_size_t size, devm_ioremap_type type)
>
> Alignment of second line should match open parenthesis.
>
>> {
>> - void __iomem **ptr, *addr;
>> -
>> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
>> - if (!ptr)
>> - return NULL;
>> -
>> - addr = ioremap(offset, size);
>> - if (addr) {
>> - *ptr = addr;
>> - devres_add(dev, ptr);
>> - } else
>> - devres_free(ptr);
>> -
>> - return addr;
>> -}
>> -EXPORT_SYMBOL(devm_ioremap);
>> -
>> -/**
>> - * devm_ioremap_nocache - Managed ioremap_nocache()
>> - * @dev: Generic device to remap IO address for
>> - * @offset: Resource address to map
>> - * @size: Size of map
>> - *
>> - * Managed ioremap_nocache(). Map is automatically unmapped on driver
>> - * detach.
>> - */
>
> That comment should be moved to io.h
>
>> -void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t offset,
>> - resource_size_t size)
>> -{
>> - void __iomem **ptr, *addr;
>> + void __iomem **ptr, *addr = NULL;
>> ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
>> if (!ptr)
>> return NULL;
>> - addr = ioremap_nocache(offset, size);
>> - if (addr) {
>> - *ptr = addr;
>> - devres_add(dev, ptr);
>> - } else
>> - devres_free(ptr);
>> -
>> - return addr;
>> -}
>> -EXPORT_SYMBOL(devm_ioremap_nocache);
>> -
>> -/**
>> - * devm_ioremap_wc - Managed ioremap_wc()
>> - * @dev: Generic device to remap IO address for
>> - * @offset: Resource address to map
>> - * @size: Size of map
>> - *
>> - * Managed ioremap_wc(). Map is automatically unmapped on driver detach.
>> - */
>
> That comment should be moved to io.h
>
>> -void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>> - resource_size_t size)
>> -{
>> - void __iomem **ptr, *addr;
>> -
>> - ptr = devres_alloc(devm_ioremap_release, sizeof(*ptr), GFP_KERNEL);
>> - if (!ptr)
>> - return NULL;
>> + switch (type) {
>> + case DEVM_IOREMAP:
>> + addr = ioremap(offset, size);
>> + break;
>> + case DEVM_IOREMAP_NC:
>> + addr = ioremap_nocache(offset, size);
>> + break;
>> + case DEVM_IOREMAP_WC:
>> + addr = ioremap_wc(offset, size);
>> + break;
>> + }
>> - addr = ioremap_wc(offset, size);
>> if (addr) {
>> *ptr = addr;
>> devres_add(dev, ptr);
>> @@ -98,7 +52,7 @@ void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
>> return addr;
>> }
>> -EXPORT_SYMBOL(devm_ioremap_wc);
>> +EXPORT_SYMBOL(__devm_ioremap);
>> /**
>> * devm_iounmap - Managed iounmap()
>>
>
> Christophe
>
> ---
> L'absence de virus dans ce courrier électronique a été vérifiée par le logiciel antivirus Avast.
> https://www.avast.com/antivirus
>
>
> .
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-15 1:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-12 11:51 [PATCH v4] devres: combine function devm_ioremap* Yisheng Xie
2018-01-13 16:39 ` christophe leroy
2018-01-15 1:06 ` Yisheng Xie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).