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

Marek Vasut marek.vasut at gmail.com
Thu Aug 17 11:56:36 UTC 2017


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.

-- 
Best regards,
Marek Vasut


More information about the U-Boot mailing list