linux-kernel - Re: [PATCH RFC 4/8] soc: samsung: Add Exynos Adaptive Supply Voltage driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 24 Apr 2019 10:20:29 +0200
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Sylwester Nawrocki <s.nawrocki@...sung.com>
Cc:     kgene@...nel.org, robh+dt@...nel.org, mark.rutland@....com,
        Chanwoo Choi <cw00.choi@...sung.com>, myungjoo.ham@...sung.com,
        "linux-samsung-soc@...r.kernel.org" 
        <linux-samsung-soc@...r.kernel.org>, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        pankaj.dubey@...sung.com,
        Bartłomiej Żołnierkiewicz 
        <b.zolnierkie@...sung.com>,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: [PATCH RFC 4/8] soc: samsung: Add Exynos Adaptive Supply Voltage driver

On Wed, 24 Apr 2019 at 10:10, Sylwester Nawrocki <s.nawrocki@...sung.com> wrote:
>
> On 4/23/19 12:50, Krzysztof Kozlowski wrote:
> >> +static int exynos5422_asv_get_table(void)
> >> +{
> >> +       unsigned int reg = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID);
> >
> > One more thought: you read this register multiple times in the same
> > function. I think it is not needed - just read once, store the value
> > and use the helpers to parse it.
>
> Yes, I have noticed that as well. I'm not sure though if it is worth
> to additionally cache registers manually like this when we use the
> regmap.  I have already converted that code to use the regmap API for
> v2.  And these are barely few registers reads at the driver
> initialization time, not any hot path.

By default regmap does not use caching so there is no benefit out of
it. In the same time reading with regmap involves its layer of
abstraction with locks, indirect calls etc... I agree that there is no
point for implementing specific "caching" but with the same code
readability/simplicity you can have it without multiple regmap
accesses one after another. Consider this:
    unsigned int pkgid = exynos_chipid_read(EXYNOS_CHIPID_REG_PKG_ID);
    asv->table = exynos5422_asv_get_table(pkgid);
    asv->is_bin2 = exynos5422_asv_check_bin2(pkgid);
(and probably renaming the helpers)
The code is equivalent. The same readability.

Best regards,
Krzysztof