public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Marek Vasut <marek.vasut@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/5] mmc: uniphier-sd: Factor out register IO
Date: Thu, 17 Aug 2017 13:56:36 +0200	[thread overview]
Message-ID: <fe0137de-c44f-9632-1e70-fe2ff444499f@gmail.com> (raw)
In-Reply-To: <CAK7LNAR_TT_XNwZYjO_7mMeYfiQB=CQzzw5PBW-Ucp=xL+q3nw@mail.gmail.com>

On 08/17/2017 09:01 AM, Masahiro Yamada wrote:
> 2017-08-17 15:39 GMT+09:00 Jaehoon Chung <jh80.chung@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@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.
>>
>> 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

  reply	other threads:[~2017-08-17 11:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20170721212451epcas1p4f79a1439eee9bb5cba0b23204b9c8412@epcas1p4.samsung.com>
2017-07-21 21:24 ` [U-Boot] [PATCH 1/5] mmc: uniphier-sd: Factor out register IO Marek Vasut
2017-07-21 21:24   ` [U-Boot] [PATCH 2/5] mmc: uniphier-sd: Add support for 64bit controller Marek Vasut
2017-07-21 21:24   ` [U-Boot] [PATCH 3/5] mmc: uniphier-sd: Add support for 64bit FIFO Marek Vasut
2017-08-03 12:40     ` Masahiro Yamada
2017-07-21 21:24   ` [U-Boot] [PATCH 4/5] mmc: uniphier-sd: Add support for quirks Marek Vasut
2017-07-21 21:24   ` [U-Boot] [PATCH 5/5] mmc: uniphier-sd: Add support for R8A7795 and R7A7796 Marek Vasut
2017-08-03 12:36   ` [U-Boot] [PATCH 1/5] mmc: uniphier-sd: Factor out register IO Masahiro Yamada
2017-08-05 19:23     ` Marek Vasut
2017-08-07  2:30       ` Masahiro Yamada
2017-08-07  8:30         ` Marek Vasut
2017-08-10  7:49           ` Masahiro Yamada
2017-08-12 16:55             ` Marek Vasut
2017-08-17  6:39               ` Jaehoon Chung
2017-08-17  7:01                 ` Masahiro Yamada
2017-08-17 11:56                   ` Marek Vasut [this message]
2017-08-20 14:24                     ` Masahiro Yamada
2017-08-17  6:55               ` Masahiro Yamada
2017-08-17  6:47   ` Jaehoon Chung

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fe0137de-c44f-9632-1e70-fe2ff444499f@gmail.com \
    --to=marek.vasut@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox