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

[Marek Vasut <marek.vasut@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@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.

>>> Also, could you use "unsigned int" or "int" for reg?
>>
>> Why?
> 
> 
> Because "unsigned int" or "int" is more natural.
> 
> No reason to use a fixed width variable for the address offset.

You're loosing the benefit of fixed width with using unsigned int though?

-- 
Best regards,
Marek Vasut