Re: [PATCH v4 12/24] PM / devfreq: tegra30: Inline all one-line functions

From: Chanwoo Choi
Date: Thu Jul 18 2019 - 21:24:16 EST


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.
But if you want to use 'inline' keyword, I recommend
that call the 'write_relaxed or read_relaxed' function directly
with detailed description.

>
>>
>>
>>
>
>


--
Best Regards,
Chanwoo Choi
Samsung Electronics