lkml.org 
[lkml]   [2018]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v3 00/27] kill devm_ioremap_nocache
    From
    Date
    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
    >
    >>

    \
     
     \ /
      Last update: 2018-01-14 23:25    [W:2.701 / U:0.420 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site