On 19. 7. 19. 오전 11:14, Dmitry Osipenko wrote: > В Fri, 19 Jul 2019 10:27:16 +0900 > Chanwoo Choi <cw00.choi@xxxxxxxxxxx> пишет: > >> On 19. 7. 19. 오전 10:24, Chanwoo Choi wrote: >>> On 19. 7. 19. 오전 10:22, Dmitry Osipenko wrote: >>>> В Thu, 18 Jul 2019 18:09:05 +0900 >>>> Chanwoo Choi <cw00.choi@xxxxxxxxxxx> пишет: >>>> >>>>> On 19. 7. 16. 오후 10:35, Dmitry Osipenko wrote: >>>>>> 16.07.2019 15:26, Chanwoo Choi пишет: >>>>>>> Hi Dmitry, >>>>>>> >>>>>>> I'm not sure that it is necessary. >>>>>>> As I knew, usally, the 'inline' is used on header file >>>>>>> to define the empty functions. >>>>>>> >>>>>>> Do we have to change it with 'inline' keyword? >>>>>> >>>>>> The 'inline' attribute tells compiler that instead of jumping >>>>>> into the function, it should take the function's code and >>>>>> replace the function's invocation with that code. This is done >>>>>> in order to help compiler optimize code properly, please see >>>>>> [1]. There is absolutely no need to create a function call into >>>>>> a function that consists of a single instruction. >>>>>> >>>>>> [1] https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Inline.html >>>>>> >>>>> >>>>> If you want to add 'inline' keyword, I recommend that >>>>> you better to remove the modified function in this patch >>>>> and then just call the 'write_relaxed or read_relaxed' function >>>>> directly. It is same result when using inline keyword. >>>> >>>> That could be done, but it makes code less readable. >>>> >>>> See the difference: >>>> >>>> device_writel(dev, ACTMON_INTR_STATUS_CLEAR, >>>> ACTMON_DEV_INTR_STATUS); >>>> >>>> writel_relaxed(ACTMON_INTR_STATUS_CLEAR, >>>> dev->regs + ACTMON_DEV_INTR_STATUS); >>> >>> No problem if you add the detailed comment and you want to use >>> the 'inline' keyword. >> >> Basically, I think that 'inline' keyword is not necessary. > > Sure, but I'm finding that it's always nicer to explicitly inline a very > simple functions because compiler may not do it properly itself in some > cases. > >> But if you want to use 'inline' keyword, I recommend >> that call the 'write_relaxed or read_relaxed' function directly >> with detailed description. > > Could you please reword this sentence? Not sure that I'm understanding > it correctly. > If you want to used 'inline' keyword, Instead, I recommend that remove 'actmon_readl/writel' wrapper functions and then you calls 'write_relaxed or read_relaxed' function directly with detailed description. -- Best Regards, Chanwoo Choi Samsung Electronics