[U-Boot] [PATCH 1/5] mmc: uniphier-sd: Factor out register IO

[Masahiro Yamada]

2017-08-13 1:55 GMT+09:00 Marek Vasut <marek.vasut at gmail.com>:
> On 08/10/2017 09:49 AM, Masahiro Yamada wrote:
>> Hi.
>>
>>
>> 2017-08-07 17:30 GMT+09:00 Marek Vasut <marek.vasut at gmail.com>:
>>> On 08/07/2017 04:30 AM, Masahiro Yamada wrote:
>>>> Hi Marek,
>>>
>>> Hi Masahiro,
>>>
>>> This is gonna be a great discussion, let's wrestle about consts and ints :-)
>>>
>>>> 2017-08-06 4:23 GMT+09:00 Marek Vasut <marek.vasut at gmail.com>:
>>>>> On 08/03/2017 02:36 PM, Masahiro Yamada wrote:
>>>>>> Hi Marek,
>>>>>
>>>>> Hi,
>>>>>
>>>>> [...]
>>>>>
>>>>>>> +static u32 uniphier_sd_readl(struct uniphier_sd_priv *priv, const u32 reg)
>>>>>>
>>>>>> "const" is unneeded here.
>>>>>
>>>>> Why? The function should not modify reg , so it is const.
>>>>
>>>>
>>>> Because "const" is useless here.
>>>>
>>>> The "reg" is not a pointer, so it is obvious
>>>> that there is no impact to the callers.
>>>>
>>>>
>>>>
>>>> Moreover, whether "reg" is constant or not
>>>> depends on how you implement the function.
>>>>
>>>>
>>>> If you force "const" to the argument, the only choice for the implementation
>>>> will be as follows:
>>>>
>>>>
>>>>
>>>> static u32 uniphier_sd_readl(struct uniphier_sd_priv *priv, const u32 reg)
>>>> {
>>>>       if (priv->caps & UNIPHIER_SD_CAP_64BIT)
>>>>              return readl(priv->regbase + (reg << 1));
>>>>       else
>>>>              return readl(priv->regbase + reg);
>>>> }
>>>>
>>>>
>>>>
>>>> If you want to implement the function as follows, you need to drop "const".
>>>>
>>>> static u32 uniphier_sd_readl(struct uniphier_sd_priv *priv, u32 reg)
>>>> {
>>>>       if (priv->caps & UNIPHIER_SD_CAP_64BIT)
>>>>               reg <<= 1;
>>>>
>>>>       return readl(priv->regbase + reg);
>>>> }
>>>
>>> My argument would be that the const prevents you from accidentally
>>> modifying the $reg inside the function.
>>
>>
>> No.
>> The arguments of a functions are local variables.
>> No impact on the outside of the scope of the function.
>
> I'm not arguing about that.
>
>> If you accidentally modify a variable where it should not,
>> it is a bug.  Just fix it.
>
> If it's const, compiler will warn the user he's doing something stupid.
>
>> If you want to wrestle more, please do it in LKML
>> by adding around "const".
>>
>> For example,
>>
>> drivers/base/regmap/regmap.c:
>> int regmap_write(struct regmap *map, unsigned int reg, unsigned int val)
>>
>> arch/arm/include/asm/io.h:
>> static inline void __raw_writel(u32 val, volatile void __iomem *addr)
>
> Well, there were some cocci patches adding const recently :)
>


Do you mean cocci scripts in scripts/coccinelle ?
I checked linux-next, but I could not find it.




-- 
Best Regards
Masahiro Yamada