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

[Marek Vasut]

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 :)

-- 
Best regards,
Marek Vasut