Hi Greg, Christophe,

Any comment about this version?  And sorry to disturb  :)

Thanks
Yisheng

On 2018/1/29 19:48, Yisheng Xie wrote:
> 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 version, 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 8052 Bytes in my compile environment.
> 
> Suggested-by: Greg KH <gre...@linuxfoundation.org>
> Suggested-by: Christophe LEROY <christophe.le...@c-s.fr>
> Signed-off-by: Yisheng Xie <xieyishe...@huawei.com>
> ---
> v2:
>  - use MARCO for ioremap
> v3:
>  - kill dev_ioremap_nocache
> v4:
>  - combine function devm_ioremap*       - per Christophe
> v5:
>  - fix code style.                      - per Christophe
> v6:
>  - just put the cleanup in the devres.c - per Greg
> 
>  lib/devres.c | 78 
> +++++++++++++++++++++++++++++-------------------------------
>  1 file changed, 38 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/devres.c b/lib/devres.c
> index 5f2aedd..5bec112 100644
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -5,6 +5,12 @@
>  #include <linux/gfp.h>
>  #include <linux/export.h>
>  
> +enum devm_ioremap_type {
> +     DEVM_IOREMAP = 0,
> +     DEVM_IOREMAP_NC,
> +     DEVM_IOREMAP_WC,
> +};
> +
>  void devm_ioremap_release(struct device *dev, void *res)
>  {
>       iounmap(*(void __iomem **)res);
> @@ -15,24 +21,28 @@ static int devm_ioremap_match(struct device *dev, void 
> *res, void *match_data)
>       return *(void **)res == match_data;
>  }
>  
> -/**
> - * devm_ioremap - Managed ioremap()
> - * @dev: Generic device to remap IO address for
> - * @offset: Resource address to map
> - * @size: Size of map
> - *
> - * Managed ioremap().  Map is automatically unmapped on driver detach.
> - */
> -void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> -                        resource_size_t size)
> +static void __iomem *__devm_ioremap(struct device *dev, resource_size_t 
> offset,
> +                                 resource_size_t size,
> +                                 enum devm_ioremap_type type)
>  {
> -     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(offset, size);
> +     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;
> +     }
> +
>       if (addr) {
>               *ptr = addr;
>               devres_add(dev, ptr);
> @@ -41,6 +51,20 @@ void __iomem *devm_ioremap(struct device *dev, 
> resource_size_t offset,
>  
>       return addr;
>  }
> +
> +/**
> + * devm_ioremap - Managed ioremap()
> + * @dev: Generic device to remap IO address for
> + * @offset: Resource address to map
> + * @size: Size of map
> + *
> + * Managed ioremap().  Map is automatically unmapped on driver detach.
> + */
> +void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
> +                        resource_size_t size)
> +{
> +     return __devm_ioremap(dev, offset, size, DEVM_IOREMAP);
> +}
>  EXPORT_SYMBOL(devm_ioremap);
>  
>  /**
> @@ -55,20 +79,7 @@ void __iomem *devm_ioremap(struct device *dev, 
> resource_size_t offset,
>  void __iomem *devm_ioremap_nocache(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;
> -
> -     addr = ioremap_nocache(offset, size);
> -     if (addr) {
> -             *ptr = addr;
> -             devres_add(dev, ptr);
> -     } else
> -             devres_free(ptr);
> -
> -     return addr;
> +     return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_NC);
>  }
>  EXPORT_SYMBOL(devm_ioremap_nocache);
>  
> @@ -83,20 +94,7 @@ void __iomem *devm_ioremap_nocache(struct device *dev, 
> resource_size_t offset,
>  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;
> -
> -     addr = ioremap_wc(offset, size);
> -     if (addr) {
> -             *ptr = addr;
> -             devres_add(dev, ptr);
> -     } else
> -             devres_free(ptr);
> -
> -     return addr;
> +     return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_WC);
>  }
>  EXPORT_SYMBOL(devm_ioremap_wc);
>  
> 

Reply via email to