Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] pinctrl: samsung: use 'int' for register masks in
 Exynos
To:     Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-gpio@vger.kernel.org,
        linux-kernel@vger.kernel.org
Cc:     Marek Szyprowski <m.szyprowski@samsung.com>,
        Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>,
        Tomasz Figa <tomasz.figa@gmail.com>,
        Linus Walleij <linus.walleij@linaro.org>
From:   Sylwester Nawrocki <s.nawrocki@samsung.com>
Message-ID: <3b8d0145-bf01-9ec0-acb3-54081fb57d7f@samsung.com>
Date:   Fri, 9 Apr 2021 13:53:32 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0)
        Gecko/20100101 Thunderbird/78.9.0
MIME-Version: 1.0
In-Reply-To: <20210408195029.69974-1-krzysztof.kozlowski@canonical.com>
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <CGME20210408195041eucas1p1b63616039af4e1156aa4cfd8101b1362@eucas1p1.samsung.com>
        <20210408195029.69974-1-krzysztof.kozlowski@canonical.com>
Precedence: bulk

On 08.04.2021 21:50, Krzysztof Kozlowski wrote:
> The Special Function Registers on all Exynos SoC, including ARM64, are
> 32-bit wide, so entire driver uses matching functions like readl() or
> writel().  On 64-bit ARM using unsigned long for register masks:
> 1. makes little sense as immediately after bitwise operation it will be
>    cast to 32-bit value when calling writel(),
> 2. is actually error-prone because it might promote other operands to
>    64-bit.
> 
> Addresses-Coverity: Unintentional integer overflow
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>

Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>

> ---
> 
> Not tested on ARM64.

I have tested it on exynos5433/tm2e and didn't notice any issues
as we could expect. 
The patch looks good to me, however I would personally use u32
rather than "unsigned int", like in other places for the register
value variables.

--
Regards, 
Sylwester