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

[Jaehoon Chung]

On 08/13/2017 01:55 AM, Marek Vasut wrote:
> 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.

Is there any case about modifying the regs value in this function?
Well..I think that it makes sense about both. (Masahiro and Marek opinion)

But There is nothing wrong to prevent from accidentally.

Best Regards,
Jaehoon Chung

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