From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marek Vasut Date: Thu, 17 Aug 2017 13:56:36 +0200 Subject: [U-Boot] [PATCH 1/5] mmc: uniphier-sd: Factor out register IO In-Reply-To: References: <20170721212436.26073-1-marek.vasut+renesas@gmail.com> <82da5c44-de05-0322-ba43-5c818c2e2a3a@gmail.com> <8b8aff4a-c4b9-f636-8c58-6af47643ddac@gmail.com> <843e160b-1b92-9fc4-efa7-ffa75683f0c0@samsung.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 08/17/2017 09:01 AM, Masahiro Yamada wrote: > 2017-08-17 15:39 GMT+09:00 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 : >>>>> 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 : >>>>>>> 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