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

[Masahiro Yamada]

Hi Marek,


2017-08-17 20:56 GMT+09:00 Marek Vasut <marek.vasut at gmail.com>:
> On 08/17/2017 09:01 AM, Masahiro Yamada wrote:
>> 2017-08-17 15:39 GMT+09:00 Jaehoon Chung <jh80.chung at samsung.com>:
>>> 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.
>>
>>
>> In my opinion, const is useful for pointer dereference,
>> but unneeded in this case.
>>
>> I believe it is the taste
>> from what I saw from Linux code.  So, I follow it.
>
> I don't care either way, I was just wrestling Masahiro for the sake of
> it (you know the thing about wrestling engineers). There's a V2, so feel
> free to collect that if Masahiro can give me an AB/RB.


I still see const in 1/5 and 3/5.

Please remove them if my AB/RB is needed.



-- 
Best Regards
Masahiro Yamada